Skip to content

[InstCombine] Ensure Safe Handling of Flags in foldFNegIntoConstant #94148

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Jun 12, 2025
Merged

[InstCombine] Ensure Safe Handling of Flags in foldFNegIntoConstant #94148

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e69cbb4
[InstCombine] Ensure Safe Handling of Flags in foldFNegIntoConstant
SahilPatidar May 31, 2024
9973683
Fixes logic and update tests
SahilPatidar Jun 7, 2024
12ab999
Propagate the `FNegOp` FMF flag instead of `I`.
SahilPatidar Jul 9, 2024
c47dc0a
Update Flag Propagation Logic
SahilPatidar Aug 23, 2024
0aff15a
Refactor logic and add additional tests
SahilPatidar Sep 27, 2024
def9464
Add helper function for flag rewriting
SahilPatidar Oct 3, 2024
699be90
Update test
SahilPatidar Mar 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2808,8 +2808,14 @@ static Instruction *foldFNegIntoConstant(Instruction &I, const DataLayout &DL) {
// Fold negation into constant operand.
// -(X * C) --> X * (-C)
if (match(FNegOp, m_FMul(m_Value(X), m_Constant(C))))
if (Constant *NegC = ConstantFoldUnaryOpOperand(Instruction::FNeg, C, DL))
return BinaryOperator::CreateFMulFMF(X, NegC, &I);
if (Constant *NegC = ConstantFoldUnaryOpOperand(Instruction::FNeg, C, DL)) {
FastMathFlags FNegF = I.getFastMathFlags();
FastMathFlags OpF = FNegOp->getFastMathFlags();
FastMathFlags FMF = FastMathFlags::unionValue(FNegF, OpF) |
FastMathFlags::intersectRewrite(FNegF, OpF);
FMF.setNoInfs(FNegF.noInfs() && OpF.noInfs());
return BinaryOperator::CreateFMulFMF(X, NegC, FMF);
}
// -(X / C) --> X / (-C)
if (match(FNegOp, m_FDiv(m_Value(X), m_Constant(C))))
if (Constant *NegC = ConstantFoldUnaryOpOperand(Instruction::FNeg, C, DL))
Expand Down
166 changes: 164 additions & 2 deletions llvm/test/Transforms/InstCombine/fneg.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ define float @fmul_fneg(float %x) {

define float @fmul_fsub_fmf(float %x) {
; CHECK-LABEL: @fmul_fsub_fmf(
; CHECK-NEXT: [[R:%.*]] = fmul reassoc nsz float [[X:%.*]], -4.200000e+01
; CHECK-NEXT: [[R:%.*]] = fmul nsz float [[X:%.*]], -4.200000e+01
; CHECK-NEXT: ret float [[R]]
;
%m = fmul float %x, 42.0
Expand All @@ -52,7 +52,7 @@ define float @fmul_fsub_fmf(float %x) {

define float @fmul_fneg_fmf(float %x) {
; CHECK-LABEL: @fmul_fneg_fmf(
; CHECK-NEXT: [[R:%.*]] = fmul reassoc nsz float [[X:%.*]], -4.200000e+01
; CHECK-NEXT: [[R:%.*]] = fmul nsz float [[X:%.*]], -4.200000e+01
; CHECK-NEXT: ret float [[R]]
;
%m = fmul float %x, 42.0
Expand Down Expand Up @@ -1142,4 +1142,166 @@ define <vscale x 2 x double> @test_fneg_select_svec_3(<vscale x 2 x i1> %cond, <
ret <vscale x 2 x double> %2
}

define float @test_fneg_ninf_mul_with_anyzero(float %a) {
; CHECK-LABEL: @test_fneg_ninf_mul_with_anyzero(
; CHECK-NEXT: [[F:%.*]] = fmul float [[A:%.*]], -0.000000e+00
; CHECK-NEXT: ret float [[F]]
;
%mul = fmul float %a, 0.0
%f = fneg ninf float %mul
ret float %f
}

define float @test_fsub_ninf_mul_with_anyzero(float %a) {
; CHECK-LABEL: @test_fsub_ninf_mul_with_anyzero(
; CHECK-NEXT: [[F2:%.*]] = fmul nsz float [[A:%.*]], -0.000000e+00
; CHECK-NEXT: ret float [[F2]]
;
%f1 = fmul nsz float %a, 0.000000
%f2 = fsub ninf float -0.000000, %f1
ret float %f2
}

define float @test_fneg_nnan_mul_with_anyzero(float %a) {
; CHECK-LABEL: @test_fneg_nnan_mul_with_anyzero(
; CHECK-NEXT: [[TMP1:%.*]] = fneg nnan float [[A:%.*]]
; CHECK-NEXT: [[F2:%.*]] = call nnan float @llvm.copysign.f32(float 0.000000e+00, float [[TMP1]])
; CHECK-NEXT: ret float [[F2]]
;
%f1 = fmul ninf float %a, 0.000000
%f2 = fneg nnan float %f1
ret float %f2
}

define float @test_fneg_nsz_mul_with_anyzero(float %a) {
; CHECK-LABEL: @test_fneg_nsz_mul_with_anyzero(
; CHECK-NEXT: [[F2:%.*]] = fmul nsz float [[A:%.*]], -0.000000e+00
; CHECK-NEXT: ret float [[F2]]
;
%f1 = fmul ninf float %a, 0.000000
%f2 = fneg nsz float %f1
ret float %f2
}

define float @test_fneg_ninf_mul_nnan_with_const(float %a) {
; CHECK-LABEL: @test_fneg_ninf_mul_nnan_with_const(
; CHECK-NEXT: [[TMP1:%.*]] = fneg float [[A:%.*]]
; CHECK-NEXT: [[F2:%.*]] = call float @llvm.copysign.f32(float 0.000000e+00, float [[TMP1]])
; CHECK-NEXT: ret float [[F2]]
;
%f1 = fmul nnan float %a, 0.000000
%f2 = fneg ninf float %f1
ret float %f2
}

define float @test_fneg_ninf_mul_nsz_with_const(float %a) {
; CHECK-LABEL: @test_fneg_ninf_mul_nsz_with_const(
; CHECK-NEXT: [[F2:%.*]] = fmul nsz float [[A:%.*]], -0.000000e+00
; CHECK-NEXT: ret float [[F2]]
;
%f1 = fmul nsz float %a, 0.000000
%f2 = fneg ninf float %f1
ret float %f2
}

define <2 x float> @test_fneg_mul_combine_nnan_ninf_with_vec_const(<2 x float> %a) {
; CHECK-LABEL: @test_fneg_mul_combine_nnan_ninf_with_vec_const(
; CHECK-NEXT: [[F2:%.*]] = fmul nnan <2 x float> [[A:%.*]], <float -0.000000e+00, float 0.000000e+00>
; CHECK-NEXT: ret <2 x float> [[F2]]
;
%f1 = fmul nnan <2 x float> %a, <float 0.000000, float -0.000000>
%f2 = fneg ninf <2 x float> %f1
ret <2 x float> %f2
}

define <2 x float> @test_fneg_mul_combine_nsz_ninf_with_vec_const(<2 x float> %a) {
; CHECK-LABEL: @test_fneg_mul_combine_nsz_ninf_with_vec_const(
; CHECK-NEXT: [[F2:%.*]] = fmul nsz <2 x float> [[A:%.*]], <float -0.000000e+00, float 0.000000e+00>
; CHECK-NEXT: ret <2 x float> [[F2]]
;
%f1 = fmul nsz <2 x float> %a, <float 0.000000, float -0.000000>
%f2 = fneg ninf <2 x float> %f1
ret <2 x float> %f2
}

define <2 x float> @test_fneg_ninf_nnan_mul_with_vec_const(<2 x float> %a) {
; CHECK-LABEL: @test_fneg_ninf_nnan_mul_with_vec_const(
; CHECK-NEXT: [[F2:%.*]] = fmul nnan <2 x float> [[A:%.*]], <float -0.000000e+00, float 0.000000e+00>
; CHECK-NEXT: ret <2 x float> [[F2]]
;
%f1 = fmul <2 x float> %a, <float 0.000000, float -0.000000>
%f2 = fneg nnan ninf <2 x float> %f1
ret <2 x float> %f2
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing a test where ninf is present on both, and is preservable

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me know if we need more tests of that kind?.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ninf combinations are most important because you need to special case them

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should also test reassoc on one, other, and both instructions. Best to require it be present on both instructions

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arsenm We require the reassoc flag to be present on both instructions before propagating it, but only if noinf is also present, right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, there is no direct connection between reassoc and ninf. They both separately are anded

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change causes the test to fail:

; With nsz and reassoc: Y - ((X * 5) + Y) --> X * -5

define float @sub_add_neg_x(float %x, float %y) {
; CHECK-LABEL: @sub_add_neg_x(
; CHECK-NEXT:    [[R:%.*]] = fmul nsz float [[X:%.*]], -5.000000e+00
; CHECK-NEXT:    ret float [[R]]
;
  %mul = fmul float %x, 5.000000e+00
  %add = fadd float %mul, %y
  %r = fsub nsz reassoc float %y, %add
  ret float %r
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fail, meaning it now preserves nsz and not the reassoc? That's fine. That is more in line with what the LangRef states the rules are for reassoc: https://llvm.org/docs/LangRef.html#rewrite-based-flags

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

; With nsz and reassoc: Y - ((X * 5) + Y) --> X * -5

That transformation should require nnan or isKnownNeverNaN(Y)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Somewhere else's bug though

define <2 x float> @test_fneg_mul_combine_nnan_ninf_with_vec_const2(<2 x float> %a) {
; CHECK-LABEL: @test_fneg_mul_combine_nnan_ninf_with_vec_const2(
; CHECK-NEXT: [[F2:%.*]] = fmul nnan ninf <2 x float> [[A:%.*]], <float -0.000000e+00, float 0.000000e+00>
; CHECK-NEXT: ret <2 x float> [[F2]]
;
%f1 = fmul ninf <2 x float> %a, <float 0.000000, float -0.000000>
%f2 = fneg nnan ninf <2 x float> %f1
ret <2 x float> %f2
Comment on lines +1242 to +1244
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have tested the ninf preservation case above with the scalar case, but this covers it

}

define <2 x float> @test_fneg_mul_combine_reassoc_ninf_with_vec_const1(<2 x float> %a) {
; CHECK-LABEL: @test_fneg_mul_combine_reassoc_ninf_with_vec_const1(
; CHECK-NEXT: [[F2:%.*]] = fmul <2 x float> [[A:%.*]], <float -0.000000e+00, float 0.000000e+00>
; CHECK-NEXT: ret <2 x float> [[F2]]
;
%f1 = fmul reassoc <2 x float> %a, <float 0.000000, float -0.000000>
%f2 = fneg ninf <2 x float> %f1
ret <2 x float> %f2
}

define <2 x float> @test_fneg_mul_combine_reassoc_ninf_with_vec_const2(<2 x float> %a) {
; CHECK-LABEL: @test_fneg_mul_combine_reassoc_ninf_with_vec_const2(
; CHECK-NEXT: [[F2:%.*]] = fmul ninf <2 x float> [[A:%.*]], <float -0.000000e+00, float 0.000000e+00>
; CHECK-NEXT: ret <2 x float> [[F2]]
;
%f1 = fmul ninf <2 x float> %a, <float 0.000000, float -0.000000>
%f2 = fneg reassoc ninf <2 x float> %f1
ret <2 x float> %f2
}

define <2 x float> @test_fneg_mul_combine_reassoc_ninf_with_vec_const3(<2 x float> %a) {
; CHECK-LABEL: @test_fneg_mul_combine_reassoc_ninf_with_vec_const3(
; CHECK-NEXT: [[F2:%.*]] = fmul reassoc <2 x float> [[A:%.*]], <float -0.000000e+00, float 0.000000e+00>
; CHECK-NEXT: ret <2 x float> [[F2]]
;
%f1 = fmul reassoc <2 x float> %a, <float 0.000000, float -0.000000>
%f2 = fneg reassoc ninf <2 x float> %f1
ret <2 x float> %f2
}

define <2 x float> @test_fneg_mul_combine_contract_ninf_with_vec_const1(<2 x float> %a) {
; CHECK-LABEL: @test_fneg_mul_combine_contract_ninf_with_vec_const1(
; CHECK-NEXT: [[F2:%.*]] = fmul <2 x float> [[A:%.*]], <float -0.000000e+00, float 0.000000e+00>
; CHECK-NEXT: ret <2 x float> [[F2]]
;
%f1 = fmul contract <2 x float> %a, <float 0.000000, float -0.000000>
%f2 = fneg ninf <2 x float> %f1
ret <2 x float> %f2
}

define <2 x float> @test_fneg_mul_combine_contract_ninf_with_vec_const2(<2 x float> %a) {
; CHECK-LABEL: @test_fneg_mul_combine_contract_ninf_with_vec_const2(
; CHECK-NEXT: [[F2:%.*]] = fmul ninf <2 x float> [[A:%.*]], <float -0.000000e+00, float 0.000000e+00>
; CHECK-NEXT: ret <2 x float> [[F2]]
;
%f1 = fmul ninf <2 x float> %a, <float 0.000000, float -0.000000>
%f2 = fneg contract ninf <2 x float> %f1
ret <2 x float> %f2
}

define <2 x float> @test_fneg_mul_combine_contract_ninf_with_vec_const3(<2 x float> %a) {
; CHECK-LABEL: @test_fneg_mul_combine_contract_ninf_with_vec_const3(
; CHECK-NEXT: [[F2:%.*]] = fmul contract <2 x float> [[A:%.*]], <float -0.000000e+00, float 0.000000e+00>
; CHECK-NEXT: ret <2 x float> [[F2]]
;
%f1 = fmul contract <2 x float> %a, <float 0.000000, float -0.000000>
%f2 = fneg contract ninf <2 x float> %f1
ret <2 x float> %f2
}

!0 = !{}
2 changes: 1 addition & 1 deletion llvm/test/Transforms/InstCombine/fsub.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ define float @sub_sub_nsz(float %x, float %y, float %z) {

define float @sub_add_neg_x(float %x, float %y) {
; CHECK-LABEL: @sub_add_neg_x(
; CHECK-NEXT: [[R:%.*]] = fmul reassoc nsz float [[X:%.*]], -5.000000e+00
; CHECK-NEXT: [[R:%.*]] = fmul nsz float [[X:%.*]], -5.000000e+00
; CHECK-NEXT: ret float [[R]]
;
%mul = fmul float %x, 5.000000e+00
Expand Down