[PATCH] D106667: SROA: Enhance speculateSelectInstLoads

[Christudasan Devadasan via Phabricator via llvm-commits]

cdevadas added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/SROA.cpp:1342-1346
+    BitCastInst *BC = dyn_cast<BitCastInst>(U);
+    if (BC && BC->hasOneUse())
+      LI = dyn_cast<LoadInst>(*BC->user_begin());
+    else
+      LI = dyn_cast<LoadInst>(U);
----------------
lebedev.ri wrote:
> cdevadas wrote:
> > lebedev.ri wrote:
> > > cdevadas wrote:
> > > > arsenm wrote:
> > > > > I think other places that handle this use a user worklist and don't have the one use restriction for the bitcast (e.g.  PointerReplacer::collectUsers)
> > > > For the transformation to happen, If there are multiple users of Bitcast, they should all be load instructions. That will make the IR non-canonical.   The one-use restriction makes sense I guess.
> > > > That will make the IR non-canonical. The one-use restriction makes sense I guess.
> > > 
> > > Could you expand on that?
> > If there are multiple loads from the same addr (bitcasted ptr here), they would have been optimized into a unified load before reaching SROA. Otherwise, it makes the IR less canonical.
> >  
> So, should this loop only handle a single non-bitcasted load too?
> I don't see why we should allow an arbitrary number of loads and bitcasts, but only a single load of bitcast.
Yes. I think so.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106667/new/

https://reviews.llvm.org/D106667