[PATCH] D81756: [SCCP] Turn sext into zext for positive ranges.

[Florian Hahn via Phabricator via llvm-commits]

fhahn marked an inline comment as done.
fhahn added a comment.

Thank you very much, Eli!



================
Comment at: llvm/lib/Transforms/Scalar/SCCP.cpp:1635
+        Inst.replaceAllUsesWith(ZExt);
+        Inst.eraseFromParent();
+        InstReplacedStat++;
----------------
efriedma wrote:
> fhahn wrote:
> > efriedma wrote:
> > > Are we keeping around stale pointers in the Solver?  That's technically UB, I think, even if the code works otherwise, and the compiler is unlikely to take advantage.
> > Yes it does currently. I don't think it would cause in problems in practice, but it is better to properly remove it. I've added a `removeLatticeValueFor` and used that. We don't remove the entries when removing other instructions, but I guess if we create new instructions we could in theory re-use existing pointers or something.
> Reusing existing pointers would only be if we tried to query the solver for an instruction we created after the solver runs; I don't think we do that.
Yep. The latest version does clear them up now, so we don't have to worry about it in the future :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81756