[flang-commits] [flang] [flang][Lower] Make reduction processing failure a hard error (PR #150233)

[Tom Eccles via flang-commits]

https://github.com/tblah created https://github.com/llvm/llvm-project/pull/150233

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.

>From fc1b93bf0ad04ec4090019bca7ccdf04b5383b47 Mon Sep 17 00:00:00 2001
From: Tom Eccles <tom.eccles at arm.com>
Date: Wed, 23 Jul 2025 10:42:04 +0000
Subject: [PATCH] [flang][Lower] Make reduction processing failure a hard error

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.
---
 flang/lib/Lower/Bridge.cpp                               | 3 ++-
 flang/lib/Lower/OpenMP/ClauseProcessor.cpp               | 6 +++---
 .../OpenMP/{ => Todo}/wsloop-reduction-non-intrinsic.f90 | 9 ++-------
 3 files changed, 7 insertions(+), 11 deletions(-)
 rename flang/test/Lower/OpenMP/{ => Todo}/wsloop-reduction-non-intrinsic.f90 (52%)

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: }