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

Fri Jan 8 13:48:19 PST 2021

ABataev accepted this revision.
ABataev added a comment.
This revision is now accepted and ready to land.

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.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94315