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

[Tom Eccles via flang-commits]

================
@@ -570,13 +459,29 @@ bool ClauseProcessor::processInitializer(
               [&](const auto &expr) -> mlir::Value {
                 mlir::Value exprResult = fir::getBase(convertExprToValue(
                     loc, converter, initExpr, symMap, stmtCtx));
-                // Conversion can either give a value or a refrence to a value,
-                // we need to return the reduction type, so an optional load may
-                // be generated.
                 if (auto refType = llvm::dyn_cast<fir::ReferenceType>(
                         exprResult.getType()))
                   if (ompPrivVar.getType() == refType)
                     exprResult = fir::LoadOp::create(builder, loc, exprResult);
+
+                // For derived types in by-ref reductions, the init value
+                // must be stored into omp_priv explicitly.
+                // populateByRefInitAndCleanupRegions doesn't handle
----------------
tblah wrote:

Would it be possible to extend populateByRefInitAndCleanupRegions instead of doing things here?

It would be great to see some documentation on the callback API to explain exactly what is supposed to be done here vs in `populateByRefInitAndCleanupRegions`. It looks to me as though perhaps this is supposed to be used only for by-value and `populateByRefInitAndCleanupRegions` does byRef. This could be made a lot clearer.

I know the current state of the implementation wasn't your doing, but I think some comments/renaming would go a long way to making this patch easier to get right.

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