From 93d88192a20874cab2234947ca5593a0aaf97873 Mon Sep 17 00:00:00 2001
From: Sanjoy Das <sanjoy@playingwithpointers.com>
Date: Wed, 25 Mar 2015 22:33:53 +0000
Subject: [PATCH] [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
---
 lib/Analysis/ValueTracking.cpp         | 21 +++++++++++++++------
 test/Analysis/ValueTracking/pr23011.ll | 15 +++++++++++++++
 2 files changed, 30 insertions(+), 6 deletions(-)
 create mode 100644 test/Analysis/ValueTracking/pr23011.ll

diff --git a/lib/Analysis/ValueTracking.cpp b/lib/Analysis/ValueTracking.cpp
index 67627a35ba8..f329e3a5084 100644
--- 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
index 00000000000..9edc1c40660
--- /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
+}