[llvm] 681a6a3 - [InstCombine] allow more folds more multi-use selects

Sanjay Patel via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 11 12:37:42 PST 2022


Author: Sanjay Patel
Date: 2022-11-11T15:26:54-05:00
New Revision: 681a6a3990226c667fb648952731d2753df2716d

URL: https://github.com/llvm/llvm-project/commit/681a6a3990226c667fb648952731d2753df2716d
DIFF: https://github.com/llvm/llvm-project/commit/681a6a3990226c667fb648952731d2753df2716d.diff

LOG: [InstCombine] allow more folds more multi-use selects

The 'and' case showed up in a recent bug report and prevented
more follow-on transforms from happening.

We could handle more patterns (for example, the select arms
simplified, but not to constant values), but this seems
like a safe, conservative enhancement. The backend can
convert select-of-constants to math/logic in many cases
if it is profitable.

There is a lot of overlapping logic for these kinds of patterns
(see SimplifySelectsFeedingBinaryOp() and FoldOpIntoSelect()),
so there may be some opportunity to improve efficiency.

There are also optimization gaps/inconsistency because we do
not call this code for all bin-opcodes (see TODO for ashr test).

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 61bac17811326..f6cd9260c1499 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -919,18 +919,24 @@ Value *InstCombinerImpl::SimplifySelectsFeedingBinaryOp(BinaryOperator &I,
       else if (True && !False)
         False = Builder.CreateBinOp(Opcode, C, F);
     }
-  } else if (LHSIsSelect && LHS->hasOneUse()) {
+  } else if (LHSIsSelect) {
     // (A ? B : C) op Y -> A ? (B op Y) : (C op Y)
     Cond = A;
     True = simplifyBinOp(Opcode, B, RHS, FMF, Q);
     False = simplifyBinOp(Opcode, C, RHS, FMF, Q);
+    if (!LHS->hasOneUse() &&
+        (!True || !False || !isa<Constant>(True) || !isa<Constant>(False)))
+      return nullptr;
     if (Value *NewSel = foldAddNegate(B, C, RHS))
       return NewSel;
-  } else if (RHSIsSelect && RHS->hasOneUse()) {
+  } else if (RHSIsSelect) {
     // X op (D ? E : F) -> D ? (X op E) : (X op F)
     Cond = D;
     True = simplifyBinOp(Opcode, LHS, E, FMF, Q);
     False = simplifyBinOp(Opcode, LHS, F, FMF, Q);
+    if (!RHS->hasOneUse() &&
+        (!True || !False || !isa<Constant>(True) || !isa<Constant>(False)))
+      return nullptr;
     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 90f41a29aed57..cce1206f1b90e 100644
--- a/llvm/test/Transforms/InstCombine/binop-select.ll
+++ b/llvm/test/Transforms/InstCombine/binop-select.ll
@@ -188,11 +188,13 @@ 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:%.*]] = and i32 [[S]], 1
+; CHECK-NEXT:    [[R:%.*]] = zext i1 [[B]] to i32
 ; CHECK-NEXT:    ret i32 [[R]]
 ;
   %s = select i1 %b, i32 25, i32 0
@@ -212,12 +214,14 @@ 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:%.*]] = mul i32 [[S]], [[X]]
+; CHECK-NEXT:    [[R:%.*]] = select i1 [[B]], i32 0, i32 42
 ; CHECK-NEXT:    ret i32 [[R]]
 ;
   %d = udiv exact i32 42, %x
@@ -238,11 +242,14 @@ 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:    [[R:%.*]] = sub nsw i32 42, [[S]]
+; CHECK-NEXT:    [[NOT_B:%.*]] = xor i1 [[B]], true
+; CHECK-NEXT:    [[R:%.*]] = zext i1 [[NOT_B]] to i32
 ; CHECK-NEXT:    ret i32 [[R]]
 ;
   %s = select i1 %b, i32 42, i32 41
@@ -261,11 +268,13 @@ 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:%.*]] = fadd nnan float [[S]], [[X:%.*]]
+; CHECK-NEXT:    [[R:%.*]] = select nnan i1 [[B]], float 0xFFF0000000000000, float 0x7FF0000000000000
 ; CHECK-NEXT:    ret float [[R]]
 ;
   %s = select i1 %b, float 0xFFF0000000000000, float 0x7FF0000000000000
@@ -284,13 +293,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:    [[R:%.*]] = fmul nnan nsz <2 x half> [[X]], [[S]]
-; CHECK-NEXT:    ret <2 x half> [[R]]
+; CHECK-NEXT:    ret <2 x half> zeroinitializer
 ;
   %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>
@@ -309,6 +318,8 @@ 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