[llvm] 6fedc6a - Revert "[InstCombine] add narrowing transform for low-masked binop with zext operand"

Sanjay Patel via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 10 05:25:15 PDT 2022


Author: Sanjay Patel
Date: 2022-06-10T08:25:10-04:00
New Revision: 6fedc6a2b41e458f0b3e6143b80aabba2a2c090d

URL: https://github.com/llvm/llvm-project/commit/6fedc6a2b41e458f0b3e6143b80aabba2a2c090d
DIFF: https://github.com/llvm/llvm-project/commit/6fedc6a2b41e458f0b3e6143b80aabba2a2c090d.diff

LOG: Revert "[InstCombine] add narrowing transform for low-masked binop with zext operand"

This reverts commit afa192cfb6049a15c5542d132d500b910b802c74.
This can cause an infinite loop as shown with an example in the
post-commit thread.

Added: 
    

Modified: 
    llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
    llvm/test/Transforms/InstCombine/and.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
index d04cdd8e1ec2..0780928c6d8a 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
@@ -1869,7 +1869,6 @@ Instruction *InstCombinerImpl::visitAnd(BinaryOperator &I) {
     };
     BinaryOperator *BO;
     if (match(Op0, m_OneUse(m_BinOp(BO))) && isSuitableBinOpcode(BO)) {
-      Instruction::BinaryOps BOpcode = BO->getOpcode();
       Value *X;
       const APInt *C1;
       // TODO: The one-use restrictions could be relaxed a little if the AND
@@ -1879,30 +1878,12 @@ Instruction *InstCombinerImpl::visitAnd(BinaryOperator &I) {
         unsigned XWidth = X->getType()->getScalarSizeInBits();
         Constant *TruncC1 = ConstantInt::get(X->getType(), C1->trunc(XWidth));
         Value *BinOp = isa<ZExtInst>(BO->getOperand(0))
-                           ? Builder.CreateBinOp(BOpcode, X, TruncC1)
-                           : Builder.CreateBinOp(BOpcode, TruncC1, X);
+                           ? Builder.CreateBinOp(BO->getOpcode(), X, TruncC1)
+                           : Builder.CreateBinOp(BO->getOpcode(), TruncC1, X);
         Constant *TruncC = ConstantInt::get(X->getType(), C->trunc(XWidth));
         Value *And = Builder.CreateAnd(BinOp, TruncC);
         return new ZExtInst(And, Ty);
       }
-
-      if (match(BO->getOperand(0), m_OneUse(m_ZExt(m_Value(X)))) &&
-          C->isMask(X->getType()->getScalarSizeInBits())) {
-        Y = BO->getOperand(1);
-        Value *TrY = Builder.CreateTrunc(Y, X->getType(), Y->getName() + ".tr");
-        Value *NewBO =
-            Builder.CreateBinOp(BOpcode, X, TrY, BO->getName() + ".narrow");
-        return new ZExtInst(NewBO, Ty);
-      }
-
-      if (match(BO->getOperand(1), m_OneUse(m_ZExt(m_Value(X)))) &&
-          C->isMask(X->getType()->getScalarSizeInBits())) {
-        Y = BO->getOperand(0);
-        Value *TrY = Builder.CreateTrunc(Y, X->getType(), Y->getName() + ".tr");
-        Value *NewBO =
-            Builder.CreateBinOp(BOpcode, TrY, X, BO->getName() + ".narrow");
-        return new ZExtInst(NewBO, Ty);
-      }
     }
 
     Constant *C1, *C2;

diff  --git a/llvm/test/Transforms/InstCombine/and.ll b/llvm/test/Transforms/InstCombine/and.ll
index 05f3d3bf708e..e09c634f4a16 100644
--- a/llvm/test/Transforms/InstCombine/and.ll
+++ b/llvm/test/Transforms/InstCombine/and.ll
@@ -744,9 +744,9 @@ define i64 @test39(i32 %X) {
 
 define i32 @lowmask_add_zext(i8 %x, i32 %y) {
 ; CHECK-LABEL: @lowmask_add_zext(
-; CHECK-NEXT:    [[Y_TR:%.*]] = trunc i32 [[Y:%.*]] to i8
-; CHECK-NEXT:    [[BO_NARROW:%.*]] = add i8 [[Y_TR]], [[X:%.*]]
-; CHECK-NEXT:    [[R:%.*]] = zext i8 [[BO_NARROW]] to i32
+; CHECK-NEXT:    [[ZX:%.*]] = zext i8 [[X:%.*]] to i32
+; CHECK-NEXT:    [[BO:%.*]] = add i32 [[ZX]], [[Y:%.*]]
+; CHECK-NEXT:    [[R:%.*]] = and i32 [[BO]], 255
 ; CHECK-NEXT:    ret i32 [[R]]
 ;
   %zx = zext i8 %x to i32
@@ -758,9 +758,9 @@ define i32 @lowmask_add_zext(i8 %x, i32 %y) {
 define i32 @lowmask_add_zext_commute(i16 %x, i32 %p) {
 ; CHECK-LABEL: @lowmask_add_zext_commute(
 ; CHECK-NEXT:    [[Y:%.*]] = mul i32 [[P:%.*]], [[P]]
-; CHECK-NEXT:    [[Y_TR:%.*]] = trunc i32 [[Y]] to i16
-; CHECK-NEXT:    [[BO_NARROW:%.*]] = add i16 [[Y_TR]], [[X:%.*]]
-; CHECK-NEXT:    [[R:%.*]] = zext i16 [[BO_NARROW]] to i32
+; CHECK-NEXT:    [[ZX:%.*]] = zext i16 [[X:%.*]] to i32
+; CHECK-NEXT:    [[BO:%.*]] = add i32 [[Y]], [[ZX]]
+; CHECK-NEXT:    [[R:%.*]] = and i32 [[BO]], 65535
 ; CHECK-NEXT:    ret i32 [[R]]
 ;
   %y = mul i32 %p, %p ; thwart complexity-based canonicalization
@@ -770,8 +770,6 @@ define i32 @lowmask_add_zext_commute(i16 %x, i32 %p) {
   ret i32 %r
 }
 
-; negative test - the mask constant must match the zext source type
-
 define i32 @lowmask_add_zext_wrong_mask(i8 %x, i32 %y) {
 ; CHECK-LABEL: @lowmask_add_zext_wrong_mask(
 ; CHECK-NEXT:    [[ZX:%.*]] = zext i8 [[X:%.*]] to i32
@@ -785,8 +783,6 @@ define i32 @lowmask_add_zext_wrong_mask(i8 %x, i32 %y) {
   ret i32 %r
 }
 
-; negative test - extra use
-
 define i32 @lowmask_add_zext_use1(i8 %x, i32 %y) {
 ; CHECK-LABEL: @lowmask_add_zext_use1(
 ; CHECK-NEXT:    [[ZX:%.*]] = zext i8 [[X:%.*]] to i32
@@ -802,8 +798,6 @@ define i32 @lowmask_add_zext_use1(i8 %x, i32 %y) {
   ret i32 %r
 }
 
-; negative test - extra use
-
 define i32 @lowmask_add_zext_use2(i8 %x, i32 %y) {
 ; CHECK-LABEL: @lowmask_add_zext_use2(
 ; CHECK-NEXT:    [[ZX:%.*]] = zext i8 [[X:%.*]] to i32
@@ -819,13 +813,11 @@ define i32 @lowmask_add_zext_use2(i8 %x, i32 %y) {
   ret i32 %r
 }
 
-; vector splats work too
-
 define <2 x i32> @lowmask_sub_zext(<2 x i4> %x, <2 x i32> %y) {
 ; CHECK-LABEL: @lowmask_sub_zext(
-; CHECK-NEXT:    [[Y_TR:%.*]] = trunc <2 x i32> [[Y:%.*]] to <2 x i4>
-; CHECK-NEXT:    [[BO_NARROW:%.*]] = sub <2 x i4> [[X:%.*]], [[Y_TR]]
-; CHECK-NEXT:    [[R:%.*]] = zext <2 x i4> [[BO_NARROW]] to <2 x i32>
+; CHECK-NEXT:    [[ZX:%.*]] = zext <2 x i4> [[X:%.*]] to <2 x i32>
+; CHECK-NEXT:    [[BO:%.*]] = sub <2 x i32> [[ZX]], [[Y:%.*]]
+; CHECK-NEXT:    [[R:%.*]] = and <2 x i32> [[BO]], <i32 15, i32 15>
 ; CHECK-NEXT:    ret <2 x i32> [[R]]
 ;
   %zx = zext <2 x i4> %x to <2 x i32>
@@ -834,13 +826,11 @@ define <2 x i32> @lowmask_sub_zext(<2 x i4> %x, <2 x i32> %y) {
   ret <2 x i32> %r
 }
 
-; weird types are allowed
-
 define i17 @lowmask_sub_zext_commute(i5 %x, i17 %y) {
 ; CHECK-LABEL: @lowmask_sub_zext_commute(
-; CHECK-NEXT:    [[Y_TR:%.*]] = trunc i17 [[Y:%.*]] to i5
-; CHECK-NEXT:    [[BO_NARROW:%.*]] = sub i5 [[Y_TR]], [[X:%.*]]
-; CHECK-NEXT:    [[R:%.*]] = zext i5 [[BO_NARROW]] to i17
+; CHECK-NEXT:    [[ZX:%.*]] = zext i5 [[X:%.*]] to i17
+; CHECK-NEXT:    [[BO:%.*]] = sub i17 [[Y:%.*]], [[ZX]]
+; CHECK-NEXT:    [[R:%.*]] = and i17 [[BO]], 31
 ; CHECK-NEXT:    ret i17 [[R]]
 ;
   %zx = zext i5 %x to i17
@@ -851,9 +841,9 @@ define i17 @lowmask_sub_zext_commute(i5 %x, i17 %y) {
 
 define i32 @lowmask_mul_zext(i8 %x, i32 %y) {
 ; CHECK-LABEL: @lowmask_mul_zext(
-; CHECK-NEXT:    [[Y_TR:%.*]] = trunc i32 [[Y:%.*]] to i8
-; CHECK-NEXT:    [[BO_NARROW:%.*]] = mul i8 [[Y_TR]], [[X:%.*]]
-; CHECK-NEXT:    [[R:%.*]] = zext i8 [[BO_NARROW]] to i32
+; CHECK-NEXT:    [[ZX:%.*]] = zext i8 [[X:%.*]] to i32
+; CHECK-NEXT:    [[BO:%.*]] = mul i32 [[ZX]], [[Y:%.*]]
+; CHECK-NEXT:    [[R:%.*]] = and i32 [[BO]], 255
 ; CHECK-NEXT:    ret i32 [[R]]
 ;
   %zx = zext i8 %x to i32
@@ -862,8 +852,6 @@ define i32 @lowmask_mul_zext(i8 %x, i32 %y) {
   ret i32 %r
 }
 
-; TODO: we could have narrowed the xor
-
 define i32 @lowmask_xor_zext_commute(i8 %x, i32 %p) {
 ; CHECK-LABEL: @lowmask_xor_zext_commute(
 ; CHECK-NEXT:    [[Y:%.*]] = mul i32 [[P:%.*]], [[P]]
@@ -879,8 +867,6 @@ define i32 @lowmask_xor_zext_commute(i8 %x, i32 %p) {
   ret i32 %r
 }
 
-; TODO: we could have narrowed the or
-
 define i24 @lowmask_or_zext_commute(i16 %x, i24 %y) {
 ; CHECK-LABEL: @lowmask_or_zext_commute(
 ; CHECK-NEXT:    [[ZX:%.*]] = zext i16 [[X:%.*]] to i24


        


More information about the llvm-commits mailing list