[flang-commits] [flang] [OpenMP][Flang] Add "IsolatedFromAbove" and "OutlineableOpenMPOpInterface" trait to omp.target (PR #67164)

[via flang-commits]

agozillon wrote:

> > I think there should be both map and block arguments, former to make lowering easier and carry map information for the captures, whilst making the mapping more explicit to readers of the IR and the latter to fulfil the IsolatedFromAbove interface, I think this patch does that currently. Perhaps sometime in the future the BlockArgs and Map clauses can be aligned so we only have one, not too sure how you'd go about doing that though, perhaps make an extended BlockArgs operation(?) for Map or a map clause interface for BlockArgs.
> 
> Not clear whether you are suggesting changes for this patch or explaining what is being done here. 

Latter half is for possible future changes if we wanted to align both block arguments and map clauses into one, not for alterations to this patch or any explanation of what's being done here. Former is advocating for what I believe is in this patch currently.

> Wouldn't the ones that are block arguments be subject to implicit data-mapping rules (https://www.openmp.org/spec-html/5.2/openmpsu60.html)?  

I think both implicit and explicit arguments need to be block arguments for IsolatedFromAbove to work, so I think both explicit and implicit map clauses are also block arguments currently @TIFitis can correct me if I'm wrong. But yes, the map clauses that are mapped as implicit via the map type flag in this patch would fall under the implicit data mapping rules, but as long as we can distinguish them from explicitly mapped variables it should be possible to lower them as specified (perhaps some extensions may be required in the future for specific rules like Fortran pointer/allocatable/target attributes on a variable).

> Would it be better to make all of them block arguments and use an attribute in the `MapInfo` operation to distinguish the implicit ones?

I think this is how it should be done and what is being done in this patch if I am understanding what you mean correctly and correctly understanding the patch itself. I believe a map clause and a block argument are generated for each implicit and explicitly captured argument to the target, and I think it is the correct or at least simplest way to do it at the moment. I believe the only difference is @TIFitis opted for the map type flag distinguishing if something is an implicit or not rather than using the attribute on the operation, which I think is fine. Perhaps I am incorrect in my understanding though.  

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