[ValueTracking] Fix PR23011.

Summary:
`ComputeNumSignBits` returns incorrect results for `srem` instructions.
This change fixes the issue and adds a test case.

Reviewers: nadav, nicholas, atrick

Subscribers: llvm-commits

Differential Revision: http://reviews.llvm.org/D8600

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@233225 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Analysis/ValueTracking.cpp b/lib/Analysis/ValueTracking.cpp
index 67627a35ba8dd43419bef7e7a02b19c0079a11ec..f329e3a5084b3b7aa968ae0cb5dbe45aade0a645 100644 (file)
--- a/lib/Analysis/ValueTracking.cpp
+++ b/lib/Analysis/ValueTracking.cpp
@@ -1914,8 +1914,9 @@ unsigned ComputeNumSignBits(Value *V, const DataLayout &DL, unsigned Depth,
 
   case Instruction::SRem: {
     const APInt *Denominator;
-    // srem X, C -> we know that the result is within 0..C-1 when C is a
-    // positive constant and the sign bits are at most TypeBits - log2(C).
+    // srem X, C -> we know that the result is within [-C+1,C) when C is a
+    // positive constant.  This let us put a lower bound on the number of sign
+    // bits.
     if (match(U->getOperand(1), m_APInt(Denominator))) {
 
       // Ignore non-positive denominator.
@@ -1928,11 +1929,19 @@ unsigned ComputeNumSignBits(Value *V, const DataLayout &DL, unsigned Depth,
           ComputeNumSignBits(U->getOperand(0), DL, Depth + 1, Q);
 
       // Calculate the leading sign bit constraints by examining the
-      // denominator. The remainder is in the range 0..C-1, which is
-      // calculated by the log2(denominator). The sign bits are the bit-width
-      // minus this value. The result of this subtraction has to be positive.
-      unsigned ResBits = TyBits - Denominator->logBase2();
-
+      // denominator.  Given that the denominator is positive, there are two
+      // cases:
+      //
+      //  1. the numerator is positive.  The result range is [0,C) and [0,C) u<
+      //     (1 << ceilLogBase2(C)).
+      //
+      //  2. the numerator is negative.  Then the result range is (-C,0] and
+      //     integers in (-C,0] are either 0 or >u (-1 << ceilLogBase2(C)).
+      //
+      // Thus a lower bound on the number of sign bits is `TyBits -
+      // ceilLogBase2(C)`.
+
+      unsigned ResBits = TyBits - Denominator->ceilLogBase2();
       return std::max(NumrBits, ResBits);
     }
     break;

diff --git a/test/Analysis/ValueTracking/pr23011.ll b/test/Analysis/ValueTracking/pr23011.ll
new file mode 100644 (file)
index 0000000..9edc1c4
--- /dev/null
+++ b/test/Analysis/ValueTracking/pr23011.ll
@@ -0,0 +1,15 @@
+; RUN: opt -indvars -S < %s | FileCheck %s
+
+declare { i8, i1 } @llvm.smul.with.overflow.i8(i8, i8) nounwind readnone
+
+define i1 @test1(i8 %x) {
+ entry:
+; CHECK-LABEL: @test1
+  %rem = srem i8 %x, 15
+  %t = call { i8, i1 } @llvm.smul.with.overflow.i8(i8 %rem, i8 %rem)
+; CHECK: %t = call { i8, i1 } @llvm.smul.with.overflow.i8(i8 %rem, i8 %rem)
+; CHECK: %obit = extractvalue { i8, i1 } %t, 1
+; CHECK: ret i1 %obit
+  %obit = extractvalue { i8, i1 } %t, 1
+  ret i1 %obit
+}