-
Notifications
You must be signed in to change notification settings - Fork 14.4k
[InstCombine] Optimize (select %x, op(%x), 0) to op(%x) for operations where op(0) == 0 #147605
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 2 commits
31d8680
7a12f56
ef056de
e08d66e
ed88808
252d55f
09536bb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -878,7 +878,11 @@ static Instruction *foldSetClearBits(SelectInst &Sel, | |
// is a vector consisting of 0 and undefs. If a constant compared with x | ||
// is a scalar undefined value or undefined vector then an expression | ||
// should be already folded into a constant. | ||
static Instruction *foldSelectZeroOrMul(SelectInst &SI, InstCombinerImpl &IC) { | ||
// | ||
// This also holds all operations such that Op(0) == 0 | ||
// e.g. Shl, Umin, etc | ||
static Instruction *foldSelectZeroOrFixedOp(SelectInst &SI, | ||
InstCombinerImpl &IC) { | ||
auto *CondVal = SI.getCondition(); | ||
auto *TrueVal = SI.getTrueValue(); | ||
auto *FalseVal = SI.getFalseValue(); | ||
|
@@ -900,9 +904,7 @@ static Instruction *foldSelectZeroOrMul(SelectInst &SI, InstCombinerImpl &IC) { | |
// non-zero elements that are masked by undef elements in the compare | ||
// constant. | ||
auto *TrueValC = dyn_cast<Constant>(TrueVal); | ||
if (TrueValC == nullptr || | ||
!match(FalseVal, m_c_Mul(m_Specific(X), m_Value(Y))) || | ||
!isa<Instruction>(FalseVal)) | ||
if (TrueValC == nullptr || !isa<Instruction>(FalseVal)) | ||
return nullptr; | ||
|
||
auto *ZeroC = cast<Constant>(cast<Instruction>(CondVal)->getOperand(1)); | ||
|
@@ -913,11 +915,28 @@ static Instruction *foldSelectZeroOrMul(SelectInst &SI, InstCombinerImpl &IC) { | |
if (!match(MergedC, m_Zero()) && !match(MergedC, m_Undef())) | ||
return nullptr; | ||
|
||
auto *FalseValI = cast<Instruction>(FalseVal); | ||
auto *FrY = IC.InsertNewInstBefore(new FreezeInst(Y, Y->getName() + ".fr"), | ||
FalseValI->getIterator()); | ||
IC.replaceOperand(*FalseValI, FalseValI->getOperand(0) == Y ? 0 : 1, FrY); | ||
return IC.replaceInstUsesWith(SI, FalseValI); | ||
if (match(FalseVal, m_c_Mul(m_Specific(X), m_Value(Y))) || | ||
match(FalseVal, m_c_And(m_Specific(X), m_Value(Y))) || | ||
match(FalseVal, m_Shl(m_Specific(X), m_Value(Y))) || | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It doesn't work for shift operators. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, I will remove From what I can tell, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, fshl/fshr shifts are modulo the bitwidth. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If I wanted to perform this transformation on the shift operators ( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
You can check if and/mul: valid with freeze(Y) if |
||
match(FalseVal, m_AShr(m_Specific(X), m_Value(Y))) || | ||
match(FalseVal, m_LShr(m_Specific(X), m_Value(Y))) || | ||
match(FalseVal, m_FShl(m_Specific(X), m_Specific(X), m_Value(Y))) || | ||
match(FalseVal, m_FShr(m_Specific(X), m_Specific(X), m_Value(Y))) || | ||
match(FalseVal, m_SDiv(m_Specific(X), m_Value(Y))) || | ||
match(FalseVal, m_UDiv(m_Specific(X), m_Value(Y))) || | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For more context, Alive2 says the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
It is safe, even without inserting freeze. If |
||
match(FalseVal, m_c_UMin(m_Specific(X), m_Value(Y)))) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is dangerous because m_c_UMin can also match the icmp+select form. We should only allow the umin intrinsic here. |
||
auto *FalseValI = cast<Instruction>(FalseVal); | ||
auto *FrY = IC.InsertNewInstBefore(new FreezeInst(Y, Y->getName() + ".fr"), | ||
FalseValI->getIterator()); | ||
IC.replaceOperand(*FalseValI, | ||
FalseValI->getOperand(0) == Y | ||
? 0 | ||
: (FalseValI->getOperand(1) == Y ? 1 : 2), | ||
FrY); | ||
return IC.replaceInstUsesWith(SI, FalseValI); | ||
} | ||
|
||
return nullptr; | ||
} | ||
|
||
/// Transform patterns such as (a > b) ? a - b : 0 into usub.sat(a, b). | ||
|
@@ -4104,7 +4123,7 @@ Instruction *InstCombinerImpl::visitSelectInst(SelectInst &SI) { | |
return Add; | ||
if (Instruction *Or = foldSetClearBits(SI, Builder)) | ||
return Or; | ||
if (Instruction *Mul = foldSelectZeroOrMul(SI, *this)) | ||
if (Instruction *Mul = foldSelectZeroOrFixedOp(SI, *this)) | ||
return Mul; | ||
|
||
// Turn (select C, (op X, Y), (op X, Z)) -> (op X, (select C, Y, Z)) | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,170 @@ | ||||||
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py | ||||||
; RUN: opt -S -passes=instcombine < %s | FileCheck %s --check-prefix=FIXED-ZERO | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Do not specify prefix if you're not using multiple. |
||||||
|
||||||
; (select (icmp x, 0, eq), 0, (umin x, y)) -> (umin x, y) | ||||||
define i64 @umin_select(i64 %a, i64 %b) { | ||||||
; FIXED-ZERO-LABEL: @umin_select( | ||||||
; FIXED-ZERO-NEXT: [[B_FR:%.*]] = freeze i64 [[B:%.*]] | ||||||
; FIXED-ZERO-NEXT: [[UMIN:%.*]] = call i64 @llvm.umin.i64(i64 [[A:%.*]], i64 [[B_FR]]) | ||||||
; FIXED-ZERO-NEXT: ret i64 [[UMIN]] | ||||||
; | ||||||
%cond = icmp eq i64 %a, 0 | ||||||
%umin = call i64 @llvm.umin.i64(i64 %a, i64 %b) | ||||||
%select = select i1 %cond, i64 0, i64 %umin | ||||||
ret i64 %select | ||||||
} | ||||||
|
||||||
; (select (icmp x, 0, eq), 0, (mul x, y)) -> (mul x, y) | ||||||
define i64 @mul_select(i64 %a, i64 %b) { | ||||||
; FIXED-ZERO-LABEL: @mul_select( | ||||||
; FIXED-ZERO-NEXT: [[B_FR:%.*]] = freeze i64 [[B:%.*]] | ||||||
; FIXED-ZERO-NEXT: [[MUL:%.*]] = mul i64 [[A:%.*]], [[B_FR]] | ||||||
; FIXED-ZERO-NEXT: ret i64 [[MUL]] | ||||||
; | ||||||
%cond = icmp eq i64 %a, 0 | ||||||
%mul = mul i64 %a, %b | ||||||
%select = select i1 %cond, i64 0, i64 %mul | ||||||
ret i64 %select | ||||||
} | ||||||
|
||||||
; (select (icmp x, 0, eq), 0, (shl x, y)) -> (shl x, y) | ||||||
define i64 @shl_select(i64 %a, i64 %b) { | ||||||
; FIXED-ZERO-LABEL: @shl_select( | ||||||
; FIXED-ZERO-NEXT: [[B_FR:%.*]] = freeze i64 [[B:%.*]] | ||||||
; FIXED-ZERO-NEXT: [[SHL:%.*]] = shl i64 [[A:%.*]], [[B_FR]] | ||||||
; FIXED-ZERO-NEXT: ret i64 [[SHL]] | ||||||
; | ||||||
%cond = icmp eq i64 %a, 0 | ||||||
%shl = shl i64 %a, %b | ||||||
%select = select i1 %cond, i64 0, i64 %shl | ||||||
ret i64 %select | ||||||
} | ||||||
|
||||||
; (select (icmp x, 0, eq), 0, (and x, y)) -> (and x, y) | ||||||
define i64 @and_select(i64 %a, i64 %b) { | ||||||
; FIXED-ZERO-LABEL: @and_select( | ||||||
; FIXED-ZERO-NEXT: [[B_FR:%.*]] = freeze i64 [[B:%.*]] | ||||||
; FIXED-ZERO-NEXT: [[AND:%.*]] = and i64 [[A:%.*]], [[B_FR]] | ||||||
; FIXED-ZERO-NEXT: ret i64 [[AND]] | ||||||
; | ||||||
%cond = icmp eq i64 %a, 0 | ||||||
%and = and i64 %a, %b | ||||||
%select = select i1 %cond, i64 0, i64 %and | ||||||
ret i64 %select | ||||||
} | ||||||
|
||||||
; (select (icmp x, 0, ne), (ashr x, y), 0) -> (ashr x, y) | ||||||
define i64 @ashr_select(i64 %a, i64 %b) { | ||||||
; FIXED-ZERO-LABEL: @ashr_select( | ||||||
; FIXED-ZERO-NEXT: [[B_FR:%.*]] = freeze i64 [[B:%.*]] | ||||||
; FIXED-ZERO-NEXT: [[ASHR:%.*]] = ashr i64 [[A:%.*]], [[B_FR]] | ||||||
; FIXED-ZERO-NEXT: ret i64 [[ASHR]] | ||||||
; | ||||||
%cond = icmp ne i64 0, %a | ||||||
%ashr = ashr i64 %a, %b | ||||||
%select = select i1 %cond, i64 %ashr, i64 0 | ||||||
ret i64 %select | ||||||
} | ||||||
|
||||||
; (select (icmp x, 0, ne), (lshr x, y), 0) -> (lshr x, y) | ||||||
define i64 @lshr_select(i64 %a, i64 %b) { | ||||||
; FIXED-ZERO-LABEL: @lshr_select( | ||||||
; FIXED-ZERO-NEXT: [[B_FR:%.*]] = freeze i64 [[B:%.*]] | ||||||
; FIXED-ZERO-NEXT: [[LSHR:%.*]] = lshr i64 [[A:%.*]], [[B_FR]] | ||||||
; FIXED-ZERO-NEXT: ret i64 [[LSHR]] | ||||||
; | ||||||
%cond = icmp ne i64 0, %a | ||||||
%lshr = lshr i64 %a, %b | ||||||
%select = select i1 %cond, i64 %lshr, i64 0 | ||||||
ret i64 %select | ||||||
} | ||||||
|
||||||
; (select (icmp x, 0, eq), 0, fshr(x, x, y)) -> fshr(x, x, y) | ||||||
define i64 @fshr_select(i64 %a, i64 %b) { | ||||||
; FIXED-ZERO-LABEL: @fshr_select( | ||||||
; FIXED-ZERO-NEXT: [[B_FR:%.*]] = freeze i64 [[B:%.*]] | ||||||
; FIXED-ZERO-NEXT: [[FSHR:%.*]] = call i64 @llvm.fshr.i64(i64 [[A:%.*]], i64 [[A]], i64 [[B_FR]]) | ||||||
; FIXED-ZERO-NEXT: ret i64 [[FSHR]] | ||||||
; | ||||||
%cond = icmp eq i64 %a, 0 | ||||||
%fshr = call i64 @llvm.fshr.i64(i64 %a, i64 %a, i64 %b) | ||||||
%select = select i1 %cond, i64 0, i64 %fshr | ||||||
ret i64 %select | ||||||
} | ||||||
|
||||||
; (select (icmp x, 0, eq), 0, (fshl x, x, y)) -> (fshl x, x, y) | ||||||
define i64 @fshl_select(i64 %a, i64 %b) { | ||||||
; FIXED-ZERO-LABEL: @fshl_select( | ||||||
; FIXED-ZERO-NEXT: [[B_FR:%.*]] = freeze i64 [[B:%.*]] | ||||||
; FIXED-ZERO-NEXT: [[FSHL:%.*]] = call i64 @llvm.fshl.i64(i64 [[A:%.*]], i64 [[A]], i64 [[B_FR]]) | ||||||
; FIXED-ZERO-NEXT: ret i64 [[FSHL]] | ||||||
; | ||||||
%cond = icmp eq i64 %a, 0 | ||||||
%fshl = call i64 @llvm.fshl.i64(i64 %a, i64 %a, i64 %b) | ||||||
%select = select i1 %cond, i64 0, i64 %fshl | ||||||
ret i64 %select | ||||||
} | ||||||
|
||||||
; (select (icmp x, 0, eq), 0, (fshr x, z, y)) -> leave as is | ||||||
define i64 @fshr_select_no_combine(i64 %a, i64 %b, i64 %c) { | ||||||
; FIXED-ZERO-LABEL: @fshr_select_no_combine( | ||||||
; FIXED-ZERO-NEXT: [[COND:%.*]] = icmp eq i64 [[A:%.*]], 0 | ||||||
; FIXED-ZERO-NEXT: [[FSHR:%.*]] = call i64 @llvm.fshr.i64(i64 [[A]], i64 [[B:%.*]], i64 [[C:%.*]]) | ||||||
; FIXED-ZERO-NEXT: [[SELECT:%.*]] = select i1 [[COND]], i64 0, i64 [[FSHR]] | ||||||
; FIXED-ZERO-NEXT: ret i64 [[SELECT]] | ||||||
; | ||||||
%cond = icmp eq i64 %a, 0 | ||||||
%fshr = call i64 @llvm.fshr.i64(i64 %a, i64 %b, i64 %c) | ||||||
%select = select i1 %cond, i64 0, i64 %fshr | ||||||
ret i64 %select | ||||||
} | ||||||
|
||||||
; (select (icmp x, 0, eq), 0, (sdiv x, y)) -> (sdiv x, y) | ||||||
define i64 @sdiv_select(i64 %a, i64 %b) { | ||||||
; FIXED-ZERO-LABEL: @sdiv_select( | ||||||
; FIXED-ZERO-NEXT: [[B:%.*]] = freeze i64 [[B1:%.*]] | ||||||
; FIXED-ZERO-NEXT: [[DIV:%.*]] = sdiv i64 [[A:%.*]], [[B]] | ||||||
; FIXED-ZERO-NEXT: ret i64 [[DIV]] | ||||||
; | ||||||
%cond = icmp eq i64 %a, 0 | ||||||
%div = sdiv i64 %a, %b | ||||||
%select = select i1 %cond, i64 0, i64 %div | ||||||
ret i64 %select | ||||||
} | ||||||
|
||||||
; (select (icmp x, 0, eq), 0, (udiv x, y)) -> (udiv x, y) | ||||||
define i64 @udiv_select(i64 %a, i64 %b) { | ||||||
; FIXED-ZERO-LABEL: @udiv_select( | ||||||
; FIXED-ZERO-NEXT: [[B:%.*]] = freeze i64 [[B1:%.*]] | ||||||
; FIXED-ZERO-NEXT: [[DIV:%.*]] = udiv i64 [[A:%.*]], [[B]] | ||||||
; FIXED-ZERO-NEXT: ret i64 [[DIV]] | ||||||
; | ||||||
%cond = icmp eq i64 %a, 0 | ||||||
%div = udiv i64 %a, %b | ||||||
%select = select i1 %cond, i64 0, i64 %div | ||||||
ret i64 %select | ||||||
} | ||||||
|
||||||
; (select (icmp x, 0, eq), 0, (icmp x, 0, slt)) -> (icmp x, 0, slt) | ||||||
define i1 @icmp_slt_select(i64 %a) { | ||||||
; FIXED-ZERO-LABEL: @icmp_slt_select( | ||||||
; FIXED-ZERO-NEXT: [[ICMP:%.*]] = icmp slt i64 [[A:%.*]], 0 | ||||||
; FIXED-ZERO-NEXT: ret i1 [[ICMP]] | ||||||
; | ||||||
%cond = icmp eq i64 %a, 0 | ||||||
%icmp = icmp slt i64 %a, 0 | ||||||
%select = select i1 %cond, i1 0, i1 %icmp | ||||||
ret i1 %select | ||||||
} | ||||||
|
||||||
; (select (icmp x, 0, eq), 0, (sub 0, x)) -> (sub 0, x) | ||||||
define i64 @sub_select(i64 %a) { | ||||||
; FIXED-ZERO-LABEL: @sub_select( | ||||||
; FIXED-ZERO-NEXT: [[SUB:%.*]] = sub i64 0, [[A:%.*]] | ||||||
; FIXED-ZERO-NEXT: ret i64 [[SUB]] | ||||||
; | ||||||
%cond = icmp eq i64 %a, 0 | ||||||
%sub = sub i64 0, %a | ||||||
%select = select i1 %cond, i64 0, i64 %sub | ||||||
ret i64 %select | ||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Crash reproducer:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks.
The issue was that I moved the pattern match to the end of the function. However, one side effect of the pattern match is that it guarantees that the types of TrueV and the Conditional constant Match which is assumed to be true by:
Reordering to perform the pattern matching where it was before this MR solved this issue.