[Mlir-commits] [llvm] [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:

> Wouldn't it still be called for the original variable (%x here)? And at that point can't we get (%arg0) and do the appropriate processing?

No. To illustrate the difference, I created 2 commits (both will be reverted but just to clarify what is happening):

1. 363ae7f6afdd61f6f350eef7c38d325a0bec7170 where I still pre-proccess the op and also added logs to `OMPIRBuilder` to list the collected inputs. Here is the list of collected inputs:
```
>>>> collected input:   %tid.addr = alloca i32, align 4
>>>> collected input:   %zero.addr = alloca i32, align 4
>>>> collected input: ptr %0
>>>> calling privCB for: ptr %0
```

2. 3933d26d1a8a3cb645d6e82849e3d3bb5a4c2936 where I do not pre-proccess the op with the same logs as the above:
```
>>>> collected input:   %tid.addr = alloca i32, align 4
>>>> collected input:   %zero.addr = alloca i32, align 4
```

So the private variable is not detected as input if we don't do the pre-processing. And this actually matches my understanding of how the `CodeExtractor` works [when it collect inputs/outputs to the region](https://github.com/llvm/llvm-project/blob/main/llvm/lib/Transforms/Utils/CodeExtractor.cpp#L649).

I think we can modify the `CodeExtractor` to detect region operands as inputs but I think there will be other problems down the line.

Let me know if I missed anything.

(both test commits will be reverted before merging of course)

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