llvm-project

e80604a6 - [flang][OpenMP] Support user-defined declare reduction with derived types (#184897)

[flang][OpenMP] Support user-defined declare reduction with derived types (#184897) Fix lowering of `!$omp declare reduction` for intrinsic operators applied to user-defined derived types (e.g., `+` on `type(t)`). Previously, this hit a TODO in `ReductionProcessor::getReductionInitValue` because the code tried to compute an init value for a non-predefined type, when it should instead use the initializer region from the `DeclareReductionOp`. This fixes the issue #176278: [Flang][OpenMP] Compilation error when type-list in declare reduction directive is derived type name. The root cause was a naming mismatch: `genOMP` for `OpenMPDeclareReductionConstruct` used a raw operator string (e.g., "Add") as the reduction name, while `processReductionArguments` at the use site computed a canonical name via `getReductionName` (e.g., "add_reduction_byref_rec__QFTt"). The `lookupSymbol` in `createDeclareReductionHelper` never found the already-created op, so it fell through to `createDeclareReduction` which called `getReductionInitValue` with the derived type and hit the TODO. The fix has three parts: 1. Consistent names: In `genOMP` for `OpenMPDeclareReductionConstruct`, compute the reduction name using the same `getReductionName` scheme that `processReductionArguments` uses, so both sites produce identical symbol names. For intrinsic operators, this maps through `ReductionIdentifier` to get the canonical name. For user-defined named reductions, the raw symbol name is used directly, matching the existing custom-reduction lookup path. 2. Reuse reduction: In `processReductionArguments`, when an intrinsic operator reduction is requested, check whether a user-defined declare reduction already exists under that canonical name before attempting to create a new one. If found, reuse it. This avoids calling `createDeclareReduction` (and thus `getReductionInitValue`) for types that have user-provided initializers. 3. Reference semantics: Change `doReductionByRef` to return true for derived types. Previously it returned false for both trivial and derived types, treating derived types as by-val. This is incorrect for user-defined combiners that operate on components via side-effects (e.g., `omp_out%x = omp_out%x + omp_in%x`): the combiner mutates `omp_out` in place and doesn't produce a whole-struct value, so `convertExprToValue` returns the component type (`i32`) rather than the struct type, causing a type mismatch in the `omp.yield`. By-ref is the correct model: the combiner stores into the lhs reference and yields it. The combiner callback in `processReductionCombiner` is also updated to handle the by-ref derived-type case: when the combiner result type doesn't match the element type (as happens with component-level assignments), the store is skipped since the assignment already wrote into omp_out as a side-effect, and only the lhs reference is yielded. Tests updates: - Update declare-reduction-intrinsic-op.f90 from a negative test (checking for the TODO error) to a positive test checking the generated MLIR. - Update omp-declare-reduction-derivedtype.f90 CHECK lines to match the reference semantics fix: the `declare_reduction` now has type `!fir.ref<...>` with a `byref_element_type` attribute, an alloc region, a two-argument init region, and a combiner that stores into the lhs and yields the reference. The function body checks for initme and mycombine are unchanged in substance but use literal type names instead of a regex capture to avoid greedy matching issues with nested angle brackets. Remaining work: declare reduction without an initializer clause is not yet supported. I plan to address that subsequently. Assisted-by: Claude Opus 4.6. Note: Relied on LLM (Claude Opus 4.6) to help navigate the Flang APIs and assist with the corresponding boilerplate code & tests updates; in particular: in order to get the aforementioned consistent naming, in `ReductionProcessor::getReductionName` I had to get rid of `parser::DefinedOperator::EnumToString` and instead introduce `getRedIdFromParserIntrOp` (which does the conversion manually; just to make sure I haven't missed anything: is there no existing conversion function? AFAICT, there is none, but I might've missed it). In any case, feedback welcome! --------- Co-authored-by: Matt P. Dziubinski <matt-p.dziubinski@hpe.com>