[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#685264, @mehdi_amini wrote:

> In https://reviews.llvm.org/D29092#685256, @hiraditya wrote:
>
> > In https://reviews.llvm.org/D29092#685046, @mehdi_amini wrote:
> >
> > > I'd add also that `isSafeToSpeculativelyExecute` may be conservative right now, but it should improve with https://reviews.llvm.org/D20116
> > >  So it is even unclear to me which cases GVN hoist would be able to catch that are not in the realm of  `isSafeToSpeculativelyExecute`.
> >
> >
> > Currently `isSafeToSpeculativelyExecute` returns false for Calls, stores. GVN Hoist will hoist them if it is legal to do so.
>
>
> Can you address everything of what I wrote please instead of repeating partially what I wrote? I don't feel we'll make progress here.


I'm sorry I was in the middle of replying you when your message came, that's why it was a partial reply.

With the link you shared, IIUC, that only addresses the call instructions and only when the attribute 'speculatable' is present. This will stop hoisting of call instructions which does not have that attribute even if it is legal to hoist them.

For example:

  int foo(int *i) {
    return *i;
  }
  
  bar() {
  int *ptr;
  if (cond)
    foo(ptr);
  else
    foo(ptr);
  }


https://reviews.llvm.org/D29092