[PATCH] D111237: [TypePromotion] Search from ZExt + PHI

[Sam Parker via Phabricator via llvm-commits]

samparker added a comment.

I'd be tempted to split this into two and handle the NFC + InstRemove fix in one and the (zext (phi)) in another, just in case the functional change needs reverting. Completely up to you though.

And thanks for doing this!



================
Comment at: llvm/lib/CodeGen/TypePromotion.cpp:128
+             SmallPtrSetImpl<Instruction *> &wrap,
+             SmallPtrSet<Instruction *, 4> &instsToRemove)
       : Ctx(C), PromotedWidth(Width), Visited(visited),
----------------
Use a SmallPtrSetImpl, like we do for `wrap`.


================
Comment at: llvm/lib/CodeGen/TypePromotion.cpp:181
   void getAnalysisUsage(AnalysisUsage &AU) const override {
+    AU.addRequired<LoopInfoWrapperPass>();
     AU.addRequired<TargetTransformInfoWrapperPass>();
----------------
We should also mark it as preserved: AU.addPreserved<LoopInfoWrapperPass>


================
Comment at: llvm/lib/CodeGen/TypePromotion.cpp:937
+  auto BBIsInLoop = [&](BasicBlock *BB) -> bool {
+      for (auto *L : LI)
+       if (L->contains(BB))
----------------
nit: indentation looks off here.


================
Comment at: llvm/lib/CodeGen/TypePromotion.cpp:954
+        EVT ZExtVT = TLI->getValueType(DL, I.getType());
+        Instruction *Phi = dyn_cast<Instruction>(I.getOperand(0));
+        MadeChange |= TryToPromote(Phi, ZExtVT.getFixedSizeInBits());
----------------
We know it's a phi, so use static_cast.


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

https://reviews.llvm.org/D111237