[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");
+          }
+          // Name the op from the resolved symbol's scoped ultimate (name,
+          // owner) plus the per-type suffix, byte-identical to the
+          // directive/materializer side, via getScopedUserReductionName.
+          // Reductions of the same spelling in different scopes refer to
+          // distinct ops, and a named reduction listing several types binds the
+          // op for the variable's type instead of colliding on the first type's
+          // op. namingType is the canonical element type (storage wrappers
+          // stripped) so the by-ref suffix matches the directive.
+          std::string reductionName =
+              ReductionProcessor::getScopedUserReductionName(
+                  converter, *sym, namingType, isByRef);
+          mlir::ModuleOp module = builder.getModule();
+          auto existingDecl = module.lookupSymbol<OpType>(reductionName);
+          // The MLIR verifier does not type-check these ops, so this is the
+          // only guard against binding a missing or mismatched declaration
+          // (e.g. a named declare reduction that does not list the variable's
+          // type). Compare unwrapped element types against namingType, the type
+          // the op was named and created from.
+          if (!existingDecl ||
+              fir::unwrapRefType(existingDecl.getType()) !=
+                  fir::unwrapRefType(namingType) ||
+              isBoxedTrivialReduction) {
+            TODO(currentLocation,
+                 "OpenMP user-defined named reduction declaration was not "
+                 "materialized for the variable's type");
----------------
tblah wrote:

Is this really something left unimplemented or is it just a bug?

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