[flang][OpenMP] Add one semantic check for data-sharing clauses

[peixin]

Summary

As OpenMP 5.0, for firstprivate, lastprivate, copyin, and copyprivate
clauses, if the list item is a polymorphic variable with the allocatable
attribute, the behavior is unspecified.

[peixin]

BTW, this is the only missed semantic check for copyin clause.

[kiranchandramohan]

flang/test/Semantics/OpenMP/omp-copyin06.f90

11 ↗

(On Diff #436226)

Do warnings work with test_errors?

[kiranchandramohan]

This test is applicable for lastprivate, firstprivate, and copyprivate as well. Could you check and add for these as well?

[peixin]

In D127601#3577825, @kiranchandramohan wrote:

This test is applicable for lastprivate, firstprivate, and copyprivate as well. Could you check and add for these as well?

Sure. Will take a look at them.

flang/test/Semantics/OpenMP/omp-copyin06.f90

11 ↗

(On Diff #436226)

Not yet. It is ongoing work in https://reviews.llvm.org/D125804. There are a number of test cases with WARNINGs, so maybe no need to wait for it to be merged?

[peixin]

In D127601#3577825, @kiranchandramohan wrote:

This test is applicable for lastprivate, firstprivate, and copyprivate as well. Could you check and add for these as well?

Confirmed and added the checks for other clauses as well.

There are some test cases using FileCheck for warnings. Use FileCheck for warnings.

[kiranchandramohan]

Thanks. LGTM.

[ekieri]

I think you can use the WARNING-directives without waiting for D125804. It will take me some time to land it, and I do not think it should block other developments. In the meantime, it would only be helpful if WARNING-directives are included in new (and existing) tests, like you did here. It would, for now, give a false impression of the warnings being tested, but at least they will be in the future (and if it regresses before then, I will notice). Refraining from testing warnings in the meantime comes with the risk of forgetting to add the tests later.

(This reasoning applies to D125891 too, and I would by the same arguments propose that you commit (after review) the test case you made for D125371 as well.)

Using FileCheck is of course an option, but the test you provided does not fail on unexpected warnings. In the RFC for D125804, it was generally agreed this is important. This, I learned just now, can be achieved with FileCheck using --implicit-check-not (cf. D126291), but check_errors.py will (when D125804 is landed) be both simpler to use and more rigorous (as it also checks that the warning is caused by the expected line). I do not object to using FileCheck (in general, I think the common infra should be preferred), but if you choose to do so, please use -- implicit-check-not for both warning and portability.

[peixin]

Change FileCheck into WARNING preparing for D125804.

[peixin]

(This reasoning applies to D125891 too, and I would by the same arguments propose that you commit (after review) the test case you made for D125371 as well.)

I am not aware of your patch at that time. Sorry for the inconvenience. Will add the test later.

check_errors.py will (when D125804 is landed) be both simpler to use and more rigorous (as it also checks that the warning is caused by the expected line).

Agree. I will use FileCheck for review since it is easy for reviewers to ensure the warnings are generated correctly. After accepted, I will change the FileCheck into WARNINGs to ease the burden of preparing for your patch.

flang/lib/Semantics/check-omp-structure.h

private:
   void CheckIsVarPartOfAnotherVar(
       const parser::CharBlock &source, const parser::OmpObjectList &objList);
   void CheckThreadprivateOrDeclareTargetVar(
       const parser::OmpObjectList &objList);
   void CheckIntentInPointer(
       const parser::OmpObjectList &, const llvm::omp::Clause);
   void GetSymbolsInObjectList(const parser::OmpObjectList &, SymbolSourceMap &);
   void CheckDefinableObjects(SymbolSourceMap &, const llvm::omp::Clause);
+  void CheckCopyingPolymorphicAllocatable(
+      SymbolSourceMap &, const llvm::omp::Clause);
   void CheckPrivateSymbolsInOuterCxt(
       SymbolSourceMap &, DirectivesClauseTriple &, const llvm::omp::Clause);
   const parser::Name GetLoopIndex(const parser::DoConstruct *x);
   void SetLoopInfo(const parser::OpenMPLoopConstruct &x);
   void CheckIsLoopIvPartOfClause(
       llvmOmpClause clause, const parser::OmpObjectList &ompObjectList);
   bool CheckTargetBlockOnlyTeams(const parser::Block &);
   void CheckWorkshareBlockStmts(const parser::Block &, parser::CharBlock);

flang/lib/Semantics/check-omp-structure.cpp

 
 // Following clauses do not have a separate node in parse-tree.h.
 CHECK_SIMPLE_CLAUSE(AcqRel, OMPC_acq_rel)
 CHECK_SIMPLE_CLAUSE(Acquire, OMPC_acquire)
 CHECK_SIMPLE_CLAUSE(AtomicDefaultMemOrder, OMPC_atomic_default_mem_order)
 CHECK_SIMPLE_CLAUSE(Affinity, OMPC_affinity)
 CHECK_SIMPLE_CLAUSE(Allocate, OMPC_allocate)
 CHECK_SIMPLE_CLAUSE(Capture, OMPC_capture)
-CHECK_SIMPLE_CLAUSE(Copyin, OMPC_copyin)
 CHECK_SIMPLE_CLAUSE(Default, OMPC_default)
 CHECK_SIMPLE_CLAUSE(Depobj, OMPC_depobj)
 CHECK_SIMPLE_CLAUSE(Destroy, OMPC_destroy)
 CHECK_SIMPLE_CLAUSE(Detach, OMPC_detach)
 CHECK_SIMPLE_CLAUSE(DeviceType, OMPC_device_type)
 CHECK_SIMPLE_CLAUSE(DistSchedule, OMPC_dist_schedule)
 CHECK_SIMPLE_CLAUSE(DynamicAllocators, OMPC_dynamic_allocators)
 CHECK_SIMPLE_CLAUSE(Exclusive, OMPC_exclusive)

 }
 
 void OmpStructureChecker::Enter(const parser::OmpClause::Firstprivate &x) {
   CheckAllowed(llvm::omp::Clause::OMPC_firstprivate);
   CheckIsLoopIvPartOfClause(llvmOmpClause::OMPC_firstprivate, x.v);
 
   SymbolSourceMap currSymbols;
   GetSymbolsInObjectList(x.v, currSymbols);
+  CheckCopyingPolymorphicAllocatable(
+      currSymbols, llvm::omp::Clause::OMPC_firstprivate);
 
   DirectivesClauseTriple dirClauseTriple;
   // Check firstprivate variables in worksharing constructs
   dirClauseTriple.emplace(llvm::omp::Directive::OMPD_do,
       std::make_pair(
           llvm::omp::Directive::OMPD_parallel, llvm::omp::privateReductionSet));
   dirClauseTriple.emplace(llvm::omp::Directive::OMPD_sections,
       std::make_pair(
for (const auto &ele : designators) {
           CheckArraySection(arr->value(), GetLastName(*dataRef),
               llvm::omp::Clause::OMPC_depend);
         }
       }
     }
   }
 }
 
+void OmpStructureChecker::CheckCopyingPolymorphicAllocatable(
+    SymbolSourceMap &symbols, const llvm::omp::Clause clause) {
+  for (auto it{symbols.begin()}; it != symbols.end(); ++it) {
+    const auto *symbol{it->first};
+    const auto source{it->second};
+    if (IsPolymorphicAllocatable(*symbol)) {
+      context_.Say(source,
+          "If a polymorphic variable with allocatable attribute '%s' is in "
+          "%s clause, the behavior is unspecified"_port_en_US,
+          symbol->name(),
+          parser::ToUpperCaseLetters(getClauseName(clause).str()));
+    }
+  }
+}
+
 void OmpStructureChecker::Enter(const parser::OmpClause::Copyprivate &x) {
   CheckAllowed(llvm::omp::Clause::OMPC_copyprivate);
   CheckIntentInPointer(x.v, llvm::omp::Clause::OMPC_copyprivate);
+  SymbolSourceMap currSymbols;
+  GetSymbolsInObjectList(x.v, currSymbols);
+  CheckCopyingPolymorphicAllocatable(
+      currSymbols, llvm::omp::Clause::OMPC_copyprivate);
 }
 
 void OmpStructureChecker::Enter(const parser::OmpClause::Lastprivate &x) {
   CheckAllowed(llvm::omp::Clause::OMPC_lastprivate);
 
   DirectivesClauseTriple dirClauseTriple;
   SymbolSourceMap currSymbols;
   GetSymbolsInObjectList(x.v, currSymbols);
   CheckDefinableObjects(currSymbols, GetClauseKindForParserClass(x));
+  CheckCopyingPolymorphicAllocatable(
+      currSymbols, llvm::omp::Clause::OMPC_lastprivate);
 
   // Check lastprivate variables in worksharing constructs
   dirClauseTriple.emplace(llvm::omp::Directive::OMPD_do,
       std::make_pair(
           llvm::omp::Directive::OMPD_parallel, llvm::omp::privateReductionSet));
   dirClauseTriple.emplace(llvm::omp::Directive::OMPD_sections,
       std::make_pair(
           llvm::omp::Directive::OMPD_parallel, llvm::omp::privateReductionSet));
 
   CheckPrivateSymbolsInOuterCxt(
       currSymbols, dirClauseTriple, GetClauseKindForParserClass(x));
 }
 
+void OmpStructureChecker::Enter(const parser::OmpClause::Copyin &x) {
+  CheckAllowed(llvm::omp::Clause::OMPC_copyin);
+
+  SymbolSourceMap currSymbols;
+  GetSymbolsInObjectList(x.v, currSymbols);
+  CheckCopyingPolymorphicAllocatable(
+      currSymbols, llvm::omp::Clause::OMPC_copyin);
+}
+
 llvm::StringRef OmpStructureChecker::getClauseName(llvm::omp::Clause clause) {
   return llvm::omp::getOpenMPClauseName(clause);
 }
 
 llvm::StringRef OmpStructureChecker::getDirectiveName(
     llvm::omp::Directive directive) {
   return llvm::omp::getOpenMPDirectiveName(directive);
 }

flang/test/Semantics/OpenMP/omp-copying.f90

+! RUN: %flang_fc1 -fopenmp %s 2>&1 | FileCheck %s
+! OpenMP Version 5.0
+! 2.19.4.4 firstprivate Clause
+! 2.19.4.5 lastprivate Clause
+! 2.19.6.1 copyin Clause
+! 2.19.6.2 copyprivate Clause
+! If the list item is a polymorphic variable with the allocatable attribute,
+! the behavior is unspecified.
+
+subroutine firstprivate()
+  class(*), allocatable, save :: x
+
+  !CHECK: portability: If a polymorphic variable with allocatable attribute 'x' is in FIRSTPRIVATE clause, the behavior is unspecified
+  !$omp parallel firstprivate(x)
+    call sub()
+  !$omp end parallel
+
+end
+
+subroutine lastprivate()
+  class(*), allocatable, save :: x
+
+  !CHECK: portability: If a polymorphic variable with allocatable attribute 'x' is in LASTPRIVATE clause, the behavior is unspecified
+  !$omp do lastprivate(x)
+  do i = 1, 10
+    call sub()
+  enddo
+  !$omp end do
+
+end
+
+subroutine copyin()
+  class(*), allocatable, save :: x
+  !$omp threadprivate(x)
+
+  !CHECK: portability: If a polymorphic variable with allocatable attribute 'x' is in COPYIN clause, the behavior is unspecified
+  !$omp parallel copyin(x)
+    call sub()
+  !$omp end parallel
+
+end
+
+subroutine copyprivate()
+  class(*), allocatable, save :: x
+  !$omp threadprivate(x)
+
+  !CHECK: portability: If a polymorphic variable with allocatable attribute 'x' is in COPYPRIVATE clause, the behavior is unspecified
+  !$omp single copyprivate(x)
+    call sub()
+  !$omp end single
+
+end