[PATCH] D118387: [IPSCCP] Switch away from Instruction::isSafeToRemove()

[Kevin P. Neal via Phabricator via llvm-commits]

kpn added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/SCCP.cpp:102
+                                     const TargetLibraryInfo *TLI) {
+  if (wouldInstructionBeTriviallyDead(I, TLI))
+    return true;
----------------
nikic wrote:
> I'd drop the TLI pass through here. SCCP should primarily be removing instructions it has simplified itself, and that shouldn't need TLI.
I didn't realize it wasn't required. Dropping it is certainly much easier.


================
Comment at: llvm/lib/Transforms/Scalar/SCCP.cpp:109
+
+  return (!isa<CallInst>(I) || !I->mayHaveSideEffects()) && !I->isTerminator();
+}
----------------
nikic wrote:
> I don't understand why we need this remaining code -- why is wouldInstructionBeTriviallyDead not sufficient?
>From more study, it looks like you are correct and this bit isn't needed. Which implies that this whole function isn't needed and we can just call wouldInstructionBeTriviallyDead() directly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118387