[PATCH] D76611: [SCCP] Use ranges for predicate info conditions.

[Florian Hahn via Phabricator via llvm-commits]

fhahn marked 2 inline comments as done.
fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/SCCP.cpp:1274
 
-      if (CmpOp0 != CopyOf)
+      // If the original value is a constant we cannot do better.
+      LatticeVal OriginalVal = getValueState(CopyOf);
----------------
efriedma wrote:
> "If the original value is a constant we cannot do better" is technically not true; in theory, if we prove a contradiction, the ssa.copy is unreachable.  But unlikely to matter in practice, I guess.
Agreed, the special case here was not really necessary. I've removed it. Intersecting the constant ranges below should give the same result in practice. I've added a test case with a contradiction (f11_contradiction). I think in those cases, the contradicting compare will get folded to false, and we never branch to the block with the contradiction.


================
Comment at: llvm/lib/Transforms/Scalar/SCCP.cpp:1299
+      LatticeVal &IV = ValueState[I];
+      if (CopyOf->getType()->isIntegerTy()) {
+        auto NewCR =
----------------
efriedma wrote:
> Do we want to try to handle the case where we have a constant expression of integer type?
Ah yes, I've change it to use ranges only if either operand is a constant range.

I've added a test with a compare of 2 constant ranges. Currently this just swaps the known values and is not too helpful in that case (f14_constexpr2). 

Also, would it be legal to fold something like `i1 icmp eq (i32 ptrtoint (i32* @B to i32), i32 ptrtoint (i32* @B to i32)` to `true`? If it is, it looks like ConstantExpr currently misses that fold.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76611