[PATCH] D126455: [FuncSpec] Make the Function Specializer part of the IPSCCP pass.

[Momchil Velikov via Phabricator via llvm-commits]

chill added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/SCCP.cpp:237
   Solver.removeLatticeValueFor(&Inst);
-  Inst.eraseFromParent();
+  Inst.removeFromParent();
+  ToDelete.insert(&Inst);
----------------
chill wrote:
> labrinea wrote:
> > chill wrote:
> > > Is there a specific reason to remove the instruction from the block?
> > > If not, I'd suggest doing deletion in a single place, as opposed to spreading parts of it all over.
> > I am not entirely sure. I wanted to avoid revisiting this instruction accidentally in either of simplifyInstsInBlock(), solve(), or resolvedUndefsIn(). For simplifyInstsInBlock() I could skip the instruction if it's present in `ToDelete`. For the others I don't know what the consequences of revisitng would be. I need to run some tests first.
> I can't see why would anything go wrong if the instruction is revisited.
> Do we know if the instruction is safe to remove? It could be `SDiv`/`SRem` with a zero divisor.
> 
Actually, never mind, we're not replacing the instruction with a constant but with another instruction.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126455