[flang-commits] [flang] [flang][OpenMP] Lower `target .. private(..)` to `omp.private` ops (PR #94195)

[Kiran Chandramohan via flang-commits]

kiranchandramohan wrote:

> I might have missed something, but target does not support the default clause, right?

Sorry, i completely missed that.

> You mean different types of variables; e.g. primitive, allocatable, pointer, array, etc? Or something else?

I specifically meant types here (integer, real, complex, character). But you can include arrays, allocatable etc. Just to ensure we are correctly handling the case when more than one type is present. I suggest also trying out multiple private clauses.

> > Could you add a few more tests? -> With a default private. -> With three different types of privates.
> > Did you get a chance to discuss with @mjklemm about how privatisation works for tasks and since target has some task like properties whether we need something more than inlining?
> 
> Yes, we'll need a way to send the privates via the target task to the target kernel so that `firstprivate` variables can be initialized. The way clang does this is by tacking on the private variables at the end of the `task_t` structure in the call to `kmpc_omp_task_alloc`. For now, this is a TODO item for both, [Tasks](https://github.com/llvm/llvm-project/blob/main/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp#L1790) and [Target](https://github.com/bhandarkar-pranav/llvm-project/blob/ompirbuilder/target_task_codegen/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp#L5529) in the `OpenMPIRBuilder`. These values can then be picked up by the inlined code (generated out of the privatizers by way of a `PrivCB` callback) in the target kernel.

What I am worried about is whether we want a different style entry in the PrivateClause Operation so that the firstprivate variable can be laid out properly in the privatisation part of the `task_t` structure.
Would it make sense to add a TODO/`Not Yet Implemented Error` for all target directives for firstprivate clauses that can be deferred?

Separately, you might need something like @luporl's patch for default firstprivate behaviour. https://github.com/llvm/llvm-project/pull/85989

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