Skip to content

Commit 862e35e

Browse files
rotaterightrotateright
committed
[InstCombine] preserve signbit semantics of NAN with fold to fabs
As discussed in issue #59279, we want fneg/fabs to conform to the IEEE-754 spec for signbit operations - quoting from section 5.5.1 of IEEE-754-2008: "negate(x) copies a floating-point operand x to a destination in the same format, reversing the sign bit" "abs(x) copies a floating-point operand x to a destination in the same format, setting the sign bit to 0 (positive)" "The operations treat floating-point numbers and NaNs alike." So we gate this transform with "nnan" in addition to "nsz": (X > 0.0) ? X : -X --> fabs(X) Without that restriction, we could have for example: (+NaN > 0.0) ? +NaN : -NaN --> -NaN (because an ordered compare with NaN is always false) That would be different than fabs(+NaN) --> +NaN. More fabs/fneg patterns demonstrated here: https://godbolt.org/z/h8ecc659d (without any FMF, these are correct independently of this patch - no fabs should be created) The code change is a one-liner, but we have lots of tests diffs because there are many variations of the basic pattern. Differential Revision: https://reviews.llvm.org/D139785
1 parent ac01ae7 commit 862e35e

File tree

4 files changed

+78
-26
lines changed

4 files changed

+78
-26
lines changed

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2634,7 +2634,11 @@ static Instruction *foldSelectWithFCmpToFabs(SelectInst &SI,
26342634
// when 'Swap' is true:
26352635
// fold (X > +/-0.0) ? X : -X or (X >= +/-0.0) ? X : -X to fabs(X)
26362636
// fold (X < +/-0.0) ? X : -X or (X <= +/-0.0) ? X : -X to -fabs(X)
2637-
if (!SI.hasNoSignedZeros())
2637+
//
2638+
// Note: We require "nnan" for this fold because fcmp ignores the signbit
2639+
// of NAN, but IEEE-754 specifies the signbit of NAN values with
2640+
// fneg/fabs operations.
2641+
if (!SI.hasNoSignedZeros() || !SI.hasNoNaNs())
26382642
return nullptr;
26392643

26402644
if (Swap)

‎llvm/test/Transforms/InstCombine/fabs.ll

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -349,10 +349,15 @@ define fp128 @select_fcmp_ogt_zero(fp128 %x) {
349349
ret fp128 %fabs
350350
}
351351

352+
; This is not fabs because that could produce a different signbit for a NAN input.
353+
; PR59279
354+
352355
define float @select_nsz_fcmp_ogt_fneg(float %a) {
353356
; CHECK-LABEL: @select_nsz_fcmp_ogt_fneg(
354-
; CHECK-NEXT: [[TMP1:%.*]] = call nsz float @llvm.fabs.f32(float [[A:%.*]])
355-
; CHECK-NEXT: ret float [[TMP1]]
357+
; CHECK-NEXT: [[FNEG:%.*]] = fneg float [[A:%.*]]
358+
; CHECK-NEXT: [[CMP:%.*]] = fcmp ogt float [[A]], 0.000000e+00
359+
; CHECK-NEXT: [[R:%.*]] = select nsz i1 [[CMP]], float [[A]], float [[FNEG]]
360+
; CHECK-NEXT: ret float [[R]]
356361
;
357362
%fneg = fneg float %a
358363
%cmp = fcmp ogt float %a, %fneg
@@ -445,19 +450,24 @@ define half @select_fcmp_nnan_oge_negzero(half %x) {
445450
ret half %fabs
446451
}
447452

448-
; X < 0.0 ? -X : X --> fabs(X)
453+
; This is not fabs because that could produce a different signbit for a NAN input.
454+
; PR59279
449455

450456
define double @select_nsz_fcmp_olt_zero_unary_fneg(double %x) {
451457
; CHECK-LABEL: @select_nsz_fcmp_olt_zero_unary_fneg(
452-
; CHECK-NEXT: [[TMP1:%.*]] = call nsz double @llvm.fabs.f64(double [[X:%.*]])
453-
; CHECK-NEXT: ret double [[TMP1]]
458+
; CHECK-NEXT: [[LTZERO:%.*]] = fcmp olt double [[X:%.*]], 0.000000e+00
459+
; CHECK-NEXT: [[NEGX:%.*]] = fneg double [[X]]
460+
; CHECK-NEXT: [[FABS:%.*]] = select nsz i1 [[LTZERO]], double [[NEGX]], double [[X]]
461+
; CHECK-NEXT: ret double [[FABS]]
454462
;
455463
%ltzero = fcmp olt double %x, 0.0
456464
%negx = fneg double %x
457465
%fabs = select nsz i1 %ltzero, double %negx, double %x
458466
ret double %fabs
459467
}
460468

469+
; X < 0.0 ? -X : X --> fabs(X)
470+
461471
define double @select_nsz_nnan_fcmp_olt_zero_unary_fneg(double %x) {
462472
; CHECK-LABEL: @select_nsz_nnan_fcmp_olt_zero_unary_fneg(
463473
; CHECK-NEXT: [[TMP1:%.*]] = call nnan nsz double @llvm.fabs.f64(double [[X:%.*]])
@@ -743,19 +753,24 @@ define float @select_fcmp_nnan_nsz_ule_negzero_unary_fneg(float %x) {
743753
ret float %fabs
744754
}
745755

746-
; X > 0.0 ? X : (-X) --> fabs(X)
756+
; This is not fabs because that could produce a different signbit for a NAN input.
757+
; PR59279
747758

748759
define <2 x float> @select_nsz_fcmp_ogt_zero_unary_fneg(<2 x float> %x) {
749760
; CHECK-LABEL: @select_nsz_fcmp_ogt_zero_unary_fneg(
750-
; CHECK-NEXT: [[TMP1:%.*]] = call nsz <2 x float> @llvm.fabs.v2f32(<2 x float> [[X:%.*]])
751-
; CHECK-NEXT: ret <2 x float> [[TMP1]]
761+
; CHECK-NEXT: [[GTZERO:%.*]] = fcmp ogt <2 x float> [[X:%.*]], zeroinitializer
762+
; CHECK-NEXT: [[NEGX:%.*]] = fneg <2 x float> [[X]]
763+
; CHECK-NEXT: [[FABS:%.*]] = select nsz <2 x i1> [[GTZERO]], <2 x float> [[X]], <2 x float> [[NEGX]]
764+
; CHECK-NEXT: ret <2 x float> [[FABS]]
752765
;
753766
%gtzero = fcmp ogt <2 x float> %x, zeroinitializer
754767
%negx = fneg <2 x float> %x
755768
%fabs = select nsz <2 x i1> %gtzero, <2 x float> %x, <2 x float> %negx
756769
ret <2 x float> %fabs
757770
}
758771

772+
; X > 0.0 ? X : (-X) --> fabs(X)
773+
759774
define <2 x float> @select_nsz_nnan_fcmp_ogt_zero_unary_fneg(<2 x float> %x) {
760775
; CHECK-LABEL: @select_nsz_nnan_fcmp_ogt_zero_unary_fneg(
761776
; CHECK-NEXT: [[TMP1:%.*]] = call nnan nsz <2 x float> @llvm.fabs.v2f32(<2 x float> [[X:%.*]])

‎llvm/test/Transforms/InstCombine/fneg-fabs.ll

Lines changed: 44 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,14 @@ define double @select_nsz_nnan_nfabs_lt_fmfProp(double %x) {
4444

4545
; Tests with various predicate types.
4646

47+
; This is not fabs because that could produce a different signbit for a NAN input.
48+
; PR59279
49+
4750
define double @select_nsz_nfabs_ult(double %x) {
4851
; CHECK-LABEL: @select_nsz_nfabs_ult(
49-
; CHECK-NEXT: [[TMP1:%.*]] = call nsz double @llvm.fabs.f64(double [[X:%.*]])
50-
; CHECK-NEXT: [[SEL:%.*]] = fneg nsz double [[TMP1]]
52+
; CHECK-NEXT: [[CMP:%.*]] = fcmp ult double [[X:%.*]], 0.000000e+00
53+
; CHECK-NEXT: [[NEGX:%.*]] = fneg double [[X]]
54+
; CHECK-NEXT: [[SEL:%.*]] = select nsz i1 [[CMP]], double [[X]], double [[NEGX]]
5155
; CHECK-NEXT: ret double [[SEL]]
5256
;
5357
%cmp = fcmp ult double %x, 0.000000e+00
@@ -68,10 +72,14 @@ define double @select_nsz_nnan_nfabs_ult(double %x) {
6872
ret double %sel
6973
}
7074

75+
; This is not fabs because that could produce a different signbit for a NAN input.
76+
; PR59279
77+
7178
define double @select_nsz_nfabs_ole(double %x) {
7279
; CHECK-LABEL: @select_nsz_nfabs_ole(
73-
; CHECK-NEXT: [[TMP1:%.*]] = call nsz double @llvm.fabs.f64(double [[X:%.*]])
74-
; CHECK-NEXT: [[SEL:%.*]] = fneg nsz double [[TMP1]]
80+
; CHECK-NEXT: [[CMP:%.*]] = fcmp ole double [[X:%.*]], 0.000000e+00
81+
; CHECK-NEXT: [[NEGX:%.*]] = fneg double [[X]]
82+
; CHECK-NEXT: [[SEL:%.*]] = select nsz i1 [[CMP]], double [[X]], double [[NEGX]]
7583
; CHECK-NEXT: ret double [[SEL]]
7684
;
7785
%cmp = fcmp ole double %x, 0.000000e+00
@@ -92,10 +100,14 @@ define double @select_nsz_nnan_nfabs_ole(double %x) {
92100
ret double %sel
93101
}
94102

103+
; This is not fabs because that could produce a different signbit for a NAN input.
104+
; PR59279
105+
95106
define double @select_nsz_nfabs_ule(double %x) {
96107
; CHECK-LABEL: @select_nsz_nfabs_ule(
97-
; CHECK-NEXT: [[TMP1:%.*]] = call nsz double @llvm.fabs.f64(double [[X:%.*]])
98-
; CHECK-NEXT: [[SEL:%.*]] = fneg nsz double [[TMP1]]
108+
; CHECK-NEXT: [[CMP:%.*]] = fcmp ule double [[X:%.*]], 0.000000e+00
109+
; CHECK-NEXT: [[NEGX:%.*]] = fneg double [[X]]
110+
; CHECK-NEXT: [[SEL:%.*]] = select nsz i1 [[CMP]], double [[X]], double [[NEGX]]
99111
; CHECK-NEXT: ret double [[SEL]]
100112
;
101113
%cmp = fcmp ule double %x, 0.000000e+00
@@ -157,11 +169,14 @@ define double @select_nsz_nnan_nfabs_gt_fmfProp(double %x) {
157169
ret double %sel
158170
}
159171

160-
; Tests with various predicate types.
172+
; This is not fabs because that could produce a different signbit for a NAN input.
173+
; PR59279
174+
161175
define double @select_nsz_nfabs_ogt(double %x) {
162176
; CHECK-LABEL: @select_nsz_nfabs_ogt(
163-
; CHECK-NEXT: [[TMP1:%.*]] = call nsz double @llvm.fabs.f64(double [[X:%.*]])
164-
; CHECK-NEXT: [[SEL:%.*]] = fneg nsz double [[TMP1]]
177+
; CHECK-NEXT: [[CMP:%.*]] = fcmp ogt double [[X:%.*]], 0.000000e+00
178+
; CHECK-NEXT: [[NEGX:%.*]] = fneg double [[X]]
179+
; CHECK-NEXT: [[SEL:%.*]] = select nsz i1 [[CMP]], double [[NEGX]], double [[X]]
165180
; CHECK-NEXT: ret double [[SEL]]
166181
;
167182
%cmp = fcmp ogt double %x, 0.000000e+00
@@ -170,6 +185,8 @@ define double @select_nsz_nfabs_ogt(double %x) {
170185
ret double %sel
171186
}
172187

188+
; Tests with various predicate types.
189+
173190
define double @select_nsz_nnan_nfabs_ogt(double %x) {
174191
; CHECK-LABEL: @select_nsz_nnan_nfabs_ogt(
175192
; CHECK-NEXT: [[TMP1:%.*]] = call nnan nsz double @llvm.fabs.f64(double [[X:%.*]])
@@ -182,10 +199,14 @@ define double @select_nsz_nnan_nfabs_ogt(double %x) {
182199
ret double %sel
183200
}
184201

202+
; This is not fabs because that could produce a different signbit for a NAN input.
203+
; PR59279
204+
185205
define double @select_nsz_nfabs_ugt(double %x) {
186206
; CHECK-LABEL: @select_nsz_nfabs_ugt(
187-
; CHECK-NEXT: [[TMP1:%.*]] = call nsz double @llvm.fabs.f64(double [[X:%.*]])
188-
; CHECK-NEXT: [[SEL:%.*]] = fneg nsz double [[TMP1]]
207+
; CHECK-NEXT: [[CMP:%.*]] = fcmp ugt double [[X:%.*]], 0.000000e+00
208+
; CHECK-NEXT: [[NEGX:%.*]] = fneg double [[X]]
209+
; CHECK-NEXT: [[SEL:%.*]] = select nsz i1 [[CMP]], double [[NEGX]], double [[X]]
189210
; CHECK-NEXT: ret double [[SEL]]
190211
;
191212
%cmp = fcmp ugt double %x, 0.000000e+00
@@ -206,10 +227,14 @@ define double @select_nsz_nnan_nfabs_ugt(double %x) {
206227
ret double %sel
207228
}
208229

230+
; This is not fabs because that could produce a different signbit for a NAN input.
231+
; PR59279
232+
209233
define double @select_nsz_nfabs_oge(double %x) {
210234
; CHECK-LABEL: @select_nsz_nfabs_oge(
211-
; CHECK-NEXT: [[TMP1:%.*]] = call nsz double @llvm.fabs.f64(double [[X:%.*]])
212-
; CHECK-NEXT: [[SEL:%.*]] = fneg nsz double [[TMP1]]
235+
; CHECK-NEXT: [[CMP:%.*]] = fcmp oge double [[X:%.*]], 0.000000e+00
236+
; CHECK-NEXT: [[NEGX:%.*]] = fneg double [[X]]
237+
; CHECK-NEXT: [[SEL:%.*]] = select nsz i1 [[CMP]], double [[NEGX]], double [[X]]
213238
; CHECK-NEXT: ret double [[SEL]]
214239
;
215240
%cmp = fcmp oge double %x, 0.000000e+00
@@ -230,10 +255,14 @@ define double @select_nsz_nnan_nfabs_oge(double %x) {
230255
ret double %sel
231256
}
232257

258+
; This is not fabs because that could produce a different signbit for a NAN input.
259+
; PR59279
260+
233261
define double @select_nsz_nfabs_uge(double %x) {
234262
; CHECK-LABEL: @select_nsz_nfabs_uge(
235-
; CHECK-NEXT: [[TMP1:%.*]] = call nsz double @llvm.fabs.f64(double [[X:%.*]])
236-
; CHECK-NEXT: [[SEL:%.*]] = fneg nsz double [[TMP1]]
263+
; CHECK-NEXT: [[CMP:%.*]] = fcmp uge double [[X:%.*]], 0.000000e+00
264+
; CHECK-NEXT: [[NEGX:%.*]] = fneg double [[X]]
265+
; CHECK-NEXT: [[SEL:%.*]] = select nsz i1 [[CMP]], double [[NEGX]], double [[X]]
237266
; CHECK-NEXT: ret double [[SEL]]
238267
;
239268
%cmp = fcmp uge double %x, 0.000000e+00

‎llvm/test/Transforms/InstCombine/fneg.ll

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -742,10 +742,14 @@ define float @fnabs_1(float %a) {
742742
ret float %fneg1
743743
}
744744

745+
; This is not fabs because that could produce a different signbit for a NAN input.
746+
; PR59279
747+
745748
define float @fnabs_2_nsz(float %a) {
746749
; CHECK-LABEL: @fnabs_2_nsz(
747-
; CHECK-NEXT: [[TMP1:%.*]] = call nsz float @llvm.fabs.f32(float [[A:%.*]])
748-
; CHECK-NEXT: [[FNEG1:%.*]] = fneg float [[TMP1]]
750+
; CHECK-NEXT: [[CMP:%.*]] = fcmp olt float [[A:%.*]], 0.000000e+00
751+
; CHECK-NEXT: [[A_NEG:%.*]] = fneg float [[A]]
752+
; CHECK-NEXT: [[FNEG1:%.*]] = select nsz i1 [[CMP]], float [[A]], float [[A_NEG]]
749753
; CHECK-NEXT: ret float [[FNEG1]]
750754
;
751755
%fneg = fneg float %a

0 commit comments

Comments
 (0)