Skip to content

Commit 7d81813

Browse files
author
Justin Lebar
committed
[TM] Restore default TargetOptions in TargetMachine::resetTargetOptions.
Summary: Previously if you had * a function with the fast-math-enabled attr, followed by * a function without the fast-math attr, the second function would inherit the first function's fast-math-ness. This means that mixing fast-math and non-fast-math functions in a module was completely broken unless you explicitly annotated every non-fast-math function with "unsafe-fp-math"="false". This appears to have been broken since r176986 (March 2013), when the resetTargetOptions function was introduced. This patch tests the correct behavior as best we can. I don't think I can test FPDenormalMode and NoTrappingFPMath, because they aren't used in any backends during function lowering. Surprisingly, I also can't find any uses at all of LessPreciseFPMAD affecting generated code. The NVPTX/fast-math.ll test changes are an expected result of fixing this bug. When FMA is disabled, we emit add as "add.rn.f32", which prevents fma combining. Before this patch, fast-math was enabled in all functions following the one which explicitly enabled it on itself, so we were emitting plain "add.f32" where we should have generated "add.rn.f32". Reviewers: mkuper Subscribers: hfinkel, majnemer, jholewinski, nemanjai, llvm-commits Differential Revision: https://reviews.llvm.org/D28507 llvm-svn: 291618
1 parent c1e2d97 commit 7d81813

File tree

5 files changed

+132
-5
lines changed

5 files changed

+132
-5
lines changed

llvm/include/llvm/Target/TargetMachine.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ class TargetMachine {
103103
unsigned O0WantsFastISel : 1;
104104

105105
public:
106+
const TargetOptions DefaultOptions;
106107
mutable TargetOptions Options;
107108

108109
virtual ~TargetMachine();

llvm/lib/Target/TargetMachine.cpp

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ TargetMachine::TargetMachine(const Target &T, StringRef DataLayoutString,
4444
const TargetOptions &Options)
4545
: TheTarget(T), DL(DataLayoutString), TargetTriple(TT), TargetCPU(CPU),
4646
TargetFS(FS), AsmInfo(nullptr), MRI(nullptr), MII(nullptr), STI(nullptr),
47-
RequireStructuredCFG(false), Options(Options) {
47+
RequireStructuredCFG(false), DefaultOptions(Options), Options(Options) {
4848
if (EnableIPRA.getNumOccurrences())
4949
this->Options.EnableIPRA = EnableIPRA;
5050
}
@@ -63,14 +63,15 @@ bool TargetMachine::isPositionIndependent() const {
6363
/// \brief Reset the target options based on the function's attributes.
6464
// FIXME: This function needs to go away for a number of reasons:
6565
// a) global state on the TargetMachine is terrible in general,
66-
// b) there's no default state here to keep,
67-
// c) these target options should be passed only on the function
66+
// b) these target options should be passed only on the function
6867
// and not on the TargetMachine (via TargetOptions) at all.
6968
void TargetMachine::resetTargetOptions(const Function &F) const {
7069
#define RESET_OPTION(X, Y) \
7170
do { \
7271
if (F.hasFnAttribute(Y)) \
7372
Options.X = (F.getFnAttribute(Y).getValueAsString() == "true"); \
73+
else \
74+
Options.X = DefaultOptions.X; \
7475
} while (0)
7576

7677
RESET_OPTION(LessPreciseFPMADOption, "less-precise-fpmad");
@@ -87,6 +88,8 @@ void TargetMachine::resetTargetOptions(const Function &F) const {
8788
Options.FPDenormalMode = FPDenormal::PreserveSign;
8889
else if (Denormal == "positive-zero")
8990
Options.FPDenormalMode = FPDenormal::PositiveZero;
91+
else
92+
Options.FPDenormalMode = DefaultOptions.FPDenormalMode;
9093
}
9194

9295
/// Returns the code generation relocation model. The choices are static, PIC,

llvm/test/CodeGen/NVPTX/fast-math.ll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,14 @@ define float @sqrt_div_fast(float %a, float %b) #0 {
2121
}
2222

2323
; CHECK-LABEL: fadd
24-
; CHECK: add.f32
24+
; CHECK: add.rn.f32
2525
define float @fadd(float %a, float %b) {
2626
%t1 = fadd float %a, %b
2727
ret float %t1
2828
}
2929

3030
; CHECK-LABEL: fadd_ftz
31-
; CHECK: add.ftz.f32
31+
; CHECK: add.rn.ftz.f32
3232
define float @fadd_ftz(float %a, float %b) #1 {
3333
%t1 = fadd float %a, %b
3434
ret float %t1
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
; Check that we can enable/disable NoInfsFPMath and NoNaNsInFPMath via function
2+
; attributes. An attribute on one function should not magically apply to the
3+
; next one.
4+
5+
; RUN: llc < %s -mtriple=powerpc64-unknown-unknown -mcpu=pwr7 -mattr=-vsx \
6+
; RUN: | FileCheck %s --check-prefix=CHECK --check-prefix=SAFE
7+
8+
; RUN: llc < %s -mtriple=powerpc64-unknown-unknown -mcpu=pwr7 -mattr=-vsx \
9+
; RUN: -enable-no-infs-fp-math -enable-no-nans-fp-math \
10+
; RUN: | FileCheck %s --check-prefix=CHECK --check-prefix=UNSAFE
11+
12+
; The fcmp+select in these functions should be converted to a fsel instruction
13+
; when both NoInfsFPMath and NoNaNsInFPMath are enabled.
14+
15+
; CHECK-LABEL: default0:
16+
define double @default0(double %a, double %y, double %z) {
17+
entry:
18+
; SAFE-NOT: fsel
19+
; UNSAFE: fsel
20+
%cmp = fcmp ult double %a, 0.000000e+00
21+
%z.y = select i1 %cmp, double %z, double %y
22+
ret double %z.y
23+
}
24+
25+
; CHECK-LABEL: unsafe_math_off:
26+
define double @unsafe_math_off(double %a, double %y, double %z) #0 #2 {
27+
entry:
28+
; SAFE-NOT: fsel
29+
; UNSAFE-NOT: fsel
30+
%cmp = fcmp ult double %a, 0.000000e+00
31+
%z.y = select i1 %cmp, double %z, double %y
32+
ret double %z.y
33+
}
34+
35+
; CHECK-LABEL: default1:
36+
define double @default1(double %a, double %y, double %z) {
37+
; SAFE-NOT: fsel
38+
; UNSAFE: fsel
39+
%cmp = fcmp ult double %a, 0.000000e+00
40+
%z.y = select i1 %cmp, double %z, double %y
41+
ret double %z.y
42+
}
43+
44+
; CHECK-LABEL: unsafe_math_on:
45+
define double @unsafe_math_on(double %a, double %y, double %z) #1 #3 {
46+
entry:
47+
; SAFE-NOT: fsel
48+
; UNSAFE-NOT: fsel
49+
%cmp = fcmp ult double %a, 0.000000e+00
50+
%z.y = select i1 %cmp, double %z, double %y
51+
ret double %z.y
52+
}
53+
54+
; CHECK-LABEL: default2:
55+
define double @default2(double %a, double %y, double %z) {
56+
; SAFE-NOT: fsel
57+
; UNSAFE: fsel
58+
%cmp = fcmp ult double %a, 0.000000e+00
59+
%z.y = select i1 %cmp, double %z, double %y
60+
ret double %z.y
61+
}
62+
63+
attributes #0 = { "no-infs-fp-math"="false" }
64+
attributes #1 = { "no-nans-fp-math"="false" }
65+
66+
attributes #2 = { "no-infs-fp-math"="false" }
67+
attributes #3 = { "no-infs-fp-math"="true" }
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
; Check that we can enable/disable UnsafeFPMath via function attributes. An
2+
; attribute on one function should not magically apply to the next one.
3+
4+
; RUN: llc < %s -mtriple=x86_64-unknown-unknown \
5+
; RUN: | FileCheck %s --check-prefix=CHECK --check-prefix=SAFE
6+
7+
; RUN: llc < %s -mtriple=x86_64-unknown-unknown -enable-unsafe-fp-math \
8+
; RUN: | FileCheck %s --check-prefix=CHECK --check-prefix=UNSAFE
9+
10+
; The div in these functions should be converted to a mul when unsafe-fp-math
11+
; is enabled.
12+
13+
; CHECK-LABEL: unsafe_fp_math_default0:
14+
define double @unsafe_fp_math_default0(double %x) {
15+
; SAFE: divsd
16+
; UNSAFE: mulsd
17+
%div = fdiv double %x, 2.0
18+
ret double %div
19+
}
20+
21+
; CHECK-LABEL: unsafe_fp_math_off:
22+
define double @unsafe_fp_math_off(double %x) #0 {
23+
; SAFE: divsd
24+
; UNSAFE: divsd
25+
%div = fdiv double %x, 2.0
26+
ret double %div
27+
}
28+
29+
; CHECK-LABEL: unsafe_fp_math_default1:
30+
define double @unsafe_fp_math_default1(double %x) {
31+
; With unsafe math enabled, can change this div to a mul.
32+
; SAFE: divsd
33+
; UNSAFE: mulsd
34+
%div = fdiv double %x, 2.0
35+
ret double %div
36+
}
37+
38+
; CHECK-LABEL: unsafe_fp_math_on:
39+
define double @unsafe_fp_math_on(double %x) #1 {
40+
; SAFE: mulsd
41+
; UNSAFE: mulsd
42+
%div = fdiv double %x, 2.0
43+
ret double %div
44+
}
45+
46+
; CHECK-LABEL: unsafe_fp_math_default2:
47+
define double @unsafe_fp_math_default2(double %x) {
48+
; With unsafe math enabled, can change this div to a mul.
49+
; SAFE: divsd
50+
; UNSAFE: mulsd
51+
%div = fdiv double %x, 2.0
52+
ret double %div
53+
}
54+
55+
attributes #0 = { "unsafe-fp-math"="false" }
56+
attributes #1 = { "unsafe-fp-math"="true" }

0 commit comments

Comments
 (0)