[flang-commits] [flang] [flang][OpenMP] Lower multi-type user-defined declare reduction (PR #207494)

[Tom Eccles via flang-commits]

================
@@ -908,17 +949,47 @@ bool ReductionProcessor::processReductionArguments(
                          &redOperator.u)) {
         if (!ReductionProcessor::supportedIntrinsicProcReduction(
                 *reductionIntrinsic)) {
-          // Custom reductions we can just add to the symbols without
-          // generating the declare reduction op.
+          // A user-defined named reduction (declare reduction(myred: ...)). The
+          // clause references the (possibly USE-associated) reduction symbol;
+          // bind its pre-materialized omp.declare_reduction op instead of
+          // generating a new one.
           semantics::Symbol *sym = reductionIntrinsic->v.sym();
-          // Qualify the name with the scope in which the user-defined
-          // reduction was declared so that reductions with the same name in
-          // different scopes refer to distinct omp.declare_reduction ops.
-          std::string reductionName = getRealName(sym).ToString();
-          reductionName =
-              converter.mangleName(reductionName, sym->GetUltimate().owner());
-          reductionDeclSymbols.push_back(
-              mlir::SymbolRefAttr::get(builder.getContext(), reductionName));
+          if (!sym->GetUltimate()
+                   .detailsIf<semantics::UserReductionDetails>()) {
+            // Not a supported intrinsic proc (checked just above) and not a
+            // user-defined reduction: a clean TODO, never a wrong binding. (A
+            // named reduction reusing an intrinsic spelling such as `max` is
+            // handled by the supportedIntrinsicProcReduction branch, not here.)
+            TODO(currentLocation, "Lowering unrecognised reduction type");
----------------
tblah wrote:

Much better for this to be an error than a wrong binding. But I wonder if this is another case of something that shouldn't be reachable? In which case `fir::emitFatalError`? I might be misunderstanding this...

https://github.com/llvm/llvm-project/pull/207494