[PATCH] D31539: Hoisting invariant.group in LICM

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy added a comment.

In https://reviews.llvm.org/D31539#716347, @Prazek 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)
>     store new vtable to ptr0
>     ptr2 = barrier(ptr0)
>     // ptr1 is not used, say
>   }
>
> Then you can clearly see that vtable.pre dominates vtable and it has the same pointer operand, so it means it has to load the same value.


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".


https://reviews.llvm.org/D31539