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

Fri Jan 8 10:05:00 PST 2021

jdoerfert added a comment.

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

> In D94315#2487150 <https://reviews.llvm.org/D94315#2487150>, @JonChesterfield wrote:
>
>> I'm guessing we're using the function boundary as a compiler barrier. That seems fragile in the face of improving cross-function optimisation.
>
> Looks like applying inaccessiblemem_or_argmemonly attribute to the OpenMP functions is way too optimistic since we still can access this (inaccessible) memory using other OpenMP functions. Not sure about the semantics of this attribute, though.

I don't understand where `inaccessiblemem_or_argmemonly` is coming from. This prevents us from inlining the outlined parallel region.

>> What is the invariant we want around entry to a data environment, which happens to be met by a function boundary?

The data environment in one function is the same. So, changes that take affect only in a new data environment will not manifest in the function.
Take:

  a = get_some_ICV_fixed_per_data_env()
  unknown();
  b = get_some_ICV_fixed_per_data_env();

We want to ensure `a == b`.

---

The problem with PR48686 could actually be solved differently as well. However, the property I describe above is generally useful.
It holds, as far as I can tell, except when we do the `if(0)` "optimization". IMHO, the if condition should be passed to the runtime
and handled there. There is little to no point in exposing the conditional in user land and exposing even more runtime calls.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94315