[PATCH] D53659: [ValueTracking] peek through shuffles in ComputeNumSignBits (PR37549)

[Sanjay Patel via Phabricator via llvm-commits]

spatel updated this revision to Diff 170969.
spatel added a comment.

Patch updated:
Don't try to analyze a shuffle with undef mask elements. 
The negative test for 'nsw' actually demonstrates the bug in the last draft. If we had applied 'nsw' in that case, some other analysis could see poison for the undef lane that didn't exist in the original code.


https://reviews.llvm.org/D53659

Files:
  lib/Analysis/ValueTracking.cpp
  test/Transforms/InstCombine/logical-select.ll
  test/Transforms/InstCombine/nsw.ll


Index: test/Transforms/InstCombine/nsw.ll
===================================================================
--- test/Transforms/InstCombine/nsw.ll
+++ test/Transforms/InstCombine/nsw.ll
@@ -99,18 +99,34 @@
   ret i8 %add
 }
 
-; TODO: ComputeNumSignBits()/computeKnownBits() should look through a shufflevector.
+; TODO: computeKnownBits() should look through a shufflevector.
 
 define <3 x i32> @shl_nuw_nsw_shuffle_splat_vec(<2 x i8> %x) {
 ; CHECK-LABEL: @shl_nuw_nsw_shuffle_splat_vec(
 ; CHECK-NEXT:    [[T2:%.*]] = zext <2 x i8> [[X:%.*]] to <2 x i32>
 ; CHECK-NEXT:    [[SHUF:%.*]] = shufflevector <2 x i32> [[T2]], <2 x i32> undef, <3 x i32> <i32 1, i32 0, i32 1>
-; CHECK-NEXT:    [[T3:%.*]] = shl <3 x i32> [[SHUF]], <i32 17, i32 17, i32 17>
+; CHECK-NEXT:    [[T3:%.*]] = shl nsw <3 x i32> [[SHUF]], <i32 17, i32 17, i32 17>
 ; CHECK-NEXT:    ret <3 x i32> [[T3]]
 ;
   %t2 = zext <2 x i8> %x to <2 x i32>
   %shuf = shufflevector <2 x i32> %t2, <2 x i32> undef, <3 x i32> <i32 1, i32 0, i32 1>
   %t3 = shl <3 x i32> %shuf, <i32 17, i32 17, i32 17>
   ret <3 x i32> %t3
 }
 
+; Negative test - if the shuffle mask contains an undef, we bail out to
+; avoid propagating information that may not be used consistently by callers.
+
+define <3 x i32> @shl_nuw_nsw_shuffle_undef_elt_splat_vec(<2 x i8> %x) {
+; CHECK-LABEL: @shl_nuw_nsw_shuffle_undef_elt_splat_vec(
+; CHECK-NEXT:    [[T2:%.*]] = zext <2 x i8> [[X:%.*]] to <2 x i32>
+; CHECK-NEXT:    [[SHUF:%.*]] = shufflevector <2 x i32> [[T2]], <2 x i32> undef, <3 x i32> <i32 1, i32 undef, i32 0>
+; CHECK-NEXT:    [[T3:%.*]] = shl <3 x i32> [[SHUF]], <i32 17, i32 17, i32 17>
+; CHECK-NEXT:    ret <3 x i32> [[T3]]
+;
+  %t2 = zext <2 x i8> %x to <2 x i32>
+  %shuf = shufflevector <2 x i32> %t2, <2 x i32> undef, <3 x i32> <i32 1, i32 undef, i32 0>
+  %t3 = shl <3 x i32> %shuf, <i32 17, i32 17, i32 17>
+  ret <3 x i32> %t3
+}
+
Index: test/Transforms/InstCombine/logical-select.ll
===================================================================
--- test/Transforms/InstCombine/logical-select.ll
+++ test/Transforms/InstCombine/logical-select.ll
@@ -594,13 +594,10 @@
 ; CHECK-NEXT:    [[S3:%.*]] = shufflevector <4 x i32> [[SHUF_OR1]], <4 x i32> undef, <4 x i32> <i32 0, i32 0, i32 1, i32 1>
 ; CHECK-NEXT:    [[S4:%.*]] = shufflevector <4 x i32> [[SHUF_OR1]], <4 x i32> undef, <4 x i32> <i32 2, i32 2, i32 3, i32 3>
 ; CHECK-NEXT:    [[SHUF_OR2:%.*]] = or <4 x i32> [[S3]], [[S4]]
-; CHECK-NEXT:    [[NOT_OR2:%.*]] = xor <4 x i32> [[SHUF_OR2]], <i32 -1, i32 -1, i32 -1, i32 -1>
-; CHECK-NEXT:    [[XBC:%.*]] = bitcast <4 x float> [[X]] to <4 x i32>
-; CHECK-NEXT:    [[ZBC:%.*]] = bitcast <4 x float> [[Z:%.*]] to <4 x i32>
-; CHECK-NEXT:    [[AND1:%.*]] = and <4 x i32> [[NOT_OR2]], [[XBC]]
-; CHECK-NEXT:    [[AND2:%.*]] = and <4 x i32> [[SHUF_OR2]], [[ZBC]]
-; CHECK-NEXT:    [[SEL:%.*]] = or <4 x i32> [[AND1]], [[AND2]]
-; CHECK-NEXT:    ret <4 x i32> [[SEL]]
+; CHECK-NEXT:    [[TMP1:%.*]] = trunc <4 x i32> [[SHUF_OR2]] to <4 x i1>
+; CHECK-NEXT:    [[DOTV:%.*]] = select <4 x i1> [[TMP1]], <4 x float> [[Z:%.*]], <4 x float> [[X]]
+; CHECK-NEXT:    [[TMP2:%.*]] = bitcast <4 x float> [[DOTV]] to <4 x i32>
+; CHECK-NEXT:    ret <4 x i32> [[TMP2]]
 ;
   %cmp = fcmp ole <4 x float> %x, %y
   %sext = sext <4 x i1> %cmp to <4 x i32>
Index: lib/Analysis/ValueTracking.cpp
===================================================================
--- lib/Analysis/ValueTracking.cpp
+++ lib/Analysis/ValueTracking.cpp
@@ -2510,6 +2510,23 @@
     // valid for all elements of the vector (for example if vector is sign
     // extended, shifted, etc).
     return ComputeNumSignBits(U->getOperand(0), Depth + 1, Q);
+
+  case Instruction::ShuffleVector:
+    // If the shuffle mask contains any undefined elements, that element of the
+    // result is undefined. Propagating information from a source operand may
+    // not be correct in that case, so just bail out.
+    if (cast<ShuffleVectorInst>(U)->getMask()->containsUndefElement())
+      break;
+
+    // Look through shuffle of 1 source vector.
+    if (isa<UndefValue>(U->getOperand(0)))
+      return ComputeNumSignBits(U->getOperand(1), Depth + 1, Q);
+    if (isa<UndefValue>(U->getOperand(1)))
+      return ComputeNumSignBits(U->getOperand(0), Depth + 1, Q);
+
+    // TODO: We can look through shuffles of 2 sources by computing the minimum
+    // sign bits for each operand (similar to what we do for binops).
+    break;
   }
 
   // Finally, if we can prove that the top bits of the result are 0's or 1's,


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D53659.170969.patch
Type: text/x-patch
Size: 4547 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181024/8e2e7672/attachment.bin>