[PATCH] D17921: [CorrelatedValuePropagation] Convert an SDiv to a UDiv if both operands are known to be nonnegative

[Philip Reames via llvm-commits]

reames requested changes to this revision.
reames added a comment.
This revision now requires changes to proceed.

Code wise, looks close to good to go, but I'm missing something design wise.  Why is it a good idea to canonicalize sdiv to udiv in general?  I can see why when the second operand is a special constant, but why in general?


================
Comment at: lib/Transforms/Scalar/CorrelatedValuePropagation.cpp:362
@@ +361,3 @@
+    LazyValueInfo::Tristate Result = LVI->getPredicateAt(
+        ICmpInst::ICMP_SGT, O, ConstantInt::get(O->getType(), 0), SDI);
+    if (Result != LazyValueInfo::True)
----------------
Pull the constant int out of the loop.

================
Comment at: test/Transforms/CorrelatedValuePropagation/sdiv.ll:15
@@ +14,3 @@
+  %cmp1 = icmp sgt i32 %j.0, 0
+; CHECK: %div1 = udiv i32 %j.0, 2 
+  %div = sdiv i32 %j.0, 2
----------------
Please add a couple of extra tests:
- a negative test to show it doesn't trigger when negative
- a simpler case without a loop to show that it works with an early exit guard


Repository:
  rL LLVM

http://reviews.llvm.org/D17921