[PATCH] D138238: [SROA] For non-speculatable `load`s of `select`s -- split block, insert then/else blocks, form two-entry PHI node

[Michael Kitzan via Phabricator via llvm-commits]

mkitzan added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/SROA.cpp:1409
+  LoadInst *TL =
+      IRB.CreateAlignedLoad(LI.getType(), TV, LI.getAlign(),
+                            LI.getName() + ".sroa.speculate.load.true");
----------------
lebedev.ri wrote:
> lebedev.ri wrote:
> > mkitzan wrote:
> > > Our out-of-tree backend is asserting in this function call (particularly at `assert(cast<PointerType>(Ptr->getType())->isOpaqueOrPointeeTypeMatches(Ty))` which occurs while creating the load). I suspect this is due to the lost bitcasts, since when I add them back there's no assert.
> > > 
> > > The code I have adding the bitcasts back looks like the following:
> > > ```
> > >   IRB.SetInsertPoint(&LI);
> > > 
> > >   // OUT OF TREE
> > >   if (SI.hasOneUse()) {
> > >     if (BitCastInst *BC = dyn_cast<BitCastInst>(SI.user_back())) {
> > >       // assert(BC->user_back() == &LI); <- unclear if this is guaranteed
> > >       TV = IRB.CreateBitCast(TV, BC->getType(), TV->getName() + ".sroa.cast");
> > >       FV = IRB.CreateBitCast(FV, BC->getType(), FV->getName() + ".sroa.cast");
> > >     }
> > >   }
> > >   // END OUT OF TREE
> > > 
> > >   LoadInst *TL = ...
> > > ```
> > > 
> > > To be honest, I have not entirely analyzed this patch, so I'm not confident that the way I have added the bitcasts back is legal (especially now that there's no while loop on SI's uses...). Could you advise if this is legal/correct (to unblock us)? I can try to get an anonymized test case for this, so a full fix can be made.
> > We are literally days away from stopping accepting such fixes.
> > That being said, let me fix this one...
> 9f27f4536e19e93349b0662338408efe6d1cb2fd, but it is probably one of last ones of it's kind.
Thanks for the quick turn around 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138238