[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) {
+                if (constOp)
+                  return;
+                if (auto arithConst =
+                        mlir::dyn_cast<mlir::arith::ConstantOp>(op)) {
+                  if (arithConst.getType() == scalarTy)
+                    constOp = op;
+                  return;
+                }
+                if (mlir::isa<fir::StringLitOp>(op)) {
+                  constOp = op;
+                  return;
+                }
+                if (mlir::isa<fir::AddrOfOp>(op)) {
+                  constOp = op;
+                  return;
+                }
+              });
+
+              if (constOp) {
+                mlir::IRMapping mapper;
+                mlir::Value cloned =
+                    builder.clone(*constOp, mapper)->getResult(0);
+                // load if the cloned op produces a reference
+                if (fir::isa_ref_type(cloned.getType()))
+                  cloned = fir::LoadOp::create(builder, loc, cloned);
+                if (cloned.getType() != scalarTy)
+                  cloned = builder.createConvert(loc, scalarTy, cloned);
+                return cloned;
+              }
+              // Fallback: zero-initialize for trivial types.
+              if (fir::isa_integer(scalarTy))
+                return builder.createIntegerConstant(loc, scalarTy, 0);
+              if (mlir::isa<mlir::FloatType>(scalarTy))
+                return builder.createRealConstant(loc, scalarTy,
+                                                  llvm::APFloat(0.0));
----------------
MattPD wrote:

This fallback crashes the compiler for a `real` allocatable reduction whose initializer is not a literal constant. `llvm::APFloat(0.0)` is IEEEdouble, but `scalarTy` is `f32` for default `real`, so `createRealConstant` fails `FloatAttr` verification and aborts.

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

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

and running `flang -fc1 -emit-hlfir -fopenmp -fopenmp-version=45 t.f90`. It aborts with `error: FloatAttr type doesn't match the type implied by its value` inside `createRealConstant`. A constant initializer such as `omp_priv = 0.0` compiles, because the walk finds the `f32` constant and this fallback never runs.

Would building the zero with the target type's own semantics avoid it, for example the double overload `builder.createRealConstant(loc, scalarTy, 0.0)`, or `APFloat(cast<FloatType>(scalarTy).getFloatSemantics(), 0)`?

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