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

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/SCCP.cpp:1311
             IV, &CB,
-            ValueLatticeElement::getRange(NewCR, /*MayIncludeUndef=*/true));
+            ValueLatticeElement::getRange(NewCR, /*MayIncludeUndef=*/false));
         return;
----------------
I'm a little uncomfortable that flipping this from true to false impacted zero testcases...

I'm not sure how to judge exactly how risky it is to land this without fixes to various passes we know have issues.  I'm particularly worried about loop unswitch, since we saw a real miscompile involving it in the past.


================
Comment at: llvm/lib/Transforms/Scalar/SCCP.cpp:1635
+        Inst.replaceAllUsesWith(ZExt);
+        Inst.eraseFromParent();
+        InstReplacedStat++;
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81756