[PATCH] [ValueTracking] Fix PR23011.

Sanjoy Das sanjoy at playingwithpointers.com
Tue Mar 24 21:34:10 PDT 2015


Hi nadav, nicholas, atrick,

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

http://reviews.llvm.org/D8600

Files:
  lib/Analysis/ValueTracking.cpp
  test/Analysis/ValueTracking/pr230011.ll

Index: lib/Analysis/ValueTracking.cpp
===================================================================
--- lib/Analysis/ValueTracking.cpp
+++ lib/Analysis/ValueTracking.cpp
@@ -1914,8 +1914,9 @@
 
   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,10 +1929,16 @@
           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.  If the numerator is positive, then the remainder is in
+      // [0,C), and the minimum number of sign bits (all zero) is "CLZ(C-1)".
+      // If the numerator is negative, then the remainder is in [-C+1,0] and the
+      // minimum number of sign bits is "CLO(-C+1)".  Therefore a lower bound on
+      // the number of sign bits is MIN(CLZ(C-1), CLO(-C+1)).
+      //
+
+      unsigned WhenPositive = (*Denominator - 1).countLeadingZeros();
+      unsigned WhenNegative = (-(*Denominator) + 1).countLeadingOnes();
+      unsigned ResBits = std::min(WhenPositive, WhenNegative);
 
       return std::max(NumrBits, ResBits);
     }
Index: test/Analysis/ValueTracking/pr230011.ll
===================================================================
--- /dev/null
+++ test/Analysis/ValueTracking/pr230011.ll
@@ -0,0 +1,23 @@
+; RUN: opt -indvars -S < %s | FileCheck %s
+
+declare void @side_effect()
+
+define i1 @test1() {
+; CHECK-LABEL: @test1
+a:
+  br label %b
+b:
+  %rem = srem i32 2, 3
+  %cmp = icmp slt i32 0, %rem
+  br i1 %cmp, label %c, label %d
+c:
+; CHECK-LABEL: c:
+  %B = phi i32 [0, %b], [%incB, %c]
+  call void @side_effect()
+  %incB = add nsw i32 %B, 1
+  %cmpB = icmp slt i32 %incB, %rem
+  br i1 %cmpB, label %c, label %d
+; CHECK: br i1 %cmpB, label %c, label %d
+d:
+  ret i1 %cmp
+}

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D8600.22626.patch
Type: text/x-patch
Size: 2467 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150325/5b685082/attachment.bin>


More information about the llvm-commits mailing list