[PATCH] D75008: [InstCombine] DCE instructions earlier

[Nikita Popov via Phabricator via llvm-commits]

nikic marked 3 inline comments as done.
nikic added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:3418
+        LLVM_DEBUG(dbgs() << "IC: DCE: " << *I << '\n');
+        eraseInstFromFunction(*I);
+        ++NumDeadInst;
----------------
spatel wrote:
> Seems like we already have redundant lines between this and what is included within "eraseInstFromFunction()". Can we:
> 1. Remove the debug print.
> 2. Remove the set of MadeIRChange.
> 
> That could be a preliminary NFC cleanup for the existing call below this block too?
Done that in rG7da3b5e45c25 for the existing use, and here as well.


================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:3682-3687
+      ++NumDeadInst;
+      LLVM_DEBUG(dbgs() << "IC: DCE: " << *Inst << '\n');
+      salvageDebugInfoOrMarkUndef(*Inst);
+      Inst->eraseFromParent();
+      MadeIRChange = true;
+      continue;
----------------
spatel wrote:
> eraseInstFromFunction(*Inst) ?
We can't use `eraseInstFromFunction()` here, because it will also push operands to the worklist. As this is the code doing the initial worklist population, these would interfere.


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

https://reviews.llvm.org/D75008