[Mlir-commits] [llvm] [mlir] [mlir][OpenMP] - Implement lowering from MLIR to LLVMIR for `private` clause on `target` constructs (PR #105471)

[Pranav Bhandarkar]

bhandarkar-pranav wrote:

> Care is taken to undo any changes to the MLIR performed during delayed privatization for ParallelOp. Here you are leaving lasting changes. I'm not sure whether it matters or not that an mlir translation changes the mlir. I don't think it would be good to add this unless there is already an existing precedent?

 I understand what you mean and ultimately I decided that it was perhaps ok to leave lasting changes because compilation from here on moves to LLVM IR. 
> 
> I'm worried that allocas already in the body region will now come after the branches. Reviewers for reduction insisted it was very important that allocas are concentrated in the entry block as much as possible (this is acceptable to me if some later LLVM pass is guaranteed to hoist the allocas back to the entry block).

I hadn't thought about this. However, there is a mechanism in the `OMPIRBuilder` that records functions that need their constant allocas hoisted. The `finalize` method of `OMPIRBuilder` then hoists such `alloca`s.

Anyway though, based on your suggestions, I have open a new PR to translate `private` on `omp.target`. https://github.com/llvm/llvm-project/pull/109668 Could you please take a look. It is a much simpler change than this PR. @ergawy  - FYI.


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