[Chapel Merge] Adjust inout to be implemented with a single argum

Notifications Commits
1

Branch: refs/heads/master
Revision: 0dddd26
Author: mppf
Log Message:

Merge pull request #16180 from mppf/inout-single-argument

Adjust inout to be implemented with a single argument

Resolves #16290
Resolves #16185
Resolves #16195
Resolves #16148
Resolves #16275
Resolves #16301
Resolves #16298
Resolves #16300
Resolves #16007

This PR takes the following steps:

  • changes inout to use a single argument instead of two arguments
  • removes chpl__unref and instead the compiler treats types that have
    chpl__initCopy return a different type specially. Additionally, for
    domains, instead of relying on runtime information, use compiler
    analysis to identify functions that return unowned domains as with
    A.domain.
  • allows types that have chpl__initCopy to return a different type to
    coerce to that type. This allows iterators to pass to array arguments
    with in intent (requested in issue #16007).

Follow-up to PR #16143 which changed inout to be implemented with an
in argument and an out argument. In reviewing that PR, Vass pointed
out that there might be alternatives to the two-argument approach. Since
the copy-for-in and write-back-for-out are both now occurring at the call
site, it is possible for the function body to just accept a ref
argument for inout. The function will accept the result of the
copy-for-in and then modify it. The code at the call site will do the
write-back from that to the original variable. This PR makes that change.
The implementation involved adjusting wrappers.cpp and adding a map to
track the value copied from in the in intent part of inout for use in
the out intent part.

There is one inout behavior change:

proc out_inout(out a: R, inout b: R) {
  writeln("a is ", a);
  writeln("b is ", b);
  a = new R(10);
  b = new R(20); // here
}
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);
}
test_out_inout();

This deinitializes new R(20) inside of the body of out_inout (instead
of transferring that responsibility to the call site). As a result, the
deinitialization order for new R(20) is different (20 is deinitialized
before 10). The order of write-backs and other deinitializations at the
call site is still the same.

In the process of addressing problems with inout for arrays, I
identified several issues related to array views and copy semantics such
as #16185 and #16195. This caused me to make the copy changes described
above including removing chpl__unref.

Here is a more detailed summary of changes:

  • Adjusts wrappers.cpp, normalize.cpp to use a single argument for inout
    handling
  • Adds logic to getInstantiationType that considers if the type should
    be used for a value to be returned/stored in a variable/passed by in
    intent. This calls getCopyTypeDuringResolution which keeps track of
    resolved initCopy calls and the types that they return. When the
    result of initCopy produces a different type, then the type needs
    special handling here. In particular, for example, proc f(in arg)
    called with an array view should instantiate with a non-view array
    (resulting from the copy).
  • Adjusts canCoerce to use getCopyTypeDuringResolution and to allow
    coercion from a type to the result of initCopy on that type when
    working with an in/const in/inout formal argument.
  • Removes chpl__unalias and most chpl__unref calls; replaces these
    with initCopy
  • For domains that are “borrowed” such as A.domain, use simple
    compile-time tracking of such domains to add an initCopy for
    returning the borrowed domain or passing it to in intent. Adds
    isCallExprTemporary and isTemporaryFromNoCopyReturn to implement
    this analysis.
  • adjust insertUnrefForArrayOrTupleReturn to use initCopy instead of
    chpl__unalias and to use the domain analysis to call it for cases
    like A.domain
  • adjusts split init to use the domain analysis for cases like
    A.domain
  • makes rank change and reindex arrays include param ownsArrInstance
    to distinguish between rank change/reindex arrays that own their
    elements vs those that do not. Adjusted isRankChangeArrayView /
    isReindexArrayView to return false for such arrays that own their
    elements.
  • adds PRIM_SET_ALIASING_ARRAY_ON_TYPE and uses it in rank change /
    reindex array views so that the FLAG_ALIASING_ARRAY flag on the type
    is set appropriately and according to `param ownsArrInstance.
  • move isAliasingArrayImplType and isAliasingArrayType to type.cpp so
    they can be used in more than one place.
  • add FLAG_NO_COPY_RETURNS_OWNED to work around an order-of-resolution
    issue.
  • removes an old, no longer necessary, workaround for initCopy functions
    in updateVariableAutoDestroy.
  • updated --print-module-resolution output to print out the path of
    modules being resolved for additional information. This is only
    currently tested in the test named print-module-resolution.chpl.
  • removed dead code in postFoldPrimop
  • adjusted tuple code to avoid copying ref tuples in tuple init
    functions and to copy all elements in initCopy functions
  • adjusts <<= and >>= functions to accept an integral shift amount
    the way that << and >> do. This was to work around an issue that
    is no longer present but seemed like an improvement.
  • adjusts sync/single initializers to use concrete const ref intent
    for sync/single arguments instead of const. This was to work around
    an issue that is no longer present but seems like an improvement.

Reviewed by @e-kayrakli - thanks!

  • [x] primers pass with verify+valgrind and do not leak
  • [x] primers pass with gasnet
  • [x] full local futures testing

Future work

  • nested call in function call returning runtime type executes twice
    #16316
  • This PR makes some errors relating to returning tuples of owned
    runtime errors instead of compile-time errors. See #14942. This should
    be revisited after the resolution of #16233 / #15973.
  • #16339 about updating the spec description of how ref intent impacts
    candidate selection

Modified Files:
A test/analysis/alias/array-arguments-reindex-rank-change.chpl
A test/analysis/alias/array-arguments-reindex-rank-change.compgood
A test/analysis/alias/array-arguments-reindex-rank-change.execgood
A test/arrays/ferguson/iterator-to-in-issue-16007.chpl
A test/arrays/ferguson/iterator-to-in-issue-16007.good
A test/arrays/ferguson/semantic-examples/9-issue-16185.chpl
A test/arrays/ferguson/semantic-examples/9-issue-16185.good
A test/arrays/ferguson/semantic-examples/9-issue-16195.chpl
A test/arrays/ferguson/semantic-examples/9-issue-16195.good
A test/arrays/ferguson/semantic-examples/9-slice-operations.chpl
A test/arrays/ferguson/semantic-examples/9-slice-operations.good
A test/arrays/ferguson/views-and-copying.chpl
A test/arrays/ferguson/views-and-copying.good
A test/domains/ferguson/array-domain-and-copying.chpl
A test/domains/ferguson/array-domain-and-copying.good
A test/domains/ferguson/array-domain-and-ifexprs.chpl
A test/domains/ferguson/array-domain-and-ifexprs.good
A test/parallel/forall/checks/forall-inout-domain-bug-16301.chpl
A test/parallel/forall/checks/forall-inout-domain-bug-16301.good
R test/arrays/ferguson/inout-tuple-ref-wait-array.bad
R test/arrays/ferguson/inout-tuple-ref-wait-array.future
R test/arrays/ferguson/inout-tuple-ref-wait-int.bad
R test/arrays/ferguson/inout-tuple-ref-wait-int.future
R test/arrays/ferguson/semantic-examples/CLEANFILES
R test/arrays/ferguson/semantic-examples/PRECOMP
R test/arrays/ferguson/semantic-examples/PREDIFF
R test/classes/initializers/compilerGenerated/copyDomainFormal.bad
R test/classes/initializers/compilerGenerated/copyDomainFormal.future
R test/classes/initializers/compilerGenerated/modifyDomain-array-created-by-init.notest
R test/users/ferguson/bulkerr.bad
R test/users/ferguson/bulkerr.future
R test/users/shetag/tensor.bad
R test/users/shetag/tensor.future
R test/users/thom/arrArgIn-promote.bad
R test/users/thom/arrArgIn-promote.future
M compiler/AST/primitive.cpp
M compiler/AST/symbol.cpp
M compiler/AST/type.cpp
M compiler/include/flags_list.h
M compiler/include/primitive_list.h
M compiler/include/resolution.h
M compiler/include/type.h
M compiler/optimizations/noAliasSets.cpp
M compiler/passes/normalize.cpp
M compiler/passes/splitInit.cpp
M compiler/resolution/AutoDestroyScope.cpp
M compiler/resolution/ResolutionCandidate.cpp
M compiler/resolution/callDestructors.cpp
M compiler/resolution/fixupExports.cpp
M compiler/resolution/functionResolution.cpp
M compiler/resolution/generics.cpp
M compiler/resolution/postFold.cpp
M compiler/resolution/preFold.cpp
M compiler/resolution/resolveFunction.cpp
M compiler/resolution/tuples.cpp
M compiler/resolution/virtualDispatch.cpp
M compiler/resolution/wrappers.cpp
M modules/internal/ArrayViewRankChange.chpl
M modules/internal/ArrayViewReindex.chpl
M modules/internal/ChapelArray.chpl
M modules/internal/ChapelBase.chpl
M modules/internal/ChapelLocale.chpl
M modules/internal/ChapelSyncvar.chpl
M modules/internal/ChapelTuple.chpl
M modules/minimal/internal/ChapelBase.chpl
M test/analysis/alias/array-arguments-in.compgood
M test/analysis/alias/array-of-classes-variant.compgood
M test/analysis/alias/array-of-classes.compgood
M test/arrays/ferguson/inout-tuple-ref.good
M test/arrays/ferguson/semantic-examples/EXECOPTS
M test/arrays/shapes/tmacd-promotion.chpl
M test/arrays/shapes/tmacd-promotion.good
M test/arrays/views/initArrFieldWithSlice.bad
M test/classes/delete-free/lifetimes/arr-dom.chpl
M test/classes/delete-free/lifetimes/ref-escapes.good
M test/classes/delete-free/owned/owned-transfer-from-nonnil-tuple.bad
M test/classes/ferguson/override-intents.good
M test/compflags/ferguson/print-module-resolution.good
M test/compflags/ferguson/print-module-resolution.prediff
M test/expressions/loop-expr/forall-over-zip-over-arrays.chpl
M test/functions/vass/varargs-1.good
M test/library/standard/DataFrames/DataFrames.chpl
M test/parallel/taskPar/taskIntents/03-error-in-fun.good
M test/studies/uts/sha1_rng.chpl
M test/studies/uts/uts_rec.chpl
M test/types/records/intents/inout-intent-order.good
M test/users/npadmana/twopt/do_smu_soa-error.bad
M test/users/shetag/tensor.chpl

Compare: https://github.com/chapel-lang/chapel/compare/0dcc6e59f7b1...0dddd2683fa6