[PATCH] D29092: PR31729: [GVNHoist] Don't hoist unsafe scalars at -Oz

[Aditya Kumar via Phabricator via llvm-commits]

hiraditya added a comment.

In https://reviews.llvm.org/D29092#685226, @MatzeB wrote:

> In https://reviews.llvm.org/D29092#685217, @hiraditya wrote:
>
> > In https://reviews.llvm.org/D29092#685017, @majnemer wrote:
> >
> > > In https://reviews.llvm.org/D29092#682961, @hans wrote:
> > >
> > > > David, do you have any further concerns? Otherwise I'd like to see this committed so we can resolve the PR for 4.0.
> > >
> > >
> > > I don't understand why we cannot assert isSafeToSpeculativelyExecute if we are not in minsize. If we are given a call instruction, why do we think it is safe to hoist it?
> >
> >
> > A call without any side effects, for example, can be hoisted.
>
>
> So if you have a readnone function with a division inside that could potentially fail, this will work correctly and not hoist it?


It will not hoist because the execution of function has to be present on all path from the hoisting point to all the leaf BasicBlocks. For example

  int foo(int i, int j) {
  return i/j;
  }
  
  if (j != 0)
    foo(i, j)

will not get hoisted because the function foo is present only on one patch from hoisting point (one of the common dominators of if-block) to the leaf basic blocks. It will get hoisted in this case however:

  int foo(int i, int j) {
  return i/j;
  }
  
  if (j != 0)
    foo(i, j);
  else
    foo(i, j)

But in this case the original code is going to fail as well. Please correct me if I'm missing something.


https://reviews.llvm.org/D29092