[flang-commits] [flang] [llvm] [mlir] [mlir][OpenMP] - MLIR to LLVMIR translation support for delayed privatization of allocatables in `omp.target` ops (PR #116576)

[Sergio Afonso via flang-commits]

================
@@ -3824,6 +3857,19 @@ convertOmpTarget(Operation &opInst, llvm::IRBuilderBase &builder,
   bool isTargetDevice = ompBuilder->Config.isTargetDevice();
   auto parentFn = opInst.getParentOfType<LLVM::LLVMFuncOp>();
   auto &targetRegion = targetOp.getRegion();
+  // Holds the private vars that have been mapped along with
+  // the block argument that corresponds to the MapInfoOp
+  // corresponding to the private var in question.
+  // So, for instance
+  //
+  // %10 = omp.map.info var_ptr(%6#0 : !fir.ref<!fir.box<!fir.heap<i32>>>, ..)
+  // omp.target map_entries(%10 -> %arg0) private(@box.privatizer %6#0-> %arg1)
+  //
+  // Then, %10 has been created so that the descriptor can be used by the
+  // privatizer
+  // @box.privatizer on the device side. Here we'd record {%6#0, 0} in the
+  // mappedPrivateVars map.
+  llvm::DenseMap<Value, int> mappedPrivateVars;
----------------
skatrak wrote:

My comment was more about the implementation of this map specifically, rather than the design of the operation. `mappedPrivateVars` will only contain indices that point to valid block arguments of the operation, so it should be possible to insert the actual value instead. Then, within `findAssociatedValue()`, you wouldn't have to call `targetOp.getRegion().getArgument()` or even pass `targetOp` at all because it would be done here already.

I'm adding a suggestion block below to illustrate what I mean.

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