[llvm] [InstCombine] Restrict multiuse in foldBinOpOfSelectAndCastOfSelectCondition (PR #86545)

via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 25 10:40:57 PDT 2024


Fabian =?utf-8?q?Hügel?= <fabian.huegel at fau.de>
Message-ID:
In-Reply-To: <llvm.org/llvm/llvm-project/pull/86545 at github.com>


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-transforms

Author: None (gwastar)

<details>
<summary>Changes</summary>

This fold currently allows multiuse selects which is not profitable in the general case (see https://godbolt.org/z/TacMbKa3j).

I couldn't figure out a cheap and easy way to avoid the regressions when only one select arm is constant,
but this case probably never comes up in real code anyway.

I tried to handle this fold in FoldOpIntoSelect instead, but that function and its callers currently assume
that the Op must have a constant operand (e.g. for ADDs the function gets called via foldAddWithConstant 
and foldBinOpIntoSelectOrPhi). And changing this pre-condition would probably lead to (compile time)
regressions, so I abandoned the attempt for now and went for a simpler fix. 

Related to #<!-- -->86176


---
Full diff: https://github.com/llvm/llvm-project/pull/86545.diff


2 Files Affected:

- (modified) llvm/lib/Transforms/InstCombine/InstructionCombining.cpp (+10-4) 
- (modified) llvm/test/Transforms/InstCombine/binop-select-cast-of-select-cond.ll (+102) 


``````````diff
diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index 7c40fb4fc86082..46a5adf412b4ba 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -1004,6 +1004,7 @@ InstCombinerImpl::foldBinOpOfSelectAndCastOfSelectCondition(BinaryOperator &I) {
   Value *LHS = I.getOperand(0), *RHS = I.getOperand(1);
   Value *A, *CondVal, *TrueVal, *FalseVal;
   Value *CastOp;
+  SelectInst *SI;
 
   auto MatchSelectAndCast = [&](Value *CastOp, Value *SelectOp) {
     return match(CastOp, m_ZExtOrSExt(m_Value(A))) &&
@@ -1011,14 +1012,15 @@ InstCombinerImpl::foldBinOpOfSelectAndCastOfSelectCondition(BinaryOperator &I) {
            match(SelectOp, m_Select(m_Value(CondVal), m_Value(TrueVal),
                                     m_Value(FalseVal)));
   };
-
   // Make sure one side of the binop is a select instruction, and the other is a
   // zero/sign extension operating on a i1.
-  if (MatchSelectAndCast(LHS, RHS))
+  if (MatchSelectAndCast(LHS, RHS)) {
     CastOp = LHS;
-  else if (MatchSelectAndCast(RHS, LHS))
+    SI = cast<SelectInst>(RHS);
+  } else if (MatchSelectAndCast(RHS, LHS)) {
     CastOp = RHS;
-  else
+    SI = cast<SelectInst>(LHS);
+  } else
     return nullptr;
 
   auto NewFoldedConst = [&](bool IsTrueArm, Value *V) {
@@ -1039,6 +1041,10 @@ InstCombinerImpl::foldBinOpOfSelectAndCastOfSelectCondition(BinaryOperator &I) {
                        : Builder.CreateBinOp(Opc, C, V);
   };
 
+  if (!SI->hasOneUse() && !(isa<Constant>(SI->getTrueValue()) && isa<Constant>(SI->getFalseValue()))) {
+    return nullptr;
+  }
+
   // If the value used in the zext/sext is the select condition, or the negated
   // of the select condition, the binop can be simplified.
   if (CondVal == A) {
diff --git a/llvm/test/Transforms/InstCombine/binop-select-cast-of-select-cond.ll b/llvm/test/Transforms/InstCombine/binop-select-cast-of-select-cond.ll
index 7dc2fe1cb88e1f..84bc4017d76021 100644
--- a/llvm/test/Transforms/InstCombine/binop-select-cast-of-select-cond.ll
+++ b/llvm/test/Transforms/InstCombine/binop-select-cast-of-select-cond.ll
@@ -184,6 +184,108 @@ define i64 @select_non_const_sides(i1 %c, i64 %arg1, i64 %arg2) {
   ret i64 %sub
 }
 
+declare void @use(i64)
+
+define i64 @multiuse_select_non_const(i1 %c, i64 %x, i64 %y) {
+; CHECK-LABEL: define i64 @multiuse_select_non_const
+; CHECK-SAME: (i1 [[C:%.*]], i64 [[X:%.*]], i64 [[Y:%.*]]) {
+; CHECK-NEXT:    [[SEL:%.*]] = select i1 [[C]], i64 [[X]], i64 [[Y]]
+; CHECK-NEXT:    call void @use(i64 [[SEL]])
+; CHECK-NEXT:    [[EXT:%.*]] = zext i1 [[C]] to i64
+; CHECK-NEXT:    [[ADD:%.*]] = add i64 [[SEL]], [[EXT]]
+; CHECK-NEXT:    ret i64 [[ADD]]
+;
+  %sel = select i1 %c, i64 %x, i64 %y
+  call void @use(i64 %sel)
+  %ext = zext i1 %c to i64
+  %add = add i64 %sel, %ext
+  ret i64 %add
+}
+
+define i64 @multiuse_select_non_const_chain(i1 %c, i64 %x, i64 %y) {
+; CHECK-LABEL: define i64 @multiuse_select_non_const_chain
+; CHECK-SAME: (i1 [[C:%.*]], i64 [[X:%.*]], i64 [[Y:%.*]]) {
+; CHECK-NEXT:    [[SEL:%.*]] = select i1 [[C]], i64 [[X]], i64 [[Y]]
+; CHECK-NEXT:    [[EXT:%.*]] = zext i1 [[C]] to i64
+; CHECK-NEXT:    [[ADD:%.*]] = add i64 [[SEL]], [[EXT]]
+; CHECK-NEXT:    [[MUL:%.*]] = mul i64 [[SEL]], [[ADD]]
+; CHECK-NEXT:    ret i64 [[MUL]]
+;
+  %sel = select i1 %c, i64 %x, i64 %y
+  %ext = zext i1 %c to i64
+  %add = add i64 %sel, %ext
+  %mul = mul i64 %sel, %add
+  ret i64 %mul
+}
+
+define i64 @multiuse_select_one_constant_left(i1 %c, i64 %x) {
+; CHECK-LABEL: define i64 @multiuse_select_one_constant_left
+; CHECK-SAME: (i1 [[C:%.*]], i64 [[X:%.*]]) {
+; CHECK-NEXT:    [[SEL:%.*]] = select i1 [[C]], i64 123, i64 [[X]]
+; CHECK-NEXT:    call void @use(i64 [[SEL]])
+; CHECK-NEXT:    [[EXT:%.*]] = zext i1 [[C]] to i64
+; CHECK-NEXT:    [[ADD:%.*]] = add i64 [[SEL]], [[EXT]]
+; CHECK-NEXT:    ret i64 [[ADD]]
+;
+  %sel = select i1 %c, i64 123, i64 %x
+  call void @use(i64 %sel)
+  %ext = zext i1 %c to i64
+  %add = add i64 %sel, %ext
+  ret i64 %add
+}
+
+define i64 @multiuse_select_one_constant_right(i1 %c, i64 %x) {
+; CHECK-LABEL: define i64 @multiuse_select_one_constant_right
+; CHECK-SAME: (i1 [[C:%.*]], i64 [[X:%.*]]) {
+; CHECK-NEXT:    [[SEL:%.*]] = select i1 [[C]], i64 [[X]], i64 123
+; CHECK-NEXT:    call void @use(i64 [[SEL]])
+; CHECK-NEXT:    [[EXT:%.*]] = zext i1 [[C]] to i64
+; CHECK-NEXT:    [[ADD:%.*]] = add i64 [[SEL]], [[EXT]]
+; CHECK-NEXT:    ret i64 [[ADD]]
+;
+  %sel = select i1 %c, i64 %x, i64 123
+  call void @use(i64 %sel)
+  %ext = zext i1 %c to i64
+  %add = add i64 %sel, %ext
+  ret i64 %add
+}
+
+; FIXME could fold this
+
+define i64 @multiuse_select_one_constant_left_chain(i1 %c, i64 %x) {
+; CHECK-LABEL: define i64 @multiuse_select_one_constant_left_chain
+; CHECK-SAME: (i1 [[C:%.*]], i64 [[X:%.*]]) {
+; CHECK-NEXT:    [[SEL:%.*]] = select i1 [[C]], i64 123, i64 [[X]]
+; CHECK-NEXT:    [[EXT_NEG:%.*]] = sext i1 [[C]] to i64
+; CHECK-NEXT:    [[ADD:%.*]] = add i64 [[SEL]], [[EXT_NEG]]
+; CHECK-NEXT:    [[MUL:%.*]] = mul i64 [[SEL]], [[ADD]]
+; CHECK-NEXT:    ret i64 [[MUL]]
+;
+  %sel = select i1 %c, i64 123, i64 %x
+  %ext = zext i1 %c to i64
+  %add = sub i64 %sel, %ext
+  %mul = mul i64 %sel, %add
+  ret i64 %mul
+}
+
+; FIXME could fold this
+
+define i64 @multiuse_select_one_constant_right_chain(i1 %c, i64 %x) {
+; CHECK-LABEL: define i64 @multiuse_select_one_constant_right_chain
+; CHECK-SAME: (i1 [[C:%.*]], i64 [[X:%.*]]) {
+; CHECK-NEXT:    [[SEL:%.*]] = select i1 [[C]], i64 [[X]], i64 123
+; CHECK-NEXT:    [[EXT_NEG:%.*]] = sext i1 [[C]] to i64
+; CHECK-NEXT:    [[ADD:%.*]] = add i64 [[SEL]], [[EXT_NEG]]
+; CHECK-NEXT:    [[MUL:%.*]] = mul i64 [[SEL]], [[ADD]]
+; CHECK-NEXT:    ret i64 [[MUL]]
+;
+  %sel = select i1 %c, i64 %x, i64 123
+  %ext = zext i1 %c to i64
+  %add = sub i64 %sel, %ext
+  %mul = mul i64 %sel, %add
+  ret i64 %mul
+}
+
 define i6 @sub_select_sext_op_swapped_non_const_args(i1 %c, i6 %argT, i6 %argF) {
 ; CHECK-LABEL: define i6 @sub_select_sext_op_swapped_non_const_args
 ; CHECK-SAME: (i1 [[C:%.*]], i6 [[ARGT:%.*]], i6 [[ARGF:%.*]]) {

``````````

</details>


https://github.com/llvm/llvm-project/pull/86545


More information about the llvm-commits mailing list