Skip to content

Commit 76a4c45

Browse files
rajatbajpairajatbajpai
authored
[InstCombine] Fix constant swap case of fcmp + fadd + sel xfrm (#119419)
The fcmp + fadd + sel => fcmp + sel + fadd xfrm performs incorrect transformation when select branch values are swapped. This change fixes this.
1 parent df728cf commit 76a4c45

File tree

2 files changed

+34
-26
lines changed

2 files changed

+34
-26
lines changed

‎llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp‎

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3769,22 +3769,9 @@ static Value *foldSelectIntoAddConstant(SelectInst &SI,
37693769
if (!SIFOp || !SIFOp->hasNoSignedZeros() || !SIFOp->hasNoNaNs())
37703770
return nullptr;
37713771

3772-
// select((fcmp Pred, X, 0), (fadd X, C), C)
3773-
// => fadd((select (fcmp Pred, X, 0), X, 0), C)
3774-
//
3775-
// Pred := OGT, OGE, OLT, OLE, UGT, UGE, ULT, and ULE
3776-
Instruction *FAdd;
3777-
Constant *C;
3778-
Value *X, *Z;
3779-
CmpPredicate Pred;
3780-
3781-
// Note: OneUse check for `Cmp` is necessary because it makes sure that other
3782-
// InstCombine folds don't undo this transformation and cause an infinite
3783-
// loop. Furthermore, it could also increase the operation count.
3784-
if (match(&SI, m_Select(m_OneUse(m_FCmp(Pred, m_Value(X), m_Value(Z))),
3785-
m_OneUse(m_Instruction(FAdd)), m_Constant(C))) ||
3786-
match(&SI, m_Select(m_OneUse(m_FCmp(Pred, m_Value(X), m_Value(Z))),
3787-
m_Constant(C), m_OneUse(m_Instruction(FAdd))))) {
3772+
auto TryFoldIntoAddConstant =
3773+
[&Builder, &SI](CmpInst::Predicate Pred, Value *X, Value *Z,
3774+
Instruction *FAdd, Constant *C, bool Swapped) -> Value * {
37883775
// Only these relational predicates can be transformed into maxnum/minnum
37893776
// intrinsic.
37903777
if (!CmpInst::isRelational(Pred) || !match(Z, m_AnyZeroFP()))
@@ -3793,7 +3780,8 @@ static Value *foldSelectIntoAddConstant(SelectInst &SI,
37933780
if (!match(FAdd, m_FAdd(m_Specific(X), m_Specific(C))))
37943781
return nullptr;
37953782

3796-
Value *NewSelect = Builder.CreateSelect(SI.getCondition(), X, Z, "", &SI);
3783+
Value *NewSelect = Builder.CreateSelect(SI.getCondition(), Swapped ? Z : X,
3784+
Swapped ? X : Z, "", &SI);
37973785
NewSelect->takeName(&SI);
37983786

37993787
Value *NewFAdd = Builder.CreateFAdd(NewSelect, C);
@@ -3808,7 +3796,27 @@ static Value *foldSelectIntoAddConstant(SelectInst &SI,
38083796
cast<Instruction>(NewSelect)->setFastMathFlags(NewFMF);
38093797

38103798
return NewFAdd;
3811-
}
3799+
};
3800+
3801+
// select((fcmp Pred, X, 0), (fadd X, C), C)
3802+
// => fadd((select (fcmp Pred, X, 0), X, 0), C)
3803+
//
3804+
// Pred := OGT, OGE, OLT, OLE, UGT, UGE, ULT, and ULE
3805+
Instruction *FAdd;
3806+
Constant *C;
3807+
Value *X, *Z;
3808+
CmpPredicate Pred;
3809+
3810+
// Note: OneUse check for `Cmp` is necessary because it makes sure that other
3811+
// InstCombine folds don't undo this transformation and cause an infinite
3812+
// loop. Furthermore, it could also increase the operation count.
3813+
if (match(&SI, m_Select(m_OneUse(m_FCmp(Pred, m_Value(X), m_Value(Z))),
3814+
m_OneUse(m_Instruction(FAdd)), m_Constant(C))))
3815+
return TryFoldIntoAddConstant(Pred, X, Z, FAdd, C, /*Swapped=*/false);
3816+
3817+
if (match(&SI, m_Select(m_OneUse(m_FCmp(Pred, m_Value(X), m_Value(Z))),
3818+
m_Constant(C), m_OneUse(m_Instruction(FAdd)))))
3819+
return TryFoldIntoAddConstant(Pred, X, Z, FAdd, C, /*Swapped=*/true);
38123820

38133821
return nullptr;
38143822
}

‎llvm/test/Transforms/InstCombine/fcmp-fadd-select.ll‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ define float @test_fcmp_ogt_fadd_select_constant(float %in) {
1919
define float @test_fcmp_ogt_fadd_select_constant_swapped(float %in) {
2020
; CHECK-LABEL: define float @test_fcmp_ogt_fadd_select_constant_swapped(
2121
; CHECK-SAME: float [[IN:%.*]]) {
22-
; CHECK-NEXT: [[SEL_NEW:%.*]] = call nsz float @llvm.maxnum.f32(float [[IN]], float 0.000000e+00)
22+
; CHECK-NEXT: [[SEL_NEW:%.*]] = call nsz float @llvm.minnum.f32(float [[IN]], float 0.000000e+00)
2323
; CHECK-NEXT: [[ADD_NEW:%.*]] = fadd nnan nsz float [[SEL_NEW]], 1.000000e+00
2424
; CHECK-NEXT: ret float [[ADD_NEW]]
2525
;
@@ -87,7 +87,7 @@ define float @test_fcmp_olt_fadd_select_constant(float %in) {
8787
define float @test_fcmp_olt_fadd_select_constant_swapped(float %in) {
8888
; CHECK-LABEL: define float @test_fcmp_olt_fadd_select_constant_swapped(
8989
; CHECK-SAME: float [[IN:%.*]]) {
90-
; CHECK-NEXT: [[SEL_NEW:%.*]] = call nsz float @llvm.minnum.f32(float [[IN]], float 0.000000e+00)
90+
; CHECK-NEXT: [[SEL_NEW:%.*]] = call nsz float @llvm.maxnum.f32(float [[IN]], float 0.000000e+00)
9191
; CHECK-NEXT: [[ADD_NEW:%.*]] = fadd nnan nsz float [[SEL_NEW]], 1.000000e+00
9292
; CHECK-NEXT: ret float [[ADD_NEW]]
9393
;
@@ -155,7 +155,7 @@ define float @test_fcmp_oge_fadd_select_constant(float %in) {
155155
define float @test_fcmp_oge_fadd_select_constant_swapped(float %in) {
156156
; CHECK-LABEL: define float @test_fcmp_oge_fadd_select_constant_swapped(
157157
; CHECK-SAME: float [[IN:%.*]]) {
158-
; CHECK-NEXT: [[SEL_NEW:%.*]] = call nsz float @llvm.maxnum.f32(float [[IN]], float 0.000000e+00)
158+
; CHECK-NEXT: [[SEL_NEW:%.*]] = call nsz float @llvm.minnum.f32(float [[IN]], float 0.000000e+00)
159159
; CHECK-NEXT: [[ADD_NEW:%.*]] = fadd nnan nsz float [[SEL_NEW]], 1.000000e+00
160160
; CHECK-NEXT: ret float [[ADD_NEW]]
161161
;
@@ -223,7 +223,7 @@ define float @test_fcmp_ole_fadd_select_constant(float %in) {
223223
define float @test_fcmp_ole_fadd_select_constant_swapped(float %in) {
224224
; CHECK-LABEL: define float @test_fcmp_ole_fadd_select_constant_swapped(
225225
; CHECK-SAME: float [[IN:%.*]]) {
226-
; CHECK-NEXT: [[SEL_NEW:%.*]] = call nsz float @llvm.minnum.f32(float [[IN]], float 0.000000e+00)
226+
; CHECK-NEXT: [[SEL_NEW:%.*]] = call nsz float @llvm.maxnum.f32(float [[IN]], float 0.000000e+00)
227227
; CHECK-NEXT: [[ADD_NEW:%.*]] = fadd nnan nsz float [[SEL_NEW]], 1.000000e+00
228228
; CHECK-NEXT: ret float [[ADD_NEW]]
229229
;
@@ -293,7 +293,7 @@ define float @test_fcmp_ugt_fadd_select_constant_swapped(float %in) {
293293
; CHECK-LABEL: define float @test_fcmp_ugt_fadd_select_constant_swapped(
294294
; CHECK-SAME: float [[IN:%.*]]) {
295295
; CHECK-NEXT: [[CMP1_INV:%.*]] = fcmp ole float [[IN]], 0.000000e+00
296-
; CHECK-NEXT: [[SEL_NEW:%.*]] = select i1 [[CMP1_INV]], float 0.000000e+00, float [[IN]]
296+
; CHECK-NEXT: [[SEL_NEW:%.*]] = select i1 [[CMP1_INV]], float [[IN]], float 0.000000e+00
297297
; CHECK-NEXT: [[ADD_NEW:%.*]] = fadd nnan nsz float [[SEL_NEW]], 1.000000e+00
298298
; CHECK-NEXT: ret float [[ADD_NEW]]
299299
;
@@ -366,7 +366,7 @@ define float @test_fcmp_uge_fadd_select_constant_swapped(float %in) {
366366
; CHECK-LABEL: define float @test_fcmp_uge_fadd_select_constant_swapped(
367367
; CHECK-SAME: float [[IN:%.*]]) {
368368
; CHECK-NEXT: [[CMP1_INV:%.*]] = fcmp olt float [[IN]], 0.000000e+00
369-
; CHECK-NEXT: [[SEL_NEW:%.*]] = select i1 [[CMP1_INV]], float 0.000000e+00, float [[IN]]
369+
; CHECK-NEXT: [[SEL_NEW:%.*]] = select i1 [[CMP1_INV]], float [[IN]], float 0.000000e+00
370370
; CHECK-NEXT: [[ADD_NEW:%.*]] = fadd nnan nsz float [[SEL_NEW]], 1.000000e+00
371371
; CHECK-NEXT: ret float [[ADD_NEW]]
372372
;
@@ -439,7 +439,7 @@ define float @test_fcmp_ult_fadd_select_constant_swapped(float %in) {
439439
; CHECK-LABEL: define float @test_fcmp_ult_fadd_select_constant_swapped(
440440
; CHECK-SAME: float [[IN:%.*]]) {
441441
; CHECK-NEXT: [[CMP1_INV:%.*]] = fcmp oge float [[IN]], 0.000000e+00
442-
; CHECK-NEXT: [[SEL_NEW:%.*]] = select i1 [[CMP1_INV]], float 0.000000e+00, float [[IN]]
442+
; CHECK-NEXT: [[SEL_NEW:%.*]] = select i1 [[CMP1_INV]], float [[IN]], float 0.000000e+00
443443
; CHECK-NEXT: [[ADD_NEW:%.*]] = fadd nnan nsz float [[SEL_NEW]], 1.000000e+00
444444
; CHECK-NEXT: ret float [[ADD_NEW]]
445445
;
@@ -512,7 +512,7 @@ define float @test_fcmp_ule_fadd_select_constant_swapped(float %in) {
512512
; CHECK-LABEL: define float @test_fcmp_ule_fadd_select_constant_swapped(
513513
; CHECK-SAME: float [[IN:%.*]]) {
514514
; CHECK-NEXT: [[CMP1_INV:%.*]] = fcmp ogt float [[IN]], 0.000000e+00
515-
; CHECK-NEXT: [[SEL_NEW:%.*]] = select i1 [[CMP1_INV]], float 0.000000e+00, float [[IN]]
515+
; CHECK-NEXT: [[SEL_NEW:%.*]] = select i1 [[CMP1_INV]], float [[IN]], float 0.000000e+00
516516
; CHECK-NEXT: [[ADD_NEW:%.*]] = fadd nnan nsz float [[SEL_NEW]], 1.000000e+00
517517
; CHECK-NEXT: ret float [[ADD_NEW]]
518518
;

0 commit comments

Comments
 (0)