Add semantic checks and the corresponding lit test cases for the following OpenMP 4.5 clauses
- 2.15.4.2 - Copyprivate clause
- 2.15.3.4 - Firstprivate clause
- 2.15.3.5 - Lastprivate clause
Differential D91920
[Flang] [OpenMP] Add semantic checks for OpenMP firstprivate , lastprivate and copyprivate clauses praveen on Nov 21 2020, 11:19 AM. Authored by
Details Add semantic checks and the corresponding lit test cases for the following OpenMP 4.5 clauses
Diff Detail
Unit Tests
Event TimelineComment Actions
Can you check why a unit test is failing? Comment Actions @kiranchandramohan In the test file omp-clause-validity01.f90 , the following no symbol found errors are not being thrown. !ERROR: Internal: no symbol found for 'i' !$omp taskwait depend(sink:i-1) !ERROR: Internal: no symbol found for 'first' !$omp critical (first) Since the errors related to copyprivate clause in omp-clause-validity01.f90 (line numbers 324 and 340 above) being thrown as part of the checks for copyprivate in resolve-directives.cpp are marked as fatal errors , the flag errorOnUnresolvedName_ is being set to false and the no symbol found error is not being thrown. RewriteMutator(SemanticsContext &context) : errorOnUnresolvedName_{!context.AnyFatalError()}, messages_{context.messages()} {} void RewriteMutator::Post(parser::Name &name) { if (!name.symbol && errorOnUnresolvedName_) { messages_.Say(name.source, "Internal: no symbol found for '%s'"_err_en_US, name.source); } } Is it not necessary to throw the "no symbol found" error if there is any error marked as fatal while resolving the directives ? Should all the checks related to copyprivate be moved to check-omp-structure.cpp? Can you please suggest the approach to follow for these changes? Thanks! Comment Actions I think these checks can stay in the current file since they need to check the data sharing attributes.
Is this OK @clementval ?
Comment Actions @kiranchandramohan @kiranktp @clementval I have commented the test cases which were failing with "no symbol found" error.
Comment Actions A few comments.
Comment Actions Handle all the private symbols in enclosing context for firstprivate and lastprivate clauses Comment Actions @kiranchandramohan @sameeranjoshi Will the patch https://reviews.llvm.org/D93105 be committed to the master again?
Comment Actions I believe the checks have to be made more precise. Please have a look at the comments.
Comment Actions Quick comments/questions before I have a final look.
Comment Actions A few more questions.
Comment Actions If you can add the autogen portion that will be great. It can be added either as part of this patch or a separate patch. But it need not be a blocker for this. I am approving this patch.
Comment Actions @kiranchandramohan The test case flang/test/Semantics/resolve102.f90 is failing with the error messages possibly due to the commit actual at 13: Procedure 'p' is recursively defined. Procedures in the cycle: 'p2', 'sub', 'p' Can I push this patch now or only after the above failed case is fixed ?
|
Can you add comments on why these Data Structures are necessary either here or where they are used?