Skip to content

[InstCombine] Fix behavior for (fmul (sitfp x), 0) #85298

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

Closed
wants to merge 2 commits into from
Closed

[InstCombine] Fix behavior for (fmul (sitfp x), 0) #85298

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6451085
[InstCombine] Add test for `(fmul (sitfp x), 0)`; NFC
goldsteinn Mar 14, 2024
5a0e73d
[InstCombine] Fix behavior for `(fmul (sitfp x), 0)`
goldsteinn Mar 14, 2024
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
5 changes: 5 additions & 0 deletions llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1481,6 +1481,11 @@ Instruction *InstCombinerImpl::foldFBinOpOfIntCastsFromSign(

// If we have a constant rhs, see if we can losslessly convert it to an int.
if (Op1FpC != nullptr) {
// Signed + Mul req non-zero
if (OpsFromSigned && BO.getOpcode() == Instruction::FMul &&
!match(Op1FpC, m_NonZeroFP()))
return nullptr;

Constant *Op1IntC = ConstantFoldCastOperand(
OpsFromSigned ? Instruction::FPToSI : Instruction::FPToUI, Op1FpC,
IntTy, DL);
Expand Down
120 changes: 120 additions & 0 deletions llvm/test/Transforms/InstCombine/binop-itofp.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1004,3 +1004,123 @@ define float @test_ui_add_with_signed_constant(i32 %shr.i) {
%add = fadd float %sub, -16383.0
ret float %add
}


;; Reduced form of bug noticed due to #82555
define float @missed_nonzero_check_on_constant_for_si_fmul(i1 %c, i1 %.b, ptr %g_2345) {
; CHECK-LABEL: @missed_nonzero_check_on_constant_for_si_fmul(
; CHECK-NEXT: [[SEL:%.*]] = select i1 [[C:%.*]], i32 65529, i32 53264
; CHECK-NEXT: [[CONV_I:%.*]] = trunc i32 [[SEL]] to i16
; CHECK-NEXT: [[CONV1_I:%.*]] = sitofp i16 [[CONV_I]] to float
; CHECK-NEXT: [[MUL3_I_I:%.*]] = fmul float [[CONV1_I]], 0.000000e+00
; CHECK-NEXT: store i32 [[SEL]], ptr [[G_2345:%.*]], align 4
; CHECK-NEXT: ret float [[MUL3_I_I]]
;
%sel = select i1 %c, i32 65529, i32 53264
%conv.i = trunc i32 %sel to i16
%conv1.i = sitofp i16 %conv.i to float
%mul3.i.i = fmul float %conv1.i, 0.000000e+00
store i32 %sel, ptr %g_2345, align 4
ret float %mul3.i.i
}

define <2 x float> @missed_nonzero_check_on_constant_for_si_fmul_vec(i1 %c, i1 %.b, ptr %g_2345) {
; CHECK-LABEL: @missed_nonzero_check_on_constant_for_si_fmul_vec(
; CHECK-NEXT: [[SEL:%.*]] = select i1 [[C:%.*]], i32 65529, i32 53264
; CHECK-NEXT: [[CONV_I_S:%.*]] = trunc i32 [[SEL]] to i16
; CHECK-NEXT: [[CONV_I_V:%.*]] = insertelement <2 x i16> poison, i16 [[CONV_I_S]], i64 0
; CHECK-NEXT: [[CONV_I:%.*]] = shufflevector <2 x i16> [[CONV_I_V]], <2 x i16> poison, <2 x i32> zeroinitializer
; CHECK-NEXT: [[CONV1_I:%.*]] = sitofp <2 x i16> [[CONV_I]] to <2 x float>
; CHECK-NEXT: [[MUL3_I_I:%.*]] = fmul <2 x float> [[CONV1_I]], zeroinitializer
; CHECK-NEXT: store i32 [[SEL]], ptr [[G_2345:%.*]], align 4
; CHECK-NEXT: ret <2 x float> [[MUL3_I_I]]
;
%sel = select i1 %c, i32 65529, i32 53264
%conv.i.s = trunc i32 %sel to i16
%conv.i.v = insertelement <2 x i16> poison, i16 %conv.i.s, i64 0
%conv.i = insertelement <2 x i16> %conv.i.v, i16 %conv.i.s, i64 1
%conv1.i = sitofp <2 x i16> %conv.i to <2 x float>
%mul3.i.i = fmul <2 x float> %conv1.i, zeroinitializer
store i32 %sel, ptr %g_2345, align 4
ret <2 x float> %mul3.i.i
}

define float @negzero_check_on_constant_for_si_fmul(i1 %c, i1 %.b, ptr %g_2345) {
; CHECK-LABEL: @negzero_check_on_constant_for_si_fmul(
; CHECK-NEXT: [[SEL:%.*]] = select i1 [[C:%.*]], i32 65529, i32 53264
; CHECK-NEXT: [[CONV_I:%.*]] = trunc i32 [[SEL]] to i16
; CHECK-NEXT: [[CONV1_I:%.*]] = sitofp i16 [[CONV_I]] to float
; CHECK-NEXT: [[MUL3_I_I:%.*]] = fmul float [[CONV1_I]], -0.000000e+00
; CHECK-NEXT: store i32 [[SEL]], ptr [[G_2345:%.*]], align 4
; CHECK-NEXT: ret float [[MUL3_I_I]]
;
%sel = select i1 %c, i32 65529, i32 53264
%conv.i = trunc i32 %sel to i16
%conv1.i = sitofp i16 %conv.i to float
%mul3.i.i = fmul float %conv1.i, -0.000000e+00
store i32 %sel, ptr %g_2345, align 4
ret float %mul3.i.i
}

define <2 x float> @nonzero_check_on_constant_for_si_fmul_vec_w_undef(i1 %c, i1 %.b, ptr %g_2345) {
; CHECK-LABEL: @nonzero_check_on_constant_for_si_fmul_vec_w_undef(
; CHECK-NEXT: [[SEL:%.*]] = select i1 [[C:%.*]], i32 65529, i32 53264
; CHECK-NEXT: [[CONV_I_S:%.*]] = trunc i32 [[SEL]] to i16
; CHECK-NEXT: [[CONV_I_V:%.*]] = insertelement <2 x i16> poison, i16 [[CONV_I_S]], i64 0
; CHECK-NEXT: [[CONV_I:%.*]] = shufflevector <2 x i16> [[CONV_I_V]], <2 x i16> poison, <2 x i32> zeroinitializer
; CHECK-NEXT: [[CONV1_I:%.*]] = sitofp <2 x i16> [[CONV_I]] to <2 x float>
; CHECK-NEXT: [[MUL3_I_I:%.*]] = fmul <2 x float> [[CONV1_I]], <float undef, float 0.000000e+00>
; CHECK-NEXT: store i32 [[SEL]], ptr [[G_2345:%.*]], align 4
; CHECK-NEXT: ret <2 x float> [[MUL3_I_I]]
;
%sel = select i1 %c, i32 65529, i32 53264
%conv.i.s = trunc i32 %sel to i16
%conv.i.v = insertelement <2 x i16> poison, i16 %conv.i.s, i64 0
%conv.i = insertelement <2 x i16> %conv.i.v, i16 %conv.i.s, i64 1
%conv1.i = sitofp <2 x i16> %conv.i to <2 x float>
%mul3.i.i = fmul <2 x float> %conv1.i, <float undef, float 0.000000e+00>
store i32 %sel, ptr %g_2345, align 4
ret <2 x float> %mul3.i.i
}

define <2 x float> @nonzero_check_on_constant_for_si_fmul_nz_vec_w_undef(i1 %c, i1 %.b, ptr %g_2345) {
; CHECK-LABEL: @nonzero_check_on_constant_for_si_fmul_nz_vec_w_undef(
; CHECK-NEXT: [[SEL:%.*]] = select i1 [[C:%.*]], i32 65529, i32 53264
; CHECK-NEXT: [[CONV_I_S:%.*]] = trunc i32 [[SEL]] to i16
; CHECK-NEXT: [[CONV_I_V:%.*]] = insertelement <2 x i16> poison, i16 [[CONV_I_S]], i64 0
; CHECK-NEXT: [[CONV_I:%.*]] = shufflevector <2 x i16> [[CONV_I_V]], <2 x i16> poison, <2 x i32> zeroinitializer
; CHECK-NEXT: [[CONV1_I:%.*]] = sitofp <2 x i16> [[CONV_I]] to <2 x float>
; CHECK-NEXT: [[MUL3_I_I:%.*]] = fmul <2 x float> [[CONV1_I]], <float undef, float 1.000000e+00>
; CHECK-NEXT: store i32 [[SEL]], ptr [[G_2345:%.*]], align 4
; CHECK-NEXT: ret <2 x float> [[MUL3_I_I]]
;
%sel = select i1 %c, i32 65529, i32 53264
%conv.i.s = trunc i32 %sel to i16
%conv.i.v = insertelement <2 x i16> poison, i16 %conv.i.s, i64 0
%conv.i = insertelement <2 x i16> %conv.i.v, i16 %conv.i.s, i64 1
%conv1.i = sitofp <2 x i16> %conv.i to <2 x float>
%mul3.i.i = fmul <2 x float> %conv1.i, <float undef, float 1.000000e+00>
store i32 %sel, ptr %g_2345, align 4
ret <2 x float> %mul3.i.i
}

define <2 x float> @nonzero_check_on_constant_for_si_fmul_negz_vec_w_undef(i1 %c, i1 %.b, ptr %g_2345) {
; CHECK-LABEL: @nonzero_check_on_constant_for_si_fmul_negz_vec_w_undef(
; CHECK-NEXT: [[SEL:%.*]] = select i1 [[C:%.*]], i32 65529, i32 53264
; CHECK-NEXT: [[CONV_I_S:%.*]] = trunc i32 [[SEL]] to i16
; CHECK-NEXT: [[CONV_I_V:%.*]] = insertelement <2 x i16> poison, i16 [[CONV_I_S]], i64 0
; CHECK-NEXT: [[CONV_I:%.*]] = shufflevector <2 x i16> [[CONV_I_V]], <2 x i16> poison, <2 x i32> zeroinitializer
; CHECK-NEXT: [[CONV1_I:%.*]] = sitofp <2 x i16> [[CONV_I]] to <2 x float>
; CHECK-NEXT: [[MUL3_I_I:%.*]] = fmul <2 x float> [[CONV1_I]], <float undef, float -0.000000e+00>
; CHECK-NEXT: store i32 [[SEL]], ptr [[G_2345:%.*]], align 4
; CHECK-NEXT: ret <2 x float> [[MUL3_I_I]]
;
%sel = select i1 %c, i32 65529, i32 53264
%conv.i.s = trunc i32 %sel to i16
%conv.i.v = insertelement <2 x i16> poison, i16 %conv.i.s, i64 0
%conv.i = insertelement <2 x i16> %conv.i.v, i16 %conv.i.s, i64 1
%conv1.i = sitofp <2 x i16> %conv.i to <2 x float>
%mul3.i.i = fmul <2 x float> %conv1.i, <float undef, float -0.000000e+00>
store i32 %sel, ptr %g_2345, align 4
ret <2 x float> %mul3.i.i
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This test does not look minimal.

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 is from llvm-reduce.

The volatile store/ptrs where necessary to hit the bug (tested). Think the issue is fmul x, 0.0 tends to be simplified on its own.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can't you reduce it to at least this?

define float @missed_nonzero_check_on_constant_for_si_fmul(i1 %c, i1 %.b, ptr %g_2345) {
  %sel = select i1 %c, i32 65529, i32 53264
  %conv.i = trunc i32 %sel to i16
  %conv1.i = sitofp i16 %conv.i to float
  %mul3.i.i = fmul float %conv1.i, 0.000000e+00
  store i32 %sel, ptr %g_2345, align 4
  ret float %mul3.i.i
}

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 test.