[PATCH] D77734: [AssumeBundles] Adapt RecursivelyDeleteTriviallyDeadInstructions and depending passes.

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:7990
 
-    SmallVector<WeakTrackingVH, 32> DeadInsts;
     SmallVector<Instruction *, 32> TerminatorsToFold;
----------------
This looks suspicious. Are we sure we don't invalidate an element in here and need the tracking behavior?


================
Comment at: llvm/lib/Transforms/Utils/AssumeBundleBuilder.cpp:139
+          if (!Call->getArgOperand(Idx - 1))
+            continue;
           addAttribute(Attr, Call->getArgOperand(Idx - 1));
----------------
Could you explain this check? When would a call return a nullptr of an arg operand? isn't there an assertion against that?


================
Comment at: llvm/lib/Transforms/Utils/AssumeBundleBuilder.cpp:210
+  }
+
   void addInstruction(Instruction *I) {
----------------
This doesn't sound right. null operands will explode everywhere, how could they happen?


================
Comment at: llvm/lib/Transforms/Utils/SimplifyCFG.cpp:221
+  bool SimplifyIndirectBrOnSelect(IndirectBrInst *IBI, SelectInst *SI);
+  bool TurnSwitchRangeIntoICmp(SwitchInst *SI, IRBuilder<> &Builder);
+
----------------
I guess this is an NFC rewrite? We should split it into a separate commit though, only this file, creating the members.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77734