[flang-commits] [flang] [Flang][OpenMP] Fix crash and IR errors for user-defined reduction on allocatable variables (PR #186765)

[via flang-commits]

================
@@ -803,6 +804,185 @@ bool ReductionProcessor::processReductionArguments(
                          &redOperator.u)) {
         if (!ReductionProcessor::supportedIntrinsicProcReduction(
                 *reductionIntrinsic)) {
+          if (isByRef) {
+            // create a new declare_reduction for the boxed type, reusing
+            // the existing init and combiner
+            semantics::Symbol *sym = reductionIntrinsic->v.sym();
+            llvm::StringRef baseName = {sym->name().begin(),
+                                        sym->name().size()};
+            mlir::ModuleOp module = builder.getModule();
+            auto existingDecl = module.lookupSymbol<OpType>(baseName);
+            if (!existingDecl) {
+              TODO(currentLocation,
+                   "User-defined reductions on allocatable or pointer "
+                   "variables: cannot find base reduction declaration");
+            }
+
+            std::string byrefName = getReductionName(
+                baseName, builder.getKindMap(), redType, isByRef);
+
+            mlir::Region &existingInitRegion =
+                existingDecl.getInitializerRegion();
+            auto genInitValueCB =
+                [&existingInitRegion](fir::FirOpBuilder &builder,
+                                      mlir::Location loc, mlir::Type elemTy,
+                                      mlir::Value) -> mlir::Value {
+              // unwrap box type to get the scalar element type
+              mlir::Type scalarTy = unwrapSeqOrBoxedType(elemTy);
+              // find a constant-producing op in the existing init region
+              mlir::Operation *constOp = nullptr;
+              existingInitRegion.walk([&](mlir::Operation *op) {
----------------
MattPD wrote:

This silently initializes the private copy to the wrong value when the initializer is not a literal constant. The value is reconstructed by scanning the base init region for the first literal constant and cloning it, so it reproduces the base declaration only for a single-literal initializer. When the initializer reads `omp_orig`, or is an expression or a function call, no constant matches and the fallback substitutes silently: integer becomes `0`, and character returns an empty value, so the private is never stored and stays uninitialized.

You can see it by saving this to `t.f90`:

```fortran
program p
  integer, allocatable :: a
  integer :: i
  !$omp declare reduction (mymax : integer : omp_out = max(omp_out, omp_in)) &
  !$omp&   initializer (omp_priv = omp_orig)
  allocate(a); a = 0
  !$omp parallel do reduction(mymax : a)
  do i = 1, 4
    a = max(a, i)
  end do
end program
```

and running `flang -fc1 -emit-hlfir -fopenmp -fopenmp-version=45 t.f90 -o -`. The boxed `@mymax_byref_box_heap_i32` init stores `arith.constant 0`, while the scalar `@mymax` init loads and yields `omp_orig`. The non-allocatable version reduces correctly, so only the allocatable path diverges, and it compiles with no diagnostic.

Would it be more robust to clone the base declaration's init-region body (remapping its `omp_orig` argument to the loaded original element), or to emit a TODO when the initializer is not a single literal, rather than substitute a constant, zero, or empty value?

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