Skip to content

Commit 20034ba

Browse files
mikolaj-pirogmikolaj-pirog
authored
[X86] Don't rely on global contraction flag (#167252)
As in title. See here for more context: https://discourse.llvm.org/t/allowfpopfusion-vs-sdnodeflags-hasallowcontract/80909 Also add a warning in llc when global contract flag is encountered on x86. Remove global contract from last x86 test
1 parent 5fa3ccb commit 20034ba

File tree

4 files changed

+42
-32
lines changed

4 files changed

+42
-32
lines changed

‎llvm/lib/Target/X86/X86ISelLowering.cpp‎

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8437,9 +8437,7 @@ static bool isFMAddSubOrFMSubAdd(const X86Subtarget &Subtarget,
84378437
// DAGCombiner::visitFADDForFMACombine. It would be good to have one
84388438
// function that would answer if it is Ok to fuse MUL + ADD to FMADD
84398439
// or MUL + ADDSUB to FMADDSUB.
8440-
const TargetOptions &Options = DAG.getTarget().Options;
84418440
bool AllowFusion =
8442-
Options.AllowFPOpFusion == FPOpFusion::Fast ||
84438441
(AllowSubAddOrAddSubContract && Opnd0->getFlags().hasAllowContract());
84448442
if (!AllowFusion)
84458443
return false;
@@ -54165,11 +54163,6 @@ static SDValue combineFMulcFCMulc(SDNode *N, SelectionDAG &DAG,
5416554163
// FADD(A, FMA(B, C, 0)) and FADD(A, FMUL(B, C)) to FMA(B, C, A)
5416654164
static SDValue combineFaddCFmul(SDNode *N, SelectionDAG &DAG,
5416754165
const X86Subtarget &Subtarget) {
54168-
auto AllowContract = [&DAG](const SDNodeFlags &Flags) {
54169-
return DAG.getTarget().Options.AllowFPOpFusion == FPOpFusion::Fast ||
54170-
Flags.hasAllowContract();
54171-
};
54172-
5417354166
auto HasNoSignedZero = [&DAG](const SDNodeFlags &Flags) {
5417454167
return DAG.getTarget().Options.NoSignedZerosFPMath ||
5417554168
Flags.hasNoSignedZeros();
@@ -54182,7 +54175,7 @@ static SDValue combineFaddCFmul(SDNode *N, SelectionDAG &DAG,
5418254175
};
5418354176

5418454177
if (N->getOpcode() != ISD::FADD || !Subtarget.hasFP16() ||
54185-
!AllowContract(N->getFlags()))
54178+
!N->getFlags().hasAllowContract())
5418654179
return SDValue();
5418754180

5418854181
EVT VT = N->getValueType(0);
@@ -54193,14 +54186,13 @@ static SDValue combineFaddCFmul(SDNode *N, SelectionDAG &DAG,
5419354186
SDValue RHS = N->getOperand(1);
5419454187
bool IsConj;
5419554188
SDValue FAddOp1, MulOp0, MulOp1;
54196-
auto GetCFmulFrom = [&MulOp0, &MulOp1, &IsConj, &AllowContract,
54197-
&IsVectorAllNegativeZero,
54189+
auto GetCFmulFrom = [&MulOp0, &MulOp1, &IsConj, &IsVectorAllNegativeZero,
5419854190
&HasNoSignedZero](SDValue N) -> bool {
5419954191
if (!N.hasOneUse() || N.getOpcode() != ISD::BITCAST)
5420054192
return false;
5420154193
SDValue Op0 = N.getOperand(0);
5420254194
unsigned Opcode = Op0.getOpcode();
54203-
if (Op0.hasOneUse() && AllowContract(Op0->getFlags())) {
54195+
if (Op0.hasOneUse() && Op0->getFlags().hasAllowContract()) {
5420454196
if ((Opcode == X86ISD::VFMULC || Opcode == X86ISD::VFCMULC)) {
5420554197
MulOp0 = Op0.getOperand(0);
5420654198
MulOp1 = Op0.getOperand(1);

‎llvm/test/CodeGen/X86/llc-fp-contract-warning.ll‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
; RUN: llc < %s -mtriple=x86_64-unknown-unknown --fp-contract=fast 2>&1 | grep "X86 backend ignores --fp-contract"
2+
3+
; RUN: llc < %s -mtriple=x86_64-unknown-unknown --fp-contract=off 2>&1 | grep "X86 backend ignores --fp-contract"
4+
5+
; on, as a default setting that's passed to backend when no --fp-contract option is specified, is not diagnosed.
6+
; RUN: llc < %s -mtriple=x86_64-unknown-unknown --fp-contract=on 2>&1 | grep -v "X86 backend ignores --fp-contract"
7+
8+
define float @foo(float %f) {
9+
%res = fadd float %f, %f
10+
ret float %res
11+
}
12+

‎llvm/test/CodeGen/X86/sqrt-fastmath-mir.ll‎

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
; NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
2-
; RUN: llc -fp-contract=fast < %s -mtriple=x86_64-unknown-unknown -mattr=avx2,fma -stop-after=finalize-isel 2>&1 | FileCheck %s
2+
; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=avx2,fma -stop-after=finalize-isel 2>&1 | FileCheck %s
33

44
declare float @llvm.sqrt.f32(float) #2
55

@@ -24,17 +24,17 @@ define float @sqrt_ieee_ninf(float %f) #0 {
2424
; CHECK-NEXT: {{ $}}
2525
; CHECK-NEXT: [[COPY:%[0-9]+]]:fr32 = COPY $xmm0
2626
; CHECK-NEXT: [[DEF:%[0-9]+]]:fr32 = IMPLICIT_DEF
27-
; CHECK-NEXT: [[VRSQRTSSr:%[0-9]+]]:fr32 = ninf afn VRSQRTSSr killed [[DEF]], [[COPY]]
28-
; CHECK-NEXT: [[VMULSSrr:%[0-9]+]]:fr32 = ninf afn nofpexcept VMULSSrr [[COPY]], [[VRSQRTSSr]], implicit $mxcsr
27+
; CHECK-NEXT: [[VRSQRTSSr:%[0-9]+]]:fr32 = ninf contract afn VRSQRTSSr killed [[DEF]], [[COPY]]
28+
; CHECK-NEXT: [[VMULSSrr:%[0-9]+]]:fr32 = ninf contract afn nofpexcept VMULSSrr [[COPY]], [[VRSQRTSSr]], implicit $mxcsr
2929
; CHECK-NEXT: [[VMOVSSrm_alt:%[0-9]+]]:fr32 = VMOVSSrm_alt $rip, 1, $noreg, %const.0, $noreg :: (load (s32) from constant-pool)
30-
; CHECK-NEXT: [[VFMADD213SSr:%[0-9]+]]:fr32 = ninf afn nofpexcept VFMADD213SSr [[VRSQRTSSr]], killed [[VMULSSrr]], [[VMOVSSrm_alt]], implicit $mxcsr
30+
; CHECK-NEXT: [[VFMADD213SSr:%[0-9]+]]:fr32 = ninf contract afn nofpexcept VFMADD213SSr [[VRSQRTSSr]], killed [[VMULSSrr]], [[VMOVSSrm_alt]], implicit $mxcsr
3131
; CHECK-NEXT: [[VMOVSSrm_alt1:%[0-9]+]]:fr32 = VMOVSSrm_alt $rip, 1, $noreg, %const.1, $noreg :: (load (s32) from constant-pool)
32-
; CHECK-NEXT: [[VMULSSrr1:%[0-9]+]]:fr32 = ninf afn nofpexcept VMULSSrr [[VRSQRTSSr]], [[VMOVSSrm_alt1]], implicit $mxcsr
33-
; CHECK-NEXT: [[VMULSSrr2:%[0-9]+]]:fr32 = ninf afn nofpexcept VMULSSrr killed [[VMULSSrr1]], killed [[VFMADD213SSr]], implicit $mxcsr
34-
; CHECK-NEXT: [[VMULSSrr3:%[0-9]+]]:fr32 = ninf afn nofpexcept VMULSSrr [[COPY]], [[VMULSSrr2]], implicit $mxcsr
35-
; CHECK-NEXT: [[VFMADD213SSr1:%[0-9]+]]:fr32 = ninf afn nofpexcept VFMADD213SSr [[VMULSSrr2]], [[VMULSSrr3]], [[VMOVSSrm_alt]], implicit $mxcsr
36-
; CHECK-NEXT: [[VMULSSrr4:%[0-9]+]]:fr32 = ninf afn nofpexcept VMULSSrr [[VMULSSrr3]], [[VMOVSSrm_alt1]], implicit $mxcsr
37-
; CHECK-NEXT: [[VMULSSrr5:%[0-9]+]]:fr32 = ninf afn nofpexcept VMULSSrr killed [[VMULSSrr4]], killed [[VFMADD213SSr1]], implicit $mxcsr
32+
; CHECK-NEXT: [[VMULSSrr1:%[0-9]+]]:fr32 = ninf contract afn nofpexcept VMULSSrr [[VRSQRTSSr]], [[VMOVSSrm_alt1]], implicit $mxcsr
33+
; CHECK-NEXT: [[VMULSSrr2:%[0-9]+]]:fr32 = ninf contract afn nofpexcept VMULSSrr killed [[VMULSSrr1]], killed [[VFMADD213SSr]], implicit $mxcsr
34+
; CHECK-NEXT: [[VMULSSrr3:%[0-9]+]]:fr32 = ninf contract afn nofpexcept VMULSSrr [[COPY]], [[VMULSSrr2]], implicit $mxcsr
35+
; CHECK-NEXT: [[VFMADD213SSr1:%[0-9]+]]:fr32 = ninf contract afn nofpexcept VFMADD213SSr [[VMULSSrr2]], [[VMULSSrr3]], [[VMOVSSrm_alt]], implicit $mxcsr
36+
; CHECK-NEXT: [[VMULSSrr4:%[0-9]+]]:fr32 = ninf contract afn nofpexcept VMULSSrr [[VMULSSrr3]], [[VMOVSSrm_alt1]], implicit $mxcsr
37+
; CHECK-NEXT: [[VMULSSrr5:%[0-9]+]]:fr32 = ninf contract afn nofpexcept VMULSSrr killed [[VMULSSrr4]], killed [[VFMADD213SSr1]], implicit $mxcsr
3838
; CHECK-NEXT: [[COPY1:%[0-9]+]]:vr128 = COPY killed [[VMULSSrr5]]
3939
; CHECK-NEXT: [[COPY2:%[0-9]+]]:vr128 = COPY [[COPY]]
4040
; CHECK-NEXT: [[VPBROADCASTDrm:%[0-9]+]]:vr128 = VPBROADCASTDrm $rip, 1, $noreg, %const.2, $noreg :: (load (s32) from constant-pool)
@@ -46,7 +46,7 @@ define float @sqrt_ieee_ninf(float %f) #0 {
4646
; CHECK-NEXT: [[COPY5:%[0-9]+]]:fr32 = COPY killed [[VPANDNrr]]
4747
; CHECK-NEXT: $xmm0 = COPY [[COPY5]]
4848
; CHECK-NEXT: RET 0, $xmm0
49-
%call = tail call ninf afn float @llvm.sqrt.f32(float %f)
49+
%call = tail call ninf afn contract float @llvm.sqrt.f32(float %f)
5050
ret float %call
5151
}
5252

@@ -71,17 +71,17 @@ define float @sqrt_daz_ninf(float %f) #1 {
7171
; CHECK-NEXT: {{ $}}
7272
; CHECK-NEXT: [[COPY:%[0-9]+]]:fr32 = COPY $xmm0
7373
; CHECK-NEXT: [[DEF:%[0-9]+]]:fr32 = IMPLICIT_DEF
74-
; CHECK-NEXT: [[VRSQRTSSr:%[0-9]+]]:fr32 = ninf afn VRSQRTSSr killed [[DEF]], [[COPY]]
75-
; CHECK-NEXT: [[VMULSSrr:%[0-9]+]]:fr32 = ninf afn nofpexcept VMULSSrr [[COPY]], [[VRSQRTSSr]], implicit $mxcsr
74+
; CHECK-NEXT: [[VRSQRTSSr:%[0-9]+]]:fr32 = ninf contract afn VRSQRTSSr killed [[DEF]], [[COPY]]
75+
; CHECK-NEXT: [[VMULSSrr:%[0-9]+]]:fr32 = ninf contract afn nofpexcept VMULSSrr [[COPY]], [[VRSQRTSSr]], implicit $mxcsr
7676
; CHECK-NEXT: [[VMOVSSrm_alt:%[0-9]+]]:fr32 = VMOVSSrm_alt $rip, 1, $noreg, %const.0, $noreg :: (load (s32) from constant-pool)
77-
; CHECK-NEXT: [[VFMADD213SSr:%[0-9]+]]:fr32 = ninf afn nofpexcept VFMADD213SSr [[VRSQRTSSr]], killed [[VMULSSrr]], [[VMOVSSrm_alt]], implicit $mxcsr
77+
; CHECK-NEXT: [[VFMADD213SSr:%[0-9]+]]:fr32 = ninf contract afn nofpexcept VFMADD213SSr [[VRSQRTSSr]], killed [[VMULSSrr]], [[VMOVSSrm_alt]], implicit $mxcsr
7878
; CHECK-NEXT: [[VMOVSSrm_alt1:%[0-9]+]]:fr32 = VMOVSSrm_alt $rip, 1, $noreg, %const.1, $noreg :: (load (s32) from constant-pool)
79-
; CHECK-NEXT: [[VMULSSrr1:%[0-9]+]]:fr32 = ninf afn nofpexcept VMULSSrr [[VRSQRTSSr]], [[VMOVSSrm_alt1]], implicit $mxcsr
80-
; CHECK-NEXT: [[VMULSSrr2:%[0-9]+]]:fr32 = ninf afn nofpexcept VMULSSrr killed [[VMULSSrr1]], killed [[VFMADD213SSr]], implicit $mxcsr
81-
; CHECK-NEXT: [[VMULSSrr3:%[0-9]+]]:fr32 = ninf afn nofpexcept VMULSSrr [[COPY]], [[VMULSSrr2]], implicit $mxcsr
82-
; CHECK-NEXT: [[VFMADD213SSr1:%[0-9]+]]:fr32 = ninf afn nofpexcept VFMADD213SSr [[VMULSSrr2]], [[VMULSSrr3]], [[VMOVSSrm_alt]], implicit $mxcsr
83-
; CHECK-NEXT: [[VMULSSrr4:%[0-9]+]]:fr32 = ninf afn nofpexcept VMULSSrr [[VMULSSrr3]], [[VMOVSSrm_alt1]], implicit $mxcsr
84-
; CHECK-NEXT: [[VMULSSrr5:%[0-9]+]]:fr32 = ninf afn nofpexcept VMULSSrr killed [[VMULSSrr4]], killed [[VFMADD213SSr1]], implicit $mxcsr
79+
; CHECK-NEXT: [[VMULSSrr1:%[0-9]+]]:fr32 = ninf contract afn nofpexcept VMULSSrr [[VRSQRTSSr]], [[VMOVSSrm_alt1]], implicit $mxcsr
80+
; CHECK-NEXT: [[VMULSSrr2:%[0-9]+]]:fr32 = ninf contract afn nofpexcept VMULSSrr killed [[VMULSSrr1]], killed [[VFMADD213SSr]], implicit $mxcsr
81+
; CHECK-NEXT: [[VMULSSrr3:%[0-9]+]]:fr32 = ninf contract afn nofpexcept VMULSSrr [[COPY]], [[VMULSSrr2]], implicit $mxcsr
82+
; CHECK-NEXT: [[VFMADD213SSr1:%[0-9]+]]:fr32 = ninf contract afn nofpexcept VFMADD213SSr [[VMULSSrr2]], [[VMULSSrr3]], [[VMOVSSrm_alt]], implicit $mxcsr
83+
; CHECK-NEXT: [[VMULSSrr4:%[0-9]+]]:fr32 = ninf contract afn nofpexcept VMULSSrr [[VMULSSrr3]], [[VMOVSSrm_alt1]], implicit $mxcsr
84+
; CHECK-NEXT: [[VMULSSrr5:%[0-9]+]]:fr32 = ninf contract afn nofpexcept VMULSSrr killed [[VMULSSrr4]], killed [[VFMADD213SSr1]], implicit $mxcsr
8585
; CHECK-NEXT: [[COPY1:%[0-9]+]]:vr128 = COPY killed [[VMULSSrr5]]
8686
; CHECK-NEXT: [[FsFLD0SS:%[0-9]+]]:fr32 = FsFLD0SS
8787
; CHECK-NEXT: [[VCMPSSrri:%[0-9]+]]:fr32 = nofpexcept VCMPSSrri [[COPY]], killed [[FsFLD0SS]], 0, implicit $mxcsr
@@ -90,7 +90,7 @@ define float @sqrt_daz_ninf(float %f) #1 {
9090
; CHECK-NEXT: [[COPY3:%[0-9]+]]:fr32 = COPY killed [[VPANDNrr]]
9191
; CHECK-NEXT: $xmm0 = COPY [[COPY3]]
9292
; CHECK-NEXT: RET 0, $xmm0
93-
%call = tail call ninf afn float @llvm.sqrt.f32(float %f)
93+
%call = tail call ninf afn contract float @llvm.sqrt.f32(float %f)
9494
ret float %call
9595
}
9696

‎llvm/tools/llc/llc.cpp‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -604,6 +604,12 @@ static int compileModule(char **argv, LLVMContext &Context,
604604
InputFilename);
605605
}
606606

607+
if (TheTriple.isX86() &&
608+
codegen::getFuseFPOps() != FPOpFusion::FPOpFusionMode::Standard)
609+
WithColor::warning(errs(), argv[0])
610+
<< "X86 backend ignores --fp-contract setting; use IR fast-math "
611+
"flags instead.";
612+
607613
Options.BinutilsVersion =
608614
TargetMachine::parseBinutilsVersion(BinutilsVersion);
609615
Options.MCOptions.ShowMCEncoding = ShowMCEncoding;

0 commit comments

Comments
 (0)