[PATCH] D59617: [ValueTracking] Use ConstantRange based overflow check for signed sub

[Nikita Popov via Phabricator via llvm-commits]

nikic created this revision.
nikic added reviewers: lebedev.ri, spatel.
Herald added subscribers: llvm-commits, jdoerfert, hiraditya.
Herald added a project: LLVM.

This is D59450 <https://reviews.llvm.org/D59450>, but for signed sub. This case is not NFC, because the overflow logic in ConstantRange is more powerful than the existing check. This resolves the TODO in the function.

I've added two tests to show that this indeed catches more cases than the previous logic, but the main correctness test coverage here is in the existing ConstantRange unit tests.


Repository:
  rL LLVM

https://reviews.llvm.org/D59617

Files:
  llvm/lib/Analysis/ValueTracking.cpp
  llvm/test/Transforms/InstCombine/sub.ll


Index: llvm/test/Transforms/InstCombine/sub.ll
===================================================================
--- llvm/test/Transforms/InstCombine/sub.ll
+++ llvm/test/Transforms/InstCombine/sub.ll
@@ -1271,7 +1271,7 @@
 ; CHECK-LABEL: @nsw_inference1(
 ; CHECK-NEXT:    [[X2:%.*]] = or i32 [[X:%.*]], 1024
 ; CHECK-NEXT:    [[Y2:%.*]] = and i32 [[Y:%.*]], 1
-; CHECK-NEXT:    [[Z:%.*]] = sub nuw i32 [[X2]], [[Y2]]
+; CHECK-NEXT:    [[Z:%.*]] = sub nuw nsw i32 [[X2]], [[Y2]]
 ; CHECK-NEXT:    ret i32 [[Z]]
 ;
   %x2 = or i32 %x, 1024
@@ -1284,7 +1284,7 @@
 ; CHECK-LABEL: @nsw_inference2(
 ; CHECK-NEXT:    [[X2:%.*]] = and i32 [[X:%.*]], -1025
 ; CHECK-NEXT:    [[Y2:%.*]] = or i32 [[Y:%.*]], -2
-; CHECK-NEXT:    [[Z:%.*]] = sub i32 [[X2]], [[Y2]]
+; CHECK-NEXT:    [[Z:%.*]] = sub nsw i32 [[X2]], [[Y2]]
 ; CHECK-NEXT:    ret i32 [[Z]]
 ;
   %x2 = and i32 %x, -1025
Index: llvm/lib/Analysis/ValueTracking.cpp
===================================================================
--- llvm/lib/Analysis/ValueTracking.cpp
+++ llvm/lib/Analysis/ValueTracking.cpp
@@ -4190,17 +4190,12 @@
     return OverflowResult::NeverOverflows;
 
   KnownBits LHSKnown = computeKnownBits(LHS, DL, 0, AC, CxtI, DT);
-
   KnownBits RHSKnown = computeKnownBits(RHS, DL, 0, AC, CxtI, DT);
-
-  // Subtraction of two 2's complement numbers having identical signs will
-  // never overflow.
-  if ((LHSKnown.isNegative() && RHSKnown.isNegative()) ||
-      (LHSKnown.isNonNegative() && RHSKnown.isNonNegative()))
-    return OverflowResult::NeverOverflows;
-
-  // TODO: implement logic similar to checkRippleForAdd
-  return OverflowResult::MayOverflow;
+  ConstantRange LHSRange =
+      ConstantRange::fromKnownBits(LHSKnown, /*signed*/ true);
+  ConstantRange RHSRange =
+      ConstantRange::fromKnownBits(RHSKnown, /*signed*/ true);
+  return mapOverflowResult(LHSRange.signedSubMayOverflow(RHSRange));
 }
 
 bool llvm::isOverflowIntrinsicNoWrap(const IntrinsicInst *II,


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D59617.191589.patch
Type: text/x-patch
Size: 1962 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190320/5cb47469/attachment.bin>