[llvm] r354905 - [InstSimplify] remove zero-shift-guard fold for general funnel shift

Sanjay Patel via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 26 10:26:56 PST 2019


Author: spatel
Date: Tue Feb 26 10:26:56 2019
New Revision: 354905

URL: http://llvm.org/viewvc/llvm-project?rev=354905&view=rev
Log:
[InstSimplify] remove zero-shift-guard fold for general funnel shift

As discussed on llvm-dev:
http://lists.llvm.org/pipermail/llvm-dev/2019-February/130491.html

We can't remove the compare+select in the general case because
we are treating funnel shift like a standard instruction (as
opposed to a special instruction like select/phi).

That means that if one of the operands of the funnel shift is
poison, the result is poison regardless of whether we know that
the operand is actually unused based on the instruction's
particular semantics.

The motivating case for this transform is the more specific
rotate op (rather than funnel shift), and we are preserving the
fold for that case because there is no chance of introducing
extra poison when there is no anonymous extra operand to the
funnel shift.

Modified:
    llvm/trunk/lib/Analysis/InstructionSimplify.cpp
    llvm/trunk/test/Transforms/InstSimplify/call.ll

Modified: llvm/trunk/lib/Analysis/InstructionSimplify.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/InstructionSimplify.cpp?rev=354905&r1=354904&r2=354905&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/InstructionSimplify.cpp (original)
+++ llvm/trunk/lib/Analysis/InstructionSimplify.cpp Tue Feb 26 10:26:56 2019
@@ -3908,27 +3908,44 @@ static Value *simplifySelectWithICmpCond
                                            Pred == ICmpInst::ICMP_EQ))
         return V;
 
-    // Test for zero-shift-guard-ops around funnel shifts. These are used to
-    // avoid UB from oversized shifts in raw IR rotate patterns, but the
-    // intrinsics do not have that problem.
+    // Test for a bogus zero-shift-guard-op around funnel-shift or rotate.
     Value *ShAmt;
     auto isFsh = m_CombineOr(m_Intrinsic<Intrinsic::fshl>(m_Value(X), m_Value(),
                                                           m_Value(ShAmt)),
                              m_Intrinsic<Intrinsic::fshr>(m_Value(), m_Value(X),
                                                           m_Value(ShAmt)));
-    // (ShAmt != 0) ? fshl(X, *, ShAmt) : X --> fshl(X, *, ShAmt)
-    // (ShAmt != 0) ? fshr(*, X, ShAmt) : X --> fshr(*, X, ShAmt)
     // (ShAmt == 0) ? fshl(X, *, ShAmt) : X --> X
     // (ShAmt == 0) ? fshr(*, X, ShAmt) : X --> X
-    if (match(TrueVal, isFsh) && FalseVal == X && CmpLHS == ShAmt)
-      return Pred == ICmpInst::ICMP_NE ? TrueVal : X;
-
-    // (ShAmt == 0) ? X : fshl(X, *, ShAmt) --> fshl(X, *, ShAmt)
-    // (ShAmt == 0) ? X : fshr(*, X, ShAmt) --> fshr(*, X, ShAmt)
+    if (match(TrueVal, isFsh) && FalseVal == X && CmpLHS == ShAmt &&
+        Pred == ICmpInst::ICMP_EQ)
+      return X;
     // (ShAmt != 0) ? X : fshl(X, *, ShAmt) --> X
     // (ShAmt != 0) ? X : fshr(*, X, ShAmt) --> X
-    if (match(FalseVal, isFsh) && TrueVal == X && CmpLHS == ShAmt)
-      return Pred == ICmpInst::ICMP_EQ ? FalseVal : X;
+    if (match(FalseVal, isFsh) && TrueVal == X && CmpLHS == ShAmt &&
+        Pred == ICmpInst::ICMP_NE)
+      return X;
+
+    // Test for a zero-shift-guard-op around rotates. These are used to
+    // avoid UB from oversized shifts in raw IR rotate patterns, but the
+    // intrinsics do not have that problem.
+    // We do not allow this transform for the general funnel shift case because
+    // that would not preserve the poison safety of the original code.
+    auto isRotate = m_CombineOr(m_Intrinsic<Intrinsic::fshl>(m_Value(X),
+                                                             m_Deferred(X),
+                                                             m_Value(ShAmt)),
+                                m_Intrinsic<Intrinsic::fshr>(m_Value(X),
+                                                             m_Deferred(X),
+                                                             m_Value(ShAmt)));
+    // (ShAmt != 0) ? fshl(X, X, ShAmt) : X --> fshl(X, X, ShAmt)
+    // (ShAmt != 0) ? fshr(X, X, ShAmt) : X --> fshr(X, X, ShAmt)
+    if (match(TrueVal, isRotate) && FalseVal == X && CmpLHS == ShAmt &&
+        Pred == ICmpInst::ICMP_NE)
+      return TrueVal;
+    // (ShAmt == 0) ? X : fshl(X, X, ShAmt) --> fshl(X, X, ShAmt)
+    // (ShAmt == 0) ? X : fshr(X, X, ShAmt) --> fshr(X, X, ShAmt)
+    if (match(FalseVal, isRotate) && TrueVal == X && CmpLHS == ShAmt &&
+        Pred == ICmpInst::ICMP_EQ)
+      return FalseVal;
   }
 
   // Check for other compares that behave like bit test.

Modified: llvm/trunk/test/Transforms/InstSimplify/call.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstSimplify/call.ll?rev=354905&r1=354904&r2=354905&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/InstSimplify/call.ll (original)
+++ llvm/trunk/test/Transforms/InstSimplify/call.ll Tue Feb 26 10:26:56 2019
@@ -470,12 +470,14 @@ define <2 x i8> @fshr_no_shift_modulo_bi
   ret <2 x i8> %z
 }
 
-; When the shift amount is 0, fshl returns its 1st parameter (x), so the guard is not needed.
+; If y is poison, eliminating the guard is not safe.
 
 define i8 @fshl_zero_shift_guard(i8 %x, i8 %y, i8 %sh) {
 ; CHECK-LABEL: @fshl_zero_shift_guard(
-; CHECK-NEXT:    [[F:%.*]] = call i8 @llvm.fshl.i8(i8 [[X:%.*]], i8 [[Y:%.*]], i8 [[SH:%.*]])
-; CHECK-NEXT:    ret i8 [[F]]
+; CHECK-NEXT:    [[C:%.*]] = icmp eq i8 [[SH:%.*]], 0
+; CHECK-NEXT:    [[F:%.*]] = call i8 @llvm.fshl.i8(i8 [[X:%.*]], i8 [[Y:%.*]], i8 [[SH]])
+; CHECK-NEXT:    [[S:%.*]] = select i1 [[C]], i8 [[X]], i8 [[F]]
+; CHECK-NEXT:    ret i8 [[S]]
 ;
   %c = icmp eq i8 %sh, 0
   %f = call i8 @llvm.fshl.i8(i8 %x, i8 %y, i8 %sh)
@@ -483,12 +485,14 @@ define i8 @fshl_zero_shift_guard(i8 %x,
   ret i8 %s
 }
 
-; When the shift amount is 0, fshl returns its 1st parameter (x), so the guard is not needed.
+; If y is poison, eliminating the guard is not safe.
 
 define i8 @fshl_zero_shift_guard_swapped(i8 %x, i8 %y, i8 %sh) {
 ; CHECK-LABEL: @fshl_zero_shift_guard_swapped(
-; CHECK-NEXT:    [[F:%.*]] = call i8 @llvm.fshl.i8(i8 [[X:%.*]], i8 [[Y:%.*]], i8 [[SH:%.*]])
-; CHECK-NEXT:    ret i8 [[F]]
+; CHECK-NEXT:    [[C:%.*]] = icmp ne i8 [[SH:%.*]], 0
+; CHECK-NEXT:    [[F:%.*]] = call i8 @llvm.fshl.i8(i8 [[X:%.*]], i8 [[Y:%.*]], i8 [[SH]])
+; CHECK-NEXT:    [[S:%.*]] = select i1 [[C]], i8 [[F]], i8 [[X]]
+; CHECK-NEXT:    ret i8 [[S]]
 ;
   %c = icmp ne i8 %sh, 0
   %f = call i8 @llvm.fshl.i8(i8 %x, i8 %y, i8 %sh)
@@ -520,12 +524,14 @@ define i8 @fshl_zero_shift_guard_inverte
   ret i8 %s
 }
 
-; When the shift amount is 0, fshr returns its 2nd parameter (y), so the guard is not needed.
+; If x is poison, eliminating the guard is not safe.
 
 define i9 @fshr_zero_shift_guard(i9 %x, i9 %y, i9 %sh) {
 ; CHECK-LABEL: @fshr_zero_shift_guard(
-; CHECK-NEXT:    [[F:%.*]] = call i9 @llvm.fshr.i9(i9 [[X:%.*]], i9 [[Y:%.*]], i9 [[SH:%.*]])
-; CHECK-NEXT:    ret i9 [[F]]
+; CHECK-NEXT:    [[C:%.*]] = icmp eq i9 [[SH:%.*]], 0
+; CHECK-NEXT:    [[F:%.*]] = call i9 @llvm.fshr.i9(i9 [[X:%.*]], i9 [[Y:%.*]], i9 [[SH]])
+; CHECK-NEXT:    [[S:%.*]] = select i1 [[C]], i9 [[Y]], i9 [[F]]
+; CHECK-NEXT:    ret i9 [[S]]
 ;
   %c = icmp eq i9 %sh, 0
   %f = call i9 @llvm.fshr.i9(i9 %x, i9 %y, i9 %sh)
@@ -533,12 +539,14 @@ define i9 @fshr_zero_shift_guard(i9 %x,
   ret i9 %s
 }
 
-; When the shift amount is 0, fshr returns its 2nd parameter (y), so the guard is not needed.
+; If x is poison, eliminating the guard is not safe.
 
 define i9 @fshr_zero_shift_guard_swapped(i9 %x, i9 %y, i9 %sh) {
 ; CHECK-LABEL: @fshr_zero_shift_guard_swapped(
-; CHECK-NEXT:    [[F:%.*]] = call i9 @llvm.fshr.i9(i9 [[X:%.*]], i9 [[Y:%.*]], i9 [[SH:%.*]])
-; CHECK-NEXT:    ret i9 [[F]]
+; CHECK-NEXT:    [[C:%.*]] = icmp ne i9 [[SH:%.*]], 0
+; CHECK-NEXT:    [[F:%.*]] = call i9 @llvm.fshr.i9(i9 [[X:%.*]], i9 [[Y:%.*]], i9 [[SH]])
+; CHECK-NEXT:    [[S:%.*]] = select i1 [[C]], i9 [[F]], i9 [[Y]]
+; CHECK-NEXT:    ret i9 [[S]]
 ;
   %c = icmp ne i9 %sh, 0
   %f = call i9 @llvm.fshr.i9(i9 %x, i9 %y, i9 %sh)
@@ -687,14 +695,14 @@ define i8 @fshl_zero_shift_guard_wrong_s
 
 ; Vector types work too.
 
-define <2 x i8> @fshr_zero_shift_guard_splat(<2 x i8> %x, <2 x i8> %y, <2 x i8> %sh) {
-; CHECK-LABEL: @fshr_zero_shift_guard_splat(
-; CHECK-NEXT:    [[F:%.*]] = call <2 x i8> @llvm.fshr.v2i8(<2 x i8> [[X:%.*]], <2 x i8> [[Y:%.*]], <2 x i8> [[SH:%.*]])
+define <2 x i8> @rotr_zero_shift_guard_splat(<2 x i8> %x, <2 x i8> %sh) {
+; CHECK-LABEL: @rotr_zero_shift_guard_splat(
+; CHECK-NEXT:    [[F:%.*]] = call <2 x i8> @llvm.fshr.v2i8(<2 x i8> [[X:%.*]], <2 x i8> [[X]], <2 x i8> [[SH:%.*]])
 ; CHECK-NEXT:    ret <2 x i8> [[F]]
 ;
   %c = icmp eq <2 x i8> %sh, zeroinitializer
-  %f = call <2 x i8> @llvm.fshr.v2i8(<2 x i8> %x, <2 x i8> %y, <2 x i8> %sh)
-  %s = select <2 x i1> %c, <2 x i8> %y, <2 x i8> %f
+  %f = call <2 x i8> @llvm.fshr.v2i8(<2 x i8> %x, <2 x i8> %x, <2 x i8> %sh)
+  %s = select <2 x i1> %c, <2 x i8> %x, <2 x i8> %f
   ret <2 x i8> %s
 }
 




More information about the llvm-commits mailing list