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

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri 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");
----------------
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...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138238