[Openmp-commits] [openmp] [flang] [flang][OpenMP] Add support for `target ... private` (PR #78968)

[Kareem Ergawy via Openmp-commits]

ergawy wrote:

> What about having a proper representation of private on the target operation and delay the actual privatization to a later stage?
> 
> #75836


Thanks for the suggestion, I think that would be much better indeed. To disuss this in more detail, for the `target` op, I think it should change from this:
```
omp.target ... {
bb0(...):
    %priv_x = fir.alloca ... {bindc_name = "x"} ... // alloca for privatized var
    ... use %priv_x ...
 }
```
To something like this:
```
omp.target ... private(%x -> %argx: type) {
bb0(%argx: type):
    ... use %argx ...
}
```

And then we either:
1. Would move/adapt the alloca generation logic currently in `DataSharingProcessor::privatize()` to `OpenMPToLLVMIRTranslation.cpp`. However, we need to adapt that logic to directly generate LLVM IR `alloca` instructions instead of `fir.alloca`s.
2. Or, have a conversion pattern that gets inserted in the pipeline while we still have FIR (*). The advantage here is that, I think, we can more directly and easily share code between that `DataSharingProcessor::privatize()` and that new conversion pattern. Or even reuse the `DataSharingProcessor` util altogether somehow. What I mean is that I think the `DataSharingProcessor` can be grown into a standalone `OpConversionPattern` that does the whole privitatization logic as later in the dialect as we would like.

I am leaning towards the 2nd option since it would be cleaner and easier to adapt IMO. WDYT?

(*) I know that emitting `fir.alloca`s for privatized variables may not be a good thing to have since it couples the FIR and OMP dialects but having a separate conversion pattern will a step towards uncoupling the 2 dialects (if we prefer to). Even if we do not care about the coupling, the separate conversion patter would be a cleaner and standalone way to handle all the privatization materializations in one self-contained place.

Looking forward for any thoughts on this :).

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