[llvm] [InstCombine] Restrict multiuse in foldBinOpOfSelectAndCastOfSelectCondition (PR #86545)
via llvm-commits
llvm-commits at lists.llvm.org
Mon Mar 25 10:40:09 PDT 2024
Fabian =?utf-8?q?Hügel?= <fabian.huegel at fau.de>
Message-ID: <llvm.org/llvm/llvm-project/pull/86545 at github.com>
In-Reply-To:
https://github.com/gwastar created https://github.com/llvm/llvm-project/pull/86545
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
>From 12d67ad723bf0eafd9018557557d84fe066fb293 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabian=20H=C3=BCgel?= <fabian.huegel at fau.de>
Date: Sun, 24 Mar 2024 13:07:48 +0100
Subject: [PATCH 1/2] [InstCombine][NFC] Add more multiuse tests for
foldBinOpOfSelectAndCastOfSelectCondition
There is an existing multiuse_select test but it uses constants in the
select arms. The new multiuse_select_non_const test does not use
constants and shows that the fold is not profitable in this case.
Also add some test cases with one constant and one non-constant arm,
as well as test cases which use the select multiple times in a chain
of binops.
---
.../binop-select-cast-of-select-cond.ll | 94 +++++++++++++++++++
1 file changed, 94 insertions(+)
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..d138013da5b770 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,100 @@ 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: [[TMP1:%.*]] = add i64 [[X]], 1
+; CHECK-NEXT: [[ADD:%.*]] = select i1 [[C]], i64 [[TMP1]], i64 [[Y]]
+; 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: [[TMP1:%.*]] = add i64 [[X]], 1
+; CHECK-NEXT: [[ADD:%.*]] = select i1 [[C]], i64 [[TMP1]], i64 [[Y]]
+; 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: [[ADD:%.*]] = select i1 [[C]], i64 124, i64 [[X]]
+; 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: [[TMP1:%.*]] = add i64 [[X]], 1
+; CHECK-NEXT: [[ADD:%.*]] = select i1 [[C]], i64 [[TMP1]], i64 123
+; 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
+}
+
+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: [[TMP1:%.*]] = mul i64 [[X]], [[X]]
+; CHECK-NEXT: [[MUL:%.*]] = select i1 [[C]], i64 15006, i64 [[TMP1]]
+; 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
+}
+
+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: [[TMP1:%.*]] = add i64 [[X]], -1
+; CHECK-NEXT: [[TMP2:%.*]] = mul i64 [[TMP1]], [[X]]
+; CHECK-NEXT: [[MUL:%.*]] = select i1 [[C]], i64 [[TMP2]], i64 15129
+; 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:%.*]]) {
>From 88b39689c681e74f4332f1a083680695ded05e8b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabian=20H=C3=BCgel?= <fabian.huegel at fau.de>
Date: Mon, 25 Mar 2024 15:38:03 +0100
Subject: [PATCH 2/2] [InstCombine] Restrict multiuse in
foldBinOpOfSelectAndCastOfSelectCondition
This fold currently allows multiuse selects which is not profitable as the
new multiuse_select_non_const test shows. So don't allow multiuse selects
unless both arms of the select are constant. This way the existing
multiuse_select test, which uses constants, does not regress. However,
some of the new tests with only one constant select arm show some
regressions (especially the ones where the select is used in a chain of
binops), but it doesn't seem worth trying to handle these cases.
---
.../InstCombine/InstructionCombining.cpp | 14 +++++---
.../binop-select-cast-of-select-cond.ll | 32 ++++++++++++-------
2 files changed, 30 insertions(+), 16 deletions(-)
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 d138013da5b770..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
@@ -191,8 +191,8 @@ define i64 @multiuse_select_non_const(i1 %c, i64 %x, i64 %y) {
; 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: [[TMP1:%.*]] = add i64 [[X]], 1
-; CHECK-NEXT: [[ADD:%.*]] = select i1 [[C]], i64 [[TMP1]], i64 [[Y]]
+; 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
@@ -206,8 +206,8 @@ 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: [[TMP1:%.*]] = add i64 [[X]], 1
-; CHECK-NEXT: [[ADD:%.*]] = select i1 [[C]], i64 [[TMP1]], 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]]
;
@@ -223,7 +223,8 @@ define i64 @multiuse_select_one_constant_left(i1 %c, i64 %x) {
; 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: [[ADD:%.*]] = select i1 [[C]], i64 124, i64 [[X]]
+; 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
@@ -238,8 +239,8 @@ define i64 @multiuse_select_one_constant_right(i1 %c, i64 %x) {
; 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: [[TMP1:%.*]] = add i64 [[X]], 1
-; CHECK-NEXT: [[ADD:%.*]] = select i1 [[C]], i64 [[TMP1]], i64 123
+; 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
@@ -249,11 +250,15 @@ define i64 @multiuse_select_one_constant_right(i1 %c, i64 %x) {
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: [[TMP1:%.*]] = mul i64 [[X]], [[X]]
-; CHECK-NEXT: [[MUL:%.*]] = select i1 [[C]], i64 15006, i64 [[TMP1]]
+; 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
@@ -263,12 +268,15 @@ define i64 @multiuse_select_one_constant_left_chain(i1 %c, i64 %x) {
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: [[TMP1:%.*]] = add i64 [[X]], -1
-; CHECK-NEXT: [[TMP2:%.*]] = mul i64 [[TMP1]], [[X]]
-; CHECK-NEXT: [[MUL:%.*]] = select i1 [[C]], i64 [[TMP2]], i64 15129
+; 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
More information about the llvm-commits
mailing list