[Flang][OpenMP] Restrict check to worksharing construct reductions

[kiranchandramohan]

Summary

The outer context private check for reduction variables was firing
for all constructs. This check is not applicable to non-worksharing
constructs.

OpenMP 5.2: Section 5.5.8
A list item that appears in a reduction clause on a worksharing construct
must be shared in the parallel region to which a correspodning worksharing
region binds.

[luporl]

I'm hitting this assert while trying to test this patch:

$ ~/git/llvm-project2/buildr/bin/flang-new -fopenmp -c -o ftest.o ftest.f90
flang-new: /home/leandro.lupori/git/llvm-project2/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:1033: llvm::OpenMPIRBuilder::createParallel(const llvm::OpenMPIRBuilder::LocationDescription&, llvm::OpenMPIRBuilder::InsertPointTy, llvm::OpenMPIRBuilder::BodyGenCallbackTy, llvm::OpenMPIRBuilder::PrivatizeCallbackTy, llvm::OpenMPIRBuilder::FinalizeCallbackTy, llvm::Value*, llvm::Value*, llvm::omp::ProcBindKind, bool)::<lambda(llvm::Function&)>: Assertion `OutlinedFn.arg_size() >= 2 && "Expected at least tid and bounded tid as arguments"' failed.
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:
0.	Program arguments: /home/leandro.lupori/git/llvm-project2/buildr/bin/flang-new -fc1 -triple aarch64-unknown-linux-gnu -emit-obj -fopenmp -fcolor-diagnostics -mrelocation-model pic -pic-level 2 -pic-is-pie -target-cpu generic -target-feature +neon -target-feature +v8a -o ftest.o -x f95-cpp-input ftest.f90
 #0 0x0000aaaae11012b8 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/home/leandro.lupori/git/llvm-project2/buildr/bin/flang-new+0x32922b8)
 #1 0x0000aaaae10ff1e8 llvm::sys::RunSignalHandlers() (/home/leandro.lupori/git/llvm-project2/buildr/bin/flang-new+0x32901e8)
 #2 0x0000aaaae10ff384 SignalHandler(int) Signals.cpp:0:0
 #3 0x0000ffffaa0a65c0 (linux-vdso.so.1+0x5c0)
 #4 0x0000ffffa9bd3d78 raise /build/glibc-RIFKjK/glibc-2.31/signal/../sysdeps/unix/sysv/linux/raise.c:51:1
 #5 0x0000ffffa9bc0aac abort /build/glibc-RIFKjK/glibc-2.31/stdlib/abort.c:81:7
 #6 0x0000ffffa9bcd490 __assert_fail_base /build/glibc-RIFKjK/glibc-2.31/assert/assert.c:89:7
 #7 0x0000ffffa9bcd4f4 (/lib/aarch64-linux-gnu/libc.so.6+0x2d4f4)
 #8 0x0000aaaae3f960bc llvm::OpenMPIRBuilder::createParallel(llvm::OpenMPIRBuilder::LocationDescription const&, llvm::IRBuilderBase::InsertPoint, llvm::function_ref<void (llvm::IRBuilderBase::InsertPoint, llvm::IRBuilderBase::InsertPoint)>, llvm::function_ref<llvm::IRBuilderBase::InsertPoint (llvm::IRBuilderBase::InsertPoint, llvm::IRBuilderBase::InsertPoint, llvm::Value&, llvm::Value&, llvm::Value*&)>, std::function<void (llvm::IRBuilderBase::InsertPoint)>, llvm::Value*, llvm::Value*, llvm::omp::ProcBindKind, bool)::'lambda0'(llvm::Function&)::operator()(llvm::Function&) const OMPIRBuilder.cpp:0:0
 #9 0x0000aaaae3fb3140 llvm::OpenMPIRBuilder::finalize(llvm::Function*) (/home/leandro.lupori/git/llvm-project2/buildr/bin/flang-new+0x6144140)
#10 0x0000aaaae2c363a4 mlir::LLVM::ModuleTranslation::~ModuleTranslation() (/home/leandro.lupori/git/llvm-project2/buildr/bin/flang-new+0x4dc73a4)
#11 0x0000aaaae2c459a4 mlir::translateModuleToLLVMIR(mlir::Operation*, llvm::LLVMContext&, llvm::StringRef) (/home/leandro.lupori/git/llvm-project2/buildr/bin/flang-new+0x4dd69a4)
#12 0x0000aaaae113558c Fortran::frontend::CodeGenAction::generateLLVMIR() (/home/leandro.lupori/git/llvm-project2/buildr/bin/flang-new+0x32c658c)
#13 0x0000aaaae11f6dc0 Fortran::frontend::CodeGenAction::executeAction() (/home/leandro.lupori/git/llvm-project2/buildr/bin/flang-new+0x3387dc0)
#14 0x0000aaaae1125904 Fortran::frontend::FrontendAction::execute() (/home/leandro.lupori/git/llvm-project2/buildr/bin/flang-new+0x32b6904)
#15 0x0000aaaae1118b2c Fortran::frontend::CompilerInstance::executeAction(Fortran::frontend::FrontendAction&) (/home/leandro.lupori/git/llvm-project2/buildr/bin/flang-new+0x32a9b2c)
#16 0x0000aaaae112a434 Fortran::frontend::executeCompilerInvocation(Fortran::frontend::CompilerInstance*) (/home/leandro.lupori/git/llvm-project2/buildr/bin/flang-new+0x32bb434)
#17 0x0000aaaae0d5c89c fc1_main(llvm::ArrayRef<char const*>, char const*) (/home/leandro.lupori/git/llvm-project2/buildr/bin/flang-new+0x2eed89c)
#18 0x0000aaaae0d5c3c0 main (/home/leandro.lupori/git/llvm-project2/buildr/bin/flang-new+0x2eed3c0)
#19 0x0000ffffa9bc0e10 __libc_start_main /build/glibc-RIFKjK/glibc-2.31/csu/../csu/libc-start.c:342:3
#20 0x0000aaaae0d59654 _start (/home/leandro.lupori/git/llvm-project2/buildr/bin/flang-new+0x2eea654)
flang-new: error: unable to execute command: Aborted (core dumped)
flang-new: error: flang frontend command failed due to signal (use -v to see invocation)
flang-new version 17.0.0 (https://github.com/llvm/llvm-project.git 964997a143cfa0356d1cd21eb3e6d2170c975c38)
Target: aarch64-unknown-linux-gnu
Thread model: posix
InstalledDir: /home/leandro.lupori/git/llvm-project2/buildr/bin

I'm using this test program:

program p1
  implicit none
  integer :: a, i

  a = 0
  !$omp task firstprivate(a)
  !$omp parallel do reduction(+:a)
  do i = 1, 10
    a = a + i
  end do
  !$omp end parallel do
  !$omp end task
end program

[kiranchandramohan]

Thanks @luporl for having a look. There are a few fixes that are going in the privatisation area. I think the issue you are pointing out with your example will also be fixed by that. This fix is not causing the issue that you are seeing.

I have removed the Fixes tag and wait for resolving all the known issues before closing.

[peixin]

LGTM

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

private:
   void CheckBarrierNesting(const parser::OpenMPSimpleStandaloneConstruct &x);
   void ChecksOnOrderedAsStandalone();
   void CheckOrderedDependClause(std::optional<std::int64_t> orderedValue);
   void CheckReductionArraySection(const parser::OmpObjectList &ompObjectList);
   void CheckIntentInPointerAndDefinable(
       const parser::OmpObjectList &, const llvm::omp::Clause);
   void CheckArraySection(const parser::ArrayElement &arrayElement,
       const parser::Name &name, const llvm::omp::Clause clause);
-  void CheckMultipleAppearanceAcrossContext(
+  void CheckSharedBindingInOuterContext(
       const parser::OmpObjectList &ompObjectList);
   const parser::OmpObjectList *GetOmpObjectList(const parser::OmpClause &);
   void CheckPredefinedAllocatorRestriction(const parser::CharBlock &source,
       const parser::OmpObjectList &ompObjectList);
   void CheckPredefinedAllocatorRestriction(
       const parser::CharBlock &source, const parser::Name &name);
   bool isPredefinedAllocator{false};
   void EnterDirectiveNest(const int index) { directiveNest_[index]++; }

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

 }
 
 void OmpStructureChecker::CheckReductionTypeList(
     const parser::OmpClause::Reduction &x) {
   const auto &ompObjectList{std::get<parser::OmpObjectList>(x.v.t)};
   CheckIntentInPointerAndDefinable(
       ompObjectList, llvm::omp::Clause::OMPC_reduction);
   CheckReductionArraySection(ompObjectList);
-  CheckMultipleAppearanceAcrossContext(ompObjectList);
+  // If this is a worksharing construct then ensure the reduction variable
+  // is not private in the parallel region that it binds to.
+  OmpDirectiveSet workshareSet{llvm::omp::Directive::OMPD_do,
+      llvm::omp::Directive::OMPD_sections, llvm::omp::Directive::OMPD_do_simd};
+  if (workshareSet.test(GetContext().directive)) {
+    CheckSharedBindingInOuterContext(ompObjectList);
+  }
 }
 
 void OmpStructureChecker::CheckIntentInPointerAndDefinable(
     const parser::OmpObjectList &objectList, const llvm::omp::Clause clause) {
   for (const auto &ompObject : objectList.v) {
     if (const auto *name{parser::Unwrap<parser::Name>(ompObject)}) {
       if (const auto *symbol{name->symbol}) {
         if (IsPointer(symbol->GetUltimate()) &&
if (const auto *dataRef{parser::Unwrap<parser::DataRef>(ompObject)}) {
           CheckArraySection(*arrayElement, GetLastName(*dataRef),
               llvm::omp::Clause::OMPC_reduction);
         }
       }
     }
   }
 }
 
-void OmpStructureChecker::CheckMultipleAppearanceAcrossContext(
+void OmpStructureChecker::CheckSharedBindingInOuterContext(
     const parser::OmpObjectList &redObjectList) {
   //  TODO: Verify the assumption here that the immediately enclosing region is
   //  the parallel region to which the worksharing construct having reduction
   //  binds to.
   if (auto *enclosingContext{GetEnclosingDirContext()}) {
     for (auto it : enclosingContext->clauseInfo) {
       llvmOmpClause type = it.first;
       const auto *clause = it.second;

flang/test/Semantics/OpenMP/reduction07.f90

 !$omp sections reduction(*:a)
 a = a + 10
 !$omp end sections
 !$omp end parallel
 
 !$omp parallel reduction(*:a)
 !$omp end parallel
 
-
-!$omp parallel reduction(+:a)
-!ERROR: REDUCTION clause is not allowed on the WORKSHARE directive
-!ERROR: REDUCTION variable 'a' is REDUCTION in outer context must be shared in the parallel regions to which any of the worksharing regions arising from the worksharing construct bind.
-!$omp workshare reduction(*:a)
-a = a + 10
-!$omp end workshare
-!$omp end parallel
-
-!$omp parallel reduction(*:a)
-!$omp end parallel
-
-
-!$omp parallel reduction(+:a)
-!ERROR: REDUCTION clause is not allowed on the SINGLE directive
-!ERROR: REDUCTION variable 'a' is REDUCTION in outer context must be shared in the parallel regions to which any of the worksharing regions arising from the worksharing construct bind.
-!$omp single reduction(*:a)
-a = a + 10
-!$omp end single
-!$omp end parallel
-
-!$omp parallel reduction(+:a)
-!$omp end parallel
-
-
-!$omp parallel reduction(+:a)
-!ERROR: REDUCTION clause is not allowed on the SINGLE directive
-!ERROR: REDUCTION variable 'a' is REDUCTION in outer context must be shared in the parallel regions to which any of the worksharing regions arising from the worksharing construct bind.
-!$omp single reduction(iand:a)
-a = a + 10
-!$omp end single
-!$omp end parallel
-
-!$omp parallel reduction(iand:a)
-!$omp end parallel
-
 !$omp parallel reduction(ieor:a)
 !ERROR: REDUCTION variable 'a' is REDUCTION in outer context must be shared in the parallel regions to which any of the worksharing regions arising from the worksharing construct bind.
 !$omp sections reduction(+:a)
 a = ieor(a, 10)
 !$omp end sections
 !$omp end parallel
 
+!$omp parallel private(a)
 !$omp parallel reduction(ieor:a)
 !$omp end parallel
+!$omp end parallel
+
+!$omp task firstprivate(a)
+!$omp parallel do reduction(+:a)
+do i=1,10
+  a=a+j
+end do
+!$omp end parallel do
+!$omp end task
 
 end program omp_reduction