[Mlir-commits] [flang] [mlir] [mlir][LLVMIR][OpenMP] Move reduction allocas before stores (PR #105683)

[Sergio Afonso]

================
@@ -593,22 +593,23 @@ convertOmpOrderedRegion(Operation &opInst, llvm::IRBuilderBase &builder,
 }
 
 /// Allocate space for privatized reduction variables.
+/// `deferredStores` contains information to create store operations which needs
+/// to be inserted after all allocas
 template <typename T>
-static LogicalResult
-allocReductionVars(T loop, ArrayRef<BlockArgument> reductionArgs,
-                   llvm::IRBuilderBase &builder,
-                   LLVM::ModuleTranslation &moduleTranslation,
-                   llvm::OpenMPIRBuilder::InsertPointTy &allocaIP,
-                   SmallVectorImpl<omp::DeclareReductionOp> &reductionDecls,
-                   SmallVectorImpl<llvm::Value *> &privateReductionVariables,
-                   DenseMap<Value, llvm::Value *> &reductionVariableMap,
-                   llvm::ArrayRef<bool> isByRefs) {
+static LogicalResult allocReductionVars(
+    T loop, ArrayRef<BlockArgument> reductionArgs, llvm::IRBuilderBase &builder,
+    LLVM::ModuleTranslation &moduleTranslation,
+    llvm::OpenMPIRBuilder::InsertPointTy &allocaIP,
+    SmallVectorImpl<omp::DeclareReductionOp> &reductionDecls,
+    SmallVectorImpl<llvm::Value *> &privateReductionVariables,
+    DenseMap<Value, llvm::Value *> &reductionVariableMap,
+    SmallVectorImpl<std::pair<llvm::Value *, llvm::Value *>> &deferredStores,
----------------
skatrak wrote:

Nit: Feel free to disagree, but I think it would help readability to define a simple struct to hold elements of this vector, so that we can refer to the `value` and `address` of the deferred store rather than some undocumented pair element. Mainly because now it crosses function boundaries, so keeping track of what's what is more difficult.

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