Revert "[Flang][OpenMP] Disable lowering of omp.simd reductions in co…

[skatrak]

…mposites (#112686)"

Lowering of reductions in composite operations can now be re-enabled, since previous commits in this PR stack fix the MLIR representation produced and it no longer triggers a compiler crash during translation to LLVM IR.

This reverts commit c44860c.

[llvmbot]

@llvm/pr-subscribers-flang-openmp

@llvm/pr-subscribers-flang-fir-hlfir

Author: Sergio Afonso (skatrak)

Changes

…mposites (#112686)"

Lowering of reductions in composite operations can now be re-enabled, since previous commits in this PR stack fix the MLIR representation produced and it no longer triggers a compiler crash during translation to LLVM IR.

This reverts commit c44860c.

---

Full diff: https://github.com/llvm/llvm-project/pull/113683.diff

2 Files Affected:

• (modified) flang/lib/Lower/OpenMP/OpenMP.cpp (+6-14)
• (modified) flang/test/Lower/OpenMP/wsloop-simd.f90 (+21)

diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 149a7b9407b526..315a0bad7425a8 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -2237,12 +2237,6 @@ static void genCompositeDistributeParallelDoSimd(
   genSimdClauses(converter, semaCtx, simdItem->clauses, loc, simdClauseOps,
                  simdReductionSyms);
 
-  // TODO: Remove this after omp.simd reductions on composite constructs are
-  // supported.
-  simdClauseOps.reductionVars.clear();
-  simdClauseOps.reductionByref.clear();
-  simdClauseOps.reductionSyms.clear();
-
   mlir::omp::LoopNestOperands loopNestClauseOps;
   llvm::SmallVector<const semantics::Symbol *> iv;
   genLoopNestClauses(converter, semaCtx, eval, simdItem->clauses, loc,
@@ -2264,7 +2258,9 @@ static void genCompositeDistributeParallelDoSimd(
   wsloopOp.setComposite(/*val=*/true);
 
   EntryBlockArgs simdArgs;
-  // TODO: Add private and reduction syms and vars.
+  // TODO: Add private syms and vars.
+  simdArgs.reduction.syms = simdReductionSyms;
+  simdArgs.reduction.vars = simdClauseOps.reductionVars;
   auto simdOp =
       genWrapperOp<mlir::omp::SimdOp>(converter, loc, simdClauseOps, simdArgs);
   simdOp.setComposite(/*val=*/true);
@@ -2357,12 +2353,6 @@ static void genCompositeDoSimd(lower::AbstractConverter &converter,
   genSimdClauses(converter, semaCtx, simdItem->clauses, loc, simdClauseOps,
                  simdReductionSyms);
 
-  // TODO: Remove this after omp.simd reductions on composite constructs are
-  // supported.
-  simdClauseOps.reductionVars.clear();
-  simdClauseOps.reductionByref.clear();
-  simdClauseOps.reductionSyms.clear();
-
   // TODO: Support delayed privatization.
   DataSharingProcessor dsp(converter, semaCtx, simdItem->clauses, eval,
                            /*shouldCollectPreDeterminedSymbols=*/true,
@@ -2386,7 +2376,9 @@ static void genCompositeDoSimd(lower::AbstractConverter &converter,
   wsloopOp.setComposite(/*val=*/true);
 
   EntryBlockArgs simdArgs;
-  // TODO: Add private and reduction syms and vars.
+  // TODO: Add private syms and vars.
+  simdArgs.reduction.syms = simdReductionSyms;
+  simdArgs.reduction.vars = simdClauseOps.reductionVars;
   auto simdOp =
       genWrapperOp<mlir::omp::SimdOp>(converter, loc, simdClauseOps, simdArgs);
   simdOp.setComposite(/*val=*/true);
diff --git a/flang/test/Lower/OpenMP/wsloop-simd.f90 b/flang/test/Lower/OpenMP/wsloop-simd.f90
index 899ab59714f144..49a9a523e11fe7 100644
--- a/flang/test/Lower/OpenMP/wsloop-simd.f90
+++ b/flang/test/Lower/OpenMP/wsloop-simd.f90
@@ -45,3 +45,24 @@ subroutine do_simd_simdlen()
     end do
   !$omp end do simd
 end subroutine do_simd_simdlen
+
+! CHECK-LABEL: func.func @_QPdo_simd_reduction(
+subroutine do_simd_reduction()
+  integer :: sum
+  sum = 0
+  ! CHECK:      omp.wsloop
+  ! CHECK-SAME: reduction(@[[RED_SYM:.*]] %{{.*}} -> %[[RED_OUTER:.*]] : !fir.ref<i32>)
+  ! CHECK-NEXT: omp.simd
+  ! CHECK-SAME: reduction(@[[RED_SYM]] %[[RED_OUTER]] -> %[[RED_INNER:.*]] : !fir.ref<i32>)
+  ! CHECK-NEXT: omp.loop_nest
+  ! CHECK:      %[[RED_DECL:.*]]:2 = hlfir.declare %[[RED_INNER]]
+  ! CHECK:      %[[RED:.*]] = fir.load %[[RED_DECL]]#0 : !fir.ref<i32>
+  ! CHECK:      %[[RESULT:.*]] = arith.addi %[[RED]], %{{.*}} : i32
+  ! CHECK:      hlfir.assign %[[RESULT]] to %[[RED_DECL]]#0 : i32, !fir.ref<i32>
+  ! CHECK-NEXT: omp.yield
+  !$omp do simd reduction(+:sum)
+    do index_ = 1, 10
+      sum = sum + 1
+    end do
+  !$omp end do simd
+end subroutine do_simd_reduction

[skatrak]

PR stack:

• [MLIR][OpenMP] Prevent composite omp.simd related crashes #113680
• [Flang][OpenMP] Access full list of entry block syms and vars (NFC) #113681
• [Flang][OpenMP] Properly bind arguments of composite operations #113682
• Revert "[Flang][OpenMP] Disable lowering of omp.simd reductions in co… #113683

[NimishMishra]

This looks okay to me, given the PR stack.

There is still #113682 pending a merge; I'll take a look at that PR tomorrow. Thanks for the work on this.

[tblah]

Thanks!