Edge case where compiler inserts unneeded array copy with the `in` intent about chapel HOT 4 CLOSED

In terms of what ought to happen, IMO the original case should not get the extra copy:

proc ArrayWrapper.init(in a: [?d] ?t) {
    this.etype = t;
    this.rank = d.rank;
    this.shape = d.shape;
    this.a = a;

    writeln(d); // it's not referring to 'a' so 'a' should be copy elided
}

In contrast, this pattern should disable copy elision:

proc ArrayWrapper.init(in a: [?d] ?t) {
    this.etype = t;
    this.rank = d.rank;
    this.shape = d.shape;
    this.a = a;

    writeln(a.domain); // it's mentioning 'a' so should disable copy elision for 'a'
}

I suspect that the current situation here is largely due to how the production compiler represents d in this function.

from chapel.

@benharsh - one way would be for the normalization to introduce const ref tmp = a._dom near the beginning of the function, since it's using it to replace something from a formal. (And then use that instead of d).

Another strategy would be to wait for the dyno implementation of split init / copy elision, which will happen pre-normalization.

from chapel.

This appears to be related to an intersection between the "normalization" of d and our split-init implementation.

The writeln(d) call is transformed during normalization into a call to a._dom, which from the compiler's perspective is indistinguishable from a user-specified reference to a.

The elideCopies mini-pass in splitInit.cpp is the part that's actually responsible for getting rid of the extra init-copy for the array a. When it sees the reference to a after the init-copy, it gives up and decides not to remove the copy. For some more context, the line for this.a = a looks more like:

var a_temp = formal_a;
move(a, a_temp);

@mppf , do you have any insight as to how we might make this work within the rules of split init?

from chapel.

I believe I have a solution here: #24416

from chapel.