[PATCH] D94315: [OpenMP][FIX] Enforce a function boundary for a new data environment

Fri Jan 8 14:17:37 PST 2021

jdoerfert added a comment.

In D94315#2487809 <https://reviews.llvm.org/D94315#2487809>, @ABataev wrote:

> In D94315#2487643 <https://reviews.llvm.org/D94315#2487643>, @jdoerfert wrote:
>
>> What we should do, as we move to the OpenMPIRBuilder, is to use runtime interfaces that match OpenMP directives.
>> Here is the `omp parallel` one for the host: D94332 <https://reviews.llvm.org/D94332>
>> The device one should look the same, potentially after extending it, as we want to apply the same logic regardless of the device.
>> This will also make things like parallel region merging (@ggeorgakoudis) much easier.
>>
>> I would prefer to fix PR48686 with this patch though until we switch to a new interface. The part where I remove the fn argument
>> meddling is on it's own valuable. We should not (need to) overwrite the function attributes for performance reasons given that we
>> have not inserted them. The new `noinline` will establish the invariant D94332 <https://reviews.llvm.org/D94332> is going to have as well and which makes OpenMPOpt
>> much simpler.
>
> It would be better to teach OpenMPOpt about current limitations, because these changes may prevent some optimizations, but if this is hard to implement/something else, better to have a bugfix rather than doing something the user does not expect.

True, "some optimizations" are for now prevented on the `if(0)` code path. The idea is to apply these optimizations on the `if(1)` as well. It seems hardly useful to optimize the former through some special handling anyway, another reason for D94332 <https://reviews.llvm.org/D94332>.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94315/new/

https://reviews.llvm.org/D94315