Chapel: initialization and deinitialization order of formal temps

[mppf]

The current ordering of copy-initialization, write-back, and de-initialization for in, inout, and out formal temps does not seem right.

I would expect that in / inout / out arguments would:

• have any required copy-initialization occur in argument order
• have write-backs (for inout) in argument order
• have deinitialization occur in reverse argument order

However this is not the case right now:

• Copy-initialization for all in intent arguments is occuring before all inout arguments
• inout argument copy initialziation is occuring in reverse argument order
• in andinout deinitialization is occuring in argument order (but out deinitialization occurs in reverse argument order)

Example test hidden below:

proc inout_inout(inout a: R, inout b: R) { }

proc test_inout_inout() {
  writeln();
  writeln("test_inout_inout");
  var a = new R(1);
  var b = new R(2);
  inout_inout(a, b);
  writeln(a, " ", b); // disabling copy elision for a and b
}
test_inout_inout();

proc in_inout(in a: R, inout b: R) { }

proc test_in_inout() {
  writeln();
  writeln("test_in_inout");
  var a = new R(1);
  var b = new R(2);
  in_inout(a, b);
  writeln(a, " ", b); // disabling copy elision for a and b
}
test_in_inout();

proc inout_in(inout a: R, in b: R) { }

proc test_inout_in() {
  writeln();
  writeln("test_inout_in");
  var a = new R(1);
  var b = new R(2);
  inout_in(a, b);
  writeln(a, " ", b); // disabling copy elision for a and b
}
test_inout_in();

proc in_in(in a: R, in b: R) { }

proc test_in_in() {
  writeln();
  writeln("test_in_in");
  var a = new R(1);
  var b = new R(2);
  in_in(a, b);
  writeln(a, " ", b); // disabling copy elision for a and b
}
test_in_in();

proc out_out(out a: R, out b: R) {
  a = new R(10);
  b = new R(20);
}

proc test_out_out() {
  writeln();
  writeln("test_out_out");
  var a = new R(1);
  var b = new R(2);
  out_out(a, b);
  writeln(a, " ", b); // disabling copy elision for a and b
}
test_out_out();

proc out_inout(out a: R, inout b: R) {
  a = new R(10);
  b = new R(20);
}

proc test_out_inout() {
  writeln();
  writeln("test_out_inout");
  var a = new R(1);
  var b = new R(2);
  out_inout(a, b);
  writeln(a, " ", b); // disabling copy elision for a and b
}
test_out_inout();

proc inout_out(inout a: R, out b: R) {
  a = new R(10);
  b = new R(20);
}

proc test_inout_out() {
  writeln();
  writeln("test_inout_out");
  var a = new R(1);
  var b = new R(2);
  inout_out(a, b);
  writeln(a, " ", b); // disabling copy elision for a and b
}
test_inout_out();








class C {
  var xx: int = 0;
}

record R {
  var x: int = 0;
  var ptr: shared C = new shared C(0);
  proc init() {
    this.x = 0;
    this.ptr = new shared C(0);
    writeln("init (default)");
  }
  proc init(arg:int) {
    this.x = arg;
    this.ptr = new shared C(arg);
    writeln("init ", arg, " ", arg);
  }
  proc init=(other: R) {
    this.x = other.x;
    this.ptr = new shared C(other.ptr.xx);
    writeln("init= ", other.x, " ", other.ptr.xx);
  }
  proc deinit() {
    writeln("deinit ", x, " ", ptr.xx);
  }
  proc toString() {
    return "(" + this.x:string + " " + this.ptr.xx:string + ")";
  }
  proc set1() {
    this.x = 1;
    this.ptr.xx = 1;
    return this;
  }
}
proc =(ref lhs:R, rhs:R) {
  writeln("lhs", lhs.toString(), " = rhs", rhs.toString());
  lhs.x = rhs.x;
  lhs.ptr = new shared C(rhs.ptr.xx);
}

[bradcray]

Your expectations sound good to me. If for some reason it turned out to seem more natural to have write-backs take place in reverse argument order as well, I could imagine that being acceptable (neither order seems immediately more obviously correct to me).

[cassella]

The difference in write-back order is visible when the same lvalue is passed to more than one out or inout intent arguments:

proc foo1(out   x, out   y) {x = 1; y = 2;}
proc foo2(out   x, inout y) {x = 1; y = 2;}
proc foo3(inout x, out   y) {x = 1; y = 2;}
proc foo4(inout x, inout y) {x = 1; y = 2;}

var x = 0;

foo1(x, x); writeln(x);
foo2(x, x); writeln(x);
foo3(x, x); writeln(x);
foo4(x, x); writeln(x);

1
2
2

[mppf]

I'm running into some challenges with inout specifically.

It would be appealing to think of inout as actually 2 arguments: an in argument and an out one. Then, the in part would work just like any other in argument as would the out part. (The body of the function would move from the in formal to a formal temp var; and at the end, move from the formal temp var to the out formal). The problem with this approach is that I can't think of a way to do it without actually adding 2 arguments.

(The issue is that the in intent arguments are handled at the call site in order to enable copy elision. As a result, if we want the copies for inout arguments to interleave with those for in arguments, then the copy would occur at the call site. But the write-back for out intent occurs within the body of the function. In order to express the write-back, the function needs to have both the current value (which could come from the copy from the in intent) as well as the final destination value (which is missing if we start by treating inout like in).

Now, having Chapel routines using inout intent actually end up with 2 arguments is generally OK but it might present problems for interoperability. For extern proc functions using inout intent we could disallow it or convert it to ref. But for export proc functions using inout intent there would actually be 2 arguments and that might be confusing.

So I can see two alternatives:

1. Accept that copies for inout intent arguments will happen after copies from in arguments
2. Translate inout into 2 arguments and accept any consequences that has for export procs.

[mppf]

@lydia-duncan @bradcray - I am wondering about your reaction to the above.

[lydia-duncan]

What's the argument against following the same path for export proc as extern proc?

[bradcray]

I feel like we ran into this in some other effort in the past six months, is that familiar? (or was it just a previous attempt at this?)

I don't necessarily have a problem with changing it into two arguments under the covers if that's our best/only way forward, though it does seem a little odd / unfortunate (for example, I could imagine it resulting in confusion to myself as a developer reading the generated C code until I'd learned to expect it).

In an interoperability scenario, is there an implication that an extern/export routine using in/out/inout puts some burden or expectation on the caller/callee on the other side? (which also seems weird to me... like if export proc foo(in x: int) meant "you need to make a copy of your integer before you send it to foo()", that would feel like a weird/unnecessary burden on the user. So then my next thought would be whether we could hide such burdens in a wrapper that is actually exported. I.e., the Chapel-side version of the function splits inout into two arguments (or expects the copy for in to hae already happened) while the wrapper version takes a single argument by ref and splits it into two arguments for the Chapel-side call (and/or does the required/implied copy in). Is that workable?

[mppf]

What's the argument against following the same path for export proc as extern proc?

For an extern proc we aren't implementing the function body so the difference between inout and ref isn't really something we can control. But for an export proc the author of the routine might expect inout to actually do the copies.

[lydia-duncan]

Are you reversing export and extern mentally? Cuz I would expect the opposite myself

[lydia-duncan]

But it could also be that I'm misunderstanding what you mean

[mppf]

Oh yeah I did reverse them, sorry, just fixed in an edit.

[mppf]

Regarding this test case suggested by @cassella -

proc foo1(out   x, out   y) {x = 1; y = 2;}
proc foo2(out   x, inout y) {x = 1; y = 2;}
proc foo3(inout x, out   y) {x = 1; y = 2;}
proc foo4(inout x, inout y) {x = 1; y = 2;}

var x = 0;

foo1(x, x); writeln(x);
foo2(x, x); writeln(x);
foo3(x, x); writeln(x);
foo4(x, x); writeln(x);

These all print out 2 with the exception of foo2 which prints out 1. The reason that foo2 prints out 1 is that the out intent involves returning the result of out into a temp which is then copied into the actual argument passed. As a result, the out x part of foo2(x, x) is the last thing replacing the value of x.

I don't see a way around this - because the out intent handling benefits from assuming that the function is initializing the value. I wouldn't want to change that element of it.

Anyway I have a branch which makes the writebacks within the body of the function (not involving the out temp as above) always happen in argument order.

[mppf]

I don't see a way around this - because the out intent handling benefits from assuming that the function is initializing the value. I wouldn't want to change that element of it.

Oh, I could move the writeback for inout to run = at the call site. Arguably, out is also doing the writebacks at the call site. I'll give that a try.

[mppf]

I have a start at this in PR #16143.

[mppf]

The two-argument approach is running into problems with varargs. Because varargs are related to tuples, even if I added code to handle calling varargs functions with inout, the tuple you would need to pass to them would need to be twice as long. Of course we could adjust the compiler to handle this case but it seems tricky and more of a corner case. So I am thinking I will make this case an error or a warning for the time being. If we make it a warning, it would convert the inout intent to a ref intent and print something like "inout varargs converted to ref varargs".

[bradcray]

I'd be OK with not supporting inout varargs in favor of improving init/deinit order and issuing an error that suggests that users should consider using ref varargs. I don't think we should automatically convert it because I think it'd be better for their source code to be updated once to reflect what's really happening and avoid the warning than to support it but change the meaning.

[mppf]

Issue #16148 is for the varargs case. I can make it into an error.

Issue #16149 brings up another issue I ran into for iterators (which I plan to make an error for now).

[mppf]

@vasslitvinov asked in the review of #16143 about alternatives to having two arguments. This led me to realize that we don't actually need 2 arguments anymore since the copy-in and copy-out both occur at the call site. The function itself can just treat it as a ref argument (accepting the result of the copy-in).

[mppf]

In PR #16180 I am exploring this single argument approach for inout.