[PATCH] D45151: [LICM] Hoisting invariant.group loads

Piotr Padlewski via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 14 15:40:10 PDT 2018


Prazek added a comment.

In https://reviews.llvm.org/D45151#1197581, @sanjoy wrote:

> Can we spec `!invariant.group` in a way that lets us always keep the metadata when hoisting?  Right now it isn't clear what happens if its contract is violated, i.e. what the behavior of this program is:
>
>   store i32 0, i32* %ptr, !invariant.group !0
>   %x = load i32, i32* %ptr, !invariant.group !0
>   store i32 1, i32* %ptr, !invariant.group !0
>   %y = load i32, i32* %ptr, !invariant.group !0
>
>
> though it seems like you're assuming the *load* has UB?  Can we instead say that the second store has UB?  That way we should be able to hoist the load instruction without dropping the metadata.


I talked with Sanjoy ofline, but for the record, consider:

  %ptr = alloc 
  if (false) { 
    %val = load %ptr, !invariant.group 
  } 
  store %ptr, 1, !invariant.group 
  load %ptr, !invariant.group

I think that the current LangRef says that the store with invariant.group must store the same value (and it could also be removed if there is dominating store/load with invariant.group),
but even without it, in this example we could forward undef to the second load based on the hoisted first load, if we would keep the !invariant.group md.

Sanjoy sugested that such load could return poison instead of undef.  I am not familiar with it, and I would expect that getting such change would probably take the same amount of time as getting this simple change (posted on April 1st),
so if there are no objections, I would like to put this one in the trunk.

@hfinkel, you had some objections. Can you take a look again?


Repository:
  rL LLVM

https://reviews.llvm.org/D45151





More information about the llvm-commits mailing list