[flang-commits] [flang] [flang][OpenMP] Add lowering of subroutine calls in custom reduction combiners (PR #169808)

[Kareem Ergawy via flang-commits]

================
@@ -3621,15 +3635,32 @@ processReductionCombiner(lower::AbstractConverter &converter,
       auto declareOp =
           hlfir::DeclareOp::create(builder, loc, addr, name.ToString(), nullptr,
                                    {}, nullptr, nullptr, 0, attributes);
+      if (name.ToString() == "omp_out")
+        ompOutVar = declareOp.getResult(0);
       symTable.addVariableDefinition(*name.symbol, declareOp);
     }
 
     lower::StatementContext stmtCtx;
-    mlir::Value result = fir::getBase(
-        convertExprToValue(loc, converter, evalExpr, symTable, stmtCtx));
-    if (auto refType = llvm::dyn_cast<fir::ReferenceType>(result.getType()))
-      if (lhs.getType() == refType.getElementType())
-        result = fir::LoadOp::create(builder, loc, result);
+    mlir::Value result = common::visit(
+        common::visitors{
+            [&](const evaluate::ProcedureRef &procRef) -> mlir::Value {
+              convertCallToHLFIR(loc, converter, procRef, std::nullopt,
+                                 symTable, stmtCtx);
+              auto outVal = fir::LoadOp::create(builder, loc, ompOutVar);
+              return outVal;
+            },
+            [&](const auto &expr) -> mlir::Value {
+              mlir::Value exprResult = fir::getBase(convertExprToValue(
+                  loc, converter, evalExpr, symTable, stmtCtx));
+              // Optional load may be generated if we get a reference to the
+              // reduction type.
+              if (auto refType =
+                      llvm::dyn_cast<fir::ReferenceType>(exprResult.getType()))
+                if (lhs.getType() == refType.getElementType())
+                  exprResult = fir::LoadOp::create(builder, loc, exprResult);
----------------
ergawy wrote:

IIRC this load was done in the MLIR to LLVM translation before, right (https://github.com/llvm/llvm-project/blob/main/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp#L1214)? Or is this something different?

If so, should the load in the MLIR to LLVM translation be removed? Or do you prefer to keep both?

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