Add the collective subroutine, co_sum, to the list of intrinsics. In accordance with 16.9.50 and 16.9.137, add a check for and an error if coindexed objects are being passed to certain arguments in co_sum and in move_alloc. Add a semantics test to check that this error is successfully caught in calls to move_alloc. Remove the XFAIL directive, update the ERROR directives and add standard-conforming and non-standard conforming calls in the semantics test for co_sum.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
flang/lib/Evaluate/intrinsics.cpp | ||
---|---|---|
1088 | "A=", "STAT=", and "ERRMSG=" are not allowed to be coindexed objects (16.9.50); how will you enforce and test this restriction? |
- Fix rank of the co_sum dummy argument a in flang/lib/Evaluate/intrinsics.cpp
- Add check for and error if a coindexed object is passed to dummy arguments a, stat, and errmsg in co_sum, and dummy arguments from, to, stat, and errmsg in move_alloc.
- Add semantics test to ensure that coindexed error is being caught in calls to move_alloc.
- Update semantics test for co_sum by fixing ERROR directives for the new error and adding one non-standard conforming call.
flang/lib/Evaluate/intrinsics.cpp | ||
---|---|---|
1088 | Thanks for the feedback and sorry that it took me so long to respond. |
flang/lib/Evaluate/intrinsics.cpp | ||
---|---|---|
1657 | I think this custom check might better belong in ApplySpecificChecks that does custom checks that do not fit in the automated checks. |
flang/lib/Evaluate/intrinsics.cpp | ||
---|---|---|
1657 | Thanks for pointing that function out. I agree that moving this check to ApplySpecificChecks() seems like an appropriate change. I will work on doing that and updating the diff. Thanks for the feedback! |
Move check for coindexed objects for certain arguments in co_sum and move_alloc to a new function CheckForCoindexedObjects, which is called from ApplySpecificChecks for the specific cases of co_sum and move_alloc. Add some calls to the semantics tests to better check this implementation.
I pushed this change today and had to revert the commit because it caused some unexpected test failures. The three tests that failed on the buildbots (though these tests didn't consistently fail across the different buildbots) were all flang semantics tests and are:
- flang/test/Semantics/collective01.f90 - the test that I removed the XFAIL directive from in this differential
- flang/test/Semantics/move_alloc.f90 - the new test from this differential
- flang/test/Semantics/doconcurrent01.f90 - a test with move_alloc calls
This reopens the revision.
Updated the new function CheckForCoindexedObjects (and renamed it CheckForCoindexedObject) to take one ActualArgument and to check just that argument for a coindexed object. This fixes the non-deterministic test failure I was finding with my previous implementation, plus it also is a better function for when I add the atomic subroutines and use it to check only one of their arguments.
Forgot to run git-clang-format after one last small change I made, so this updated diff fixes the one line changed by git-clang-format.
@jeanPerier I have updated this differential since you approved it, would you like to check the changes? My new function now just checks one ActualArgument to make sure it is not a coindexed-object, instead of a list of ActualArguments. This fixes some test failures I was getting, and also I think matches better the needs of the other subroutines that will need to call this function, like the atomic subroutines, which all have only one ActualArgument that shall not be coindexed.
"A=", "STAT=", and "ERRMSG=" are not allowed to be coindexed objects (16.9.50); how will you enforce and test this restriction?