[PATCH] D31539: Hoisting invariant.group in LICM

[Piotr Padlewski via Phabricator via llvm-commits]

Prazek added a comment.

In https://reviews.llvm.org/D31539#716384, @sanjoy wrote:

> > It would, but that would mean that either the !invariant.group metadata is invalid there, or the store is storing the same value as it is loaded (so it would probably have !invariant.group
> >  there, but it doesn't matter).
> >  If I would unroll this loop:
> > 
> >   vtable.pre = load ptr0, !invariant.group
> >   use(vtable.pre)
> >   store new vtable to ptr0
> >   ptr1 = barrier(ptr0)
> >   for (;;) {
> >     vtable = load ptr0, !invariant.group
> >     use(vtable)
>
> Does this reasoning still hold if the loop has just one iteration?  That is, say the loop was
>
>   for (i = 0; i < 1; i++) {
>     vtable = load ptr0, !invariant.group
>     use(vtable)
>     store new vtable to ptr0  // S0
>     ptr1 = barrier(ptr0)
>     // ptr1 is not used, say
>   }
>
>
> then (I claim that) the program does not have UB even if `S0` is storing a new vtable.  I think this is the kind of IR we'll get from
>
>   for (i = 0; i < 1; i++) {
>     storage->f();
>     storage->~Foo();
>     new(storage) Bar;
>   }
>
>
>
>
> > Or maybe you are refering to the fact that invariant.group metadata would be preserved? If this is a case, then see my post on mailing list :)
>
> I thought your mailing list post was about //speculating// loads.  Here we're not speculating anything -- we're only sinking.  In any case, you do not need the `!invariant.group` metadata on the sunk load for the example above to "work".


So it looks like we can't sink based on invariant.group, but I guess hoisting is still possible, because
it would be only invalid if there would be a store before the load

for (i = 0; i < 1; i++) {

  store new vtable to ptr0  // S0  
  vtable = load ptr0, !invariant.group
  use(vtable)

}

but then if store stores different value, that means it is UB. Is this right?


https://reviews.llvm.org/D31539