[llvm] 362c235 - Revert "[InstCombine] allow more folds for multi-use selects (2nd try)"

Sanjay Patel via llvm-commits llvm-commits at lists.llvm.org
Sun Nov 13 08:51:45 PST 2022


Author: Sanjay Patel
Date: 2022-11-13T11:47:21-05:00
New Revision: 362c23500a500da7f42f06eb9b72326618863a97

URL: https://github.com/llvm/llvm-project/commit/362c23500a500da7f42f06eb9b72326618863a97
DIFF: https://github.com/llvm/llvm-project/commit/362c23500a500da7f42f06eb9b72326618863a97.diff

LOG: Revert "[InstCombine] allow more folds for multi-use selects (2nd try)"

This reverts commit 6eae6b3722d9204fa93b772e24afab93406cc143.

This version of the patch results in the same DFSAN bot failure as before,
so my guess about the SimplifyQuery context instruction was wrong.
I don't know what the real bug is.

Added: 
    

Modified: 
    llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
    llvm/test/Transforms/InstCombine/binop-select.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index 58c2deca4962..61bac1781132 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -882,6 +882,8 @@ Value *InstCombinerImpl::SimplifySelectsFeedingBinaryOp(BinaryOperator &I,
   }
 
   Instruction::BinaryOps Opcode = I.getOpcode();
+  SimplifyQuery Q = SQ.getWithInstruction(&I);
+
   Value *Cond, *True = nullptr, *False = nullptr;
 
   // Special-case for add/negate combination. Replace the zero in the negation
@@ -908,8 +910,8 @@ Value *InstCombinerImpl::SimplifySelectsFeedingBinaryOp(BinaryOperator &I,
   if (LHSIsSelect && RHSIsSelect && A == D) {
     // (A ? B : C) op (A ? E : F) -> A ? (B op E) : (C op F)
     Cond = A;
-    True = simplifyBinOp(Opcode, B, E, FMF, SQ);
-    False = simplifyBinOp(Opcode, C, F, FMF, SQ);
+    True = simplifyBinOp(Opcode, B, E, FMF, Q);
+    False = simplifyBinOp(Opcode, C, F, FMF, Q);
 
     if (LHS->hasOneUse() && RHS->hasOneUse()) {
       if (False && !True)
@@ -917,24 +919,18 @@ Value *InstCombinerImpl::SimplifySelectsFeedingBinaryOp(BinaryOperator &I,
       else if (True && !False)
         False = Builder.CreateBinOp(Opcode, C, F);
     }
-  } else if (LHSIsSelect) {
+  } else if (LHSIsSelect && LHS->hasOneUse()) {
     // (A ? B : C) op Y -> A ? (B op Y) : (C op Y)
     Cond = A;
-    True = simplifyBinOp(Opcode, B, RHS, FMF, SQ);
-    False = simplifyBinOp(Opcode, C, RHS, FMF, SQ);
-    if (!LHS->hasOneUse() &&
-        (!True || !False || !isa<Constant>(True) || !isa<Constant>(False)))
-      return nullptr;
+    True = simplifyBinOp(Opcode, B, RHS, FMF, Q);
+    False = simplifyBinOp(Opcode, C, RHS, FMF, Q);
     if (Value *NewSel = foldAddNegate(B, C, RHS))
       return NewSel;
-  } else if (RHSIsSelect) {
+  } else if (RHSIsSelect && RHS->hasOneUse()) {
     // X op (D ? E : F) -> D ? (X op E) : (X op F)
     Cond = D;
-    True = simplifyBinOp(Opcode, LHS, E, FMF, SQ);
-    False = simplifyBinOp(Opcode, LHS, F, FMF, SQ);
-    if (!RHS->hasOneUse() &&
-        (!True || !False || !isa<Constant>(True) || !isa<Constant>(False)))
-      return nullptr;
+    True = simplifyBinOp(Opcode, LHS, E, FMF, Q);
+    False = simplifyBinOp(Opcode, LHS, F, FMF, Q);
     if (Value *NewSel = foldAddNegate(E, F, LHS))
       return NewSel;
   }

diff  --git a/llvm/test/Transforms/InstCombine/binop-select.ll b/llvm/test/Transforms/InstCombine/binop-select.ll
index cce1206f1b90..90f41a29aed5 100644
--- a/llvm/test/Transforms/InstCombine/binop-select.ll
+++ b/llvm/test/Transforms/InstCombine/binop-select.ll
@@ -188,13 +188,11 @@ define i32 @and_sel_op0(i1 %b) {
   ret i32 %r
 }
 
-; extra use is ok
-
 define i32 @and_sel_op0_use(i1 %b) {
 ; CHECK-LABEL: @and_sel_op0_use(
 ; CHECK-NEXT:    [[S:%.*]] = select i1 [[B:%.*]], i32 25, i32 0
 ; CHECK-NEXT:    call void @use(i32 [[S]])
-; CHECK-NEXT:    [[R:%.*]] = zext i1 [[B]] to i32
+; CHECK-NEXT:    [[R:%.*]] = and i32 [[S]], 1
 ; CHECK-NEXT:    ret i32 [[R]]
 ;
   %s = select i1 %b, i32 25, i32 0
@@ -214,14 +212,12 @@ define i32 @mul_sel_op0(i1 %b, i32 %x) {
   ret i32 %r
 }
 
-; extra use is ok
-
 define i32 @mul_sel_op0_use(i1 %b, i32 %x) {
 ; CHECK-LABEL: @mul_sel_op0_use(
 ; CHECK-NEXT:    [[D:%.*]] = udiv exact i32 42, [[X:%.*]]
 ; CHECK-NEXT:    [[S:%.*]] = select i1 [[B:%.*]], i32 0, i32 [[D]]
 ; CHECK-NEXT:    call void @use(i32 [[S]])
-; CHECK-NEXT:    [[R:%.*]] = select i1 [[B]], i32 0, i32 42
+; CHECK-NEXT:    [[R:%.*]] = mul i32 [[S]], [[X]]
 ; CHECK-NEXT:    ret i32 [[R]]
 ;
   %d = udiv exact i32 42, %x
@@ -242,14 +238,11 @@ define i32 @sub_sel_op1(i1 %b) {
   ret i32 %r
 }
 
-; extra use is ok (extra instruction caused by select canonicalization to zext (!b))
-
 define i32 @sub_sel_op1_use(i1 %b) {
 ; CHECK-LABEL: @sub_sel_op1_use(
 ; CHECK-NEXT:    [[S:%.*]] = select i1 [[B:%.*]], i32 42, i32 41
 ; CHECK-NEXT:    call void @use(i32 [[S]])
-; CHECK-NEXT:    [[NOT_B:%.*]] = xor i1 [[B]], true
-; CHECK-NEXT:    [[R:%.*]] = zext i1 [[NOT_B]] to i32
+; CHECK-NEXT:    [[R:%.*]] = sub nsw i32 42, [[S]]
 ; CHECK-NEXT:    ret i32 [[R]]
 ;
   %s = select i1 %b, i32 42, i32 41
@@ -268,13 +261,11 @@ define float @fadd_sel_op0(i1 %b, float %x) {
   ret float %r
 }
 
-; extra use is ok
-
 define float @fadd_sel_op0_use(i1 %b, float %x) {
 ; CHECK-LABEL: @fadd_sel_op0_use(
 ; CHECK-NEXT:    [[S:%.*]] = select i1 [[B:%.*]], float 0xFFF0000000000000, float 0x7FF0000000000000
 ; CHECK-NEXT:    call void @use_f32(float [[S]])
-; CHECK-NEXT:    [[R:%.*]] = select nnan i1 [[B]], float 0xFFF0000000000000, float 0x7FF0000000000000
+; CHECK-NEXT:    [[R:%.*]] = fadd nnan float [[S]], [[X:%.*]]
 ; CHECK-NEXT:    ret float [[R]]
 ;
   %s = select i1 %b, float 0xFFF0000000000000, float 0x7FF0000000000000
@@ -293,13 +284,13 @@ define <2 x half> @fmul_sel_op1(i1 %b, <2 x half> %p) {
   ret <2 x half> %r
 }
 
-; extra use is ok - poison allows simplifying
-
 define <2 x half> @fmul_sel_op1_use(i1 %b, <2 x half> %p) {
 ; CHECK-LABEL: @fmul_sel_op1_use(
+; CHECK-NEXT:    [[X:%.*]] = fadd <2 x half> [[P:%.*]], <half 0xH3C00, half 0xH4000>
 ; CHECK-NEXT:    [[S:%.*]] = select i1 [[B:%.*]], <2 x half> zeroinitializer, <2 x half> <half 0xHFFFF, half 0xHFFFF>
 ; CHECK-NEXT:    call void @use_v2f16(<2 x half> [[S]])
-; CHECK-NEXT:    ret <2 x half> zeroinitializer
+; CHECK-NEXT:    [[R:%.*]] = fmul nnan nsz <2 x half> [[X]], [[S]]
+; CHECK-NEXT:    ret <2 x half> [[R]]
 ;
   %x = fadd <2 x half> %p, <half 1.0, half 2.0> ; thwart complexity-based canonicalization
   %s = select i1 %b, <2 x half> zeroinitializer, <2 x half> <half 0xHffff, half 0xHffff>
@@ -318,8 +309,6 @@ define i32 @ashr_sel_op1(i1 %b) {
   ret i32 %r
 }
 
-; TODO: SimplifySelectsFeedingBinaryOp() is not called for shifts; previous test reduced some other way.
-
 define i32 @ashr_sel_op1_use(i1 %b) {
 ; CHECK-LABEL: @ashr_sel_op1_use(
 ; CHECK-NEXT:    [[S:%.*]] = select i1 [[B:%.*]], i32 2, i32 0


        


More information about the llvm-commits mailing list