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

Philip Reames via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 10 18:08:50 PST 2016


reames accepted this revision.
reames added a comment.
This revision is now accepted and ready to land.

LGTM w/a couple of really minor comments addressed.


================
Comment at: lib/Transforms/Scalar/CorrelatedValuePropagation.cpp:38
@@ -37,2 +37,3 @@
 STATISTIC(NumDeadCases, "Number of switch cases removed");
+STATISTIC(NumSDivs,     "Number of sdiv propagated");
 
----------------
minor: propagated is the wrong word.  Possibly: "converted to udiv"

================
Comment at: lib/Transforms/Scalar/CorrelatedValuePropagation.cpp:344
@@ +343,3 @@
+/// information is sufficient to prove the both operands of this SDiv are
+/// nonnegative.  If this is the case, replace the SDiv with a UDiv. Even for
+/// local conditions, this can sometimes prove conditions instcombine can't by
----------------
minor: the code is proving the inputs positive, not non-negative.

================
Comment at: test/Transforms/CorrelatedValuePropagation/sdiv.ll:35
@@ +34,3 @@
+  %cmp1 = icmp sgt i32 %j.0, 0
+; CHECK-NOT: %div1 = udiv i32 %j.0, 2 
+  %div = sdiv i32 %j.0, 2
----------------
Please use a check line rather than a CHECK-NOT line here.  The check-not is too fragile in the face of minor changes.  (e.g. naming)


Repository:
  rL LLVM

http://reviews.llvm.org/D17921





More information about the llvm-commits mailing list