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

[Serge Pavlov via Phabricator via llvm-commits]

sepavloff added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/SCCP.cpp:94
 
+static bool canSCCPRemoveInstruction(Instruction *I) {
+  if (wouldInstructionBeTriviallyDead(I))
----------------
I would say that `SCCP` in the function name is excessive, because we are already in SCCP.


================
Comment at: llvm/lib/Transforms/Scalar/SCCP.cpp:98
+
+  return (!isa<CallInst>(I) || !I->mayHaveSideEffects()) && !I->isTerminator();
+}
----------------
kpn wrote:
> sepavloff wrote:
> > This check is almost the same as used by `Instruction::isSafeToRemove`, but the latter also checks for `isEHPad`. Why this check is omitted here? Would it be more consistent just to call `isSafeToRemove`?
> I believe you are correct and this check should not have been omitted.
> 
> I'm trying to eliminate isSafeToRemove() since the Instruction class is the wrong place to do this check, and like the summary says, I don't want another implementation of isInstructionTriviallyDead().
I wonder if you can just make
```
    return I->isSafeToRemove();
```
instead of copying the body of `isSafeToRemove()`?


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

https://reviews.llvm.org/D118387