[flang][Lower] Make reduction processing failure a hard error

[tblah]

See #150178

This may regress some test cases which only ever passed by accident.

I've tested SPEC2017 and a sample of applications to check that this doesn't break anything too obvious. Presumably this was not a widely used feature or we would have noticed the bug sooner.

I'm unsure whether this should be backported to LLVM 21 or not: I think it is much better to refuse to compile than to silently produce the wrong result, but there is a chance this could regress something which previously worked by accident. Opinions welcome.

[llvmbot]

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

@llvm/pr-subscribers-flang-openmp

Author: Tom Eccles (tblah)

Changes

See #150178

This may regress some test cases which only ever passed by accident.

I've tested SPEC2017 and a sample of applications to check that this doesn't break anything too obvious. Presumably this was not a widely used feature or we would have noticed the bug sooner.

I'm unsure whether this should be backported to LLVM 21 or not: I think it is much better to refuse to compile than to silently produce the wrong result, but there is a chance this could regress something which previously worked by accident. Opinions welcome.

---

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

3 Files Affected:

• (modified) flang/lib/Lower/Bridge.cpp (+2-1)
• (modified) flang/lib/Lower/OpenMP/ClauseProcessor.cpp (+3-3)
• (renamed) flang/test/Lower/OpenMP/Todo/wsloop-reduction-non-intrinsic.f90 (+2-7)

diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index 13420f365919d..dc1e131b64126 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -2128,7 +2128,8 @@ class FirConverter : public Fortran::lower::AbstractConverter {
     bool result = rp.processReductionArguments<fir::DeclareReductionOp>(
         toLocation(), *this, info.reduceOperatorList, reduceVars,
         reduceVarByRef, reductionDeclSymbols, info.reduceSymList);
-    assert(result && "Failed to process `do concurrent` reductions");
+    if (!result)
+      TODO(toLocation(), "Lowering unrecognised reduction type");
 
     doConcurrentLoopOp.getReduceVarsMutable().assign(reduceVars);
     doConcurrentLoopOp.setReduceSymsAttr(
diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
index 594f95ecdda63..b83ea6f175031 100644
--- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
@@ -1121,7 +1121,7 @@ bool ClauseProcessor::processInReduction(
                 std::get<typename omp::clause::ReductionOperatorList>(clause.t),
                 inReductionVars, inReduceVarByRef, inReductionDeclSymbols,
                 inReductionSyms))
-          inReductionSyms.clear();
+          TODO(currentLocation, "Lowering unrecognised reduction type");
 
         // Copy local lists into the output.
         llvm::copy(inReductionVars, std::back_inserter(result.inReductionVars));
@@ -1467,7 +1467,7 @@ bool ClauseProcessor::processReduction(
                 std::get<typename omp::clause::ReductionOperatorList>(clause.t),
                 reductionVars, reduceVarByRef, reductionDeclSymbols,
                 reductionSyms))
-          reductionSyms.clear();
+          TODO(currentLocation, "Lowering unrecognised reduction type");
         // Copy local lists into the output.
         llvm::copy(reductionVars, std::back_inserter(result.reductionVars));
         llvm::copy(reduceVarByRef, std::back_inserter(result.reductionByref));
@@ -1494,7 +1494,7 @@ bool ClauseProcessor::processTaskReduction(
                 std::get<typename omp::clause::ReductionOperatorList>(clause.t),
                 taskReductionVars, taskReduceVarByRef, taskReductionDeclSymbols,
                 taskReductionSyms))
-          taskReductionSyms.clear();
+          TODO(currentLocation, "Lowering unrecognised reduction type");
         // Copy local lists into the output.
         llvm::copy(taskReductionVars,
                    std::back_inserter(result.taskReductionVars));
diff --git a/flang/test/Lower/OpenMP/wsloop-reduction-non-intrinsic.f90 b/flang/test/Lower/OpenMP/Todo/wsloop-reduction-non-intrinsic.f90
similarity index 52%
rename from flang/test/Lower/OpenMP/wsloop-reduction-non-intrinsic.f90
rename to flang/test/Lower/OpenMP/Todo/wsloop-reduction-non-intrinsic.f90
index 70b2ae1111a50..84feb5aeeee7c 100644
--- a/flang/test/Lower/OpenMP/wsloop-reduction-non-intrinsic.f90
+++ b/flang/test/Lower/OpenMP/Todo/wsloop-reduction-non-intrinsic.f90
@@ -1,6 +1,6 @@
 ! Tests reduction processor behavior when a reduction symbol is not supported.
 
-! RUN: %flang_fc1 -emit-hlfir -fopenmp -o - %s 2>&1 | FileCheck %s
+! RUN: %not_todo_cmd %flang_fc1 -emit-hlfir -fopenmp -o - %s 2>&1 | FileCheck %s
 
 subroutine foo
   implicit none
@@ -12,14 +12,9 @@ function max(m1,m2)
     end function
   end interface
 
+  !CHECK: not yet implemented: Lowering unrecognised reduction type
   !$omp do reduction (max: k)
   do i=1,10
   end do
   !$omp end do
 end
-
-! Verify that unsupported reduction is ignored.
-! CHECK: omp.wsloop 
-! CHECK-SAME: private(@{{[^[:space:]]+}} %{{[^[:space:]]+}}
-! CHECK-SAME:         -> %{{[^[:space:]]+}} : !{{[^[:space:]]+}}) {
-! CHECK: }

[tblah]

Ping

[anchuraj]

Thank you! It makes sense to turn them to a hard error than clearing to me.

[skatrak]

LGTM, thanks!