[Mlir-commits] [mlir] [MLIR][OpenMP] Support basic materialization for `omp.private` ops (PR #81715)

[Kareem Ergawy]

================
@@ -1000,6 +1000,26 @@ convertOmpWsLoop(Operation &opInst, llvm::IRBuilderBase &builder,
   return success();
 }
 
+/// Replace the region arguments of the parallel op (which correspond to private
+/// variables) with the actual private varibles they correspond to. This
+/// prepares the parallel op so that it matches what is expected by the
+/// OMPIRBuilder.
+static void prepareOmpParallelForPrivatization(omp::ParallelOp opInst) {
----------------
ergawy wrote:

>  and add some findPrivVars that checks for if(definedInCaller(Blocks, V)) and returns the variables?

I think that will not work. If you check the definition of [definedInCaller](https://github.com/llvm/llvm-project/blob/f740366fa68d3cfceda7efe2d573348253fbb1e9/llvm/lib/Transforms/Utils/CodeExtractor.cpp#L279), you will see that it returns `true` only if a value is defined by an instruction that belongs to a block that is not an element in the `Blocks` argument to the function.

So, if you do not use `findInputsOutputs` and instead directly create a `findPrivVars` that directly calls `if(definedInCaller(Block, V))` without iterating over all instructions in the outlined region as `findInputsOutputs` [does](https://github.com/llvm/llvm-project/blob/f740366fa68d3cfceda7efe2d573348253fbb1e9/llvm/lib/Transforms/Utils/CodeExtractor.cpp#L652), I think you will falsely detect too many values as being private.

Please let me know if I misunderstood your suggestion! :)

Also, what is the disadvantage of the current approach of pre-processing the `omp.parallel` op by remapping the block argument uses to the original SSA values that will be privatized?

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