<div dir="ltr">Hi,<br><br>I saw that this patch was reverted again later, but wanted to let you know the performance impact we saw with this patch.<br><span style="color:rgb(38,50,56);font-family:Roboto,sans-serif;font-size:13px">We are testing this on a variety of x86 machines and saw across the board regressions in Eigen of around 25-50%, changing no compile time options.<br></span><br>Thanks,<br>Rumeet</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Feb 12, 2020 at 7:31 AM Melanie Blower via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
Author: Melanie Blower<br>
Date: 2020-02-12T07:30:43-08:00<br>
New Revision: abd09053bc7aa6144873c196a7d50aa6ce6ca430<br>
<br>
URL: <a href="https://github.com/llvm/llvm-project/commit/abd09053bc7aa6144873c196a7d50aa6ce6ca430" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/abd09053bc7aa6144873c196a7d50aa6ce6ca430</a><br>
DIFF: <a href="https://github.com/llvm/llvm-project/commit/abd09053bc7aa6144873c196a7d50aa6ce6ca430.diff" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/abd09053bc7aa6144873c196a7d50aa6ce6ca430.diff</a><br>
<br>
LOG: Revert "Revert "Change clang option -ffp-model=precise to select ffp-contract=on""<br>
<br>
This reverts commit 99c5bcbce89f07e68ccd89891a0300346705d013.<br>
Change clang option -ffp-model=precise to select ffp-contract=on<br>
Including some small touch-ups to the original commit<br>
<br>
Reviewers: rjmccall, Andy Kaylor<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D74436" rel="noreferrer" target="_blank">https://reviews.llvm.org/D74436</a><br>
<br>
Added: <br>
<br>
<br>
Modified: <br>
clang/docs/UsersManual.rst<br>
clang/lib/Driver/ToolChains/Clang.cpp<br>
clang/test/CodeGen/ppc-emmintrin.c<br>
clang/test/CodeGen/ppc-xmmintrin.c<br>
clang/test/Driver/fp-model.c<br>
<br>
Removed: <br>
<br>
<br>
<br>
################################################################################<br>
diff --git a/clang/docs/UsersManual.rst b/clang/docs/UsersManual.rst<br>
index 856d5e34bbcc..6c8c9f802082 100644<br>
--- a/clang/docs/UsersManual.rst<br>
+++ b/clang/docs/UsersManual.rst<br>
@@ -1190,8 +1190,50 @@ installed.<br>
Controlling Floating Point Behavior<br>
-----------------------------------<br>
<br>
-Clang provides a number of ways to control floating point behavior. The options<br>
-are listed below.<br>
+Clang provides a number of ways to control floating point behavior, including<br>
+with command line options and source pragmas. This section<br>
+describes the various floating point semantic modes and the corresponding options.<br>
+<br>
+.. csv-table:: Floating Point Semantic Modes<br>
+ :header: "Mode", "Values"<br>
+ :widths: 15, 30, 30<br>
+<br>
+ "except_behavior", "{ignore, strict, may_trap}", "ffp-exception-behavior"<br>
+ "fenv_access", "{off, on}", "(none)"<br>
+ "rounding_mode", "{dynamic, tonearest, downward, upward, towardzero}", "frounding-math"<br>
+ "contract", "{on, off, fast}", "ffp-contract"<br>
+ "denormal_fp_math", "{IEEE, PreserveSign, PositiveZero}", "fdenormal-fp-math"<br>
+ "denormal_fp32_math", "{IEEE, PreserveSign, PositiveZero}", "fdenormal-fp-math-fp32"<br>
+ "support_math_errno", "{on, off}", "fmath-errno"<br>
+ "no_honor_nans", "{on, off}", "fhonor-nans"<br>
+ "no_honor_infinities", "{on, off}", "fhonor-infinities"<br>
+ "no_signed_zeros", "{on, off}", "fsigned-zeros"<br>
+ "allow_reciprocal", "{on, off}", "freciprocal-math"<br>
+ "allow_approximate_fns", "{on, off}", "(none)"<br>
+ "allow_reassociation", "{on, off}", "fassociative-math"<br>
+<br>
+<br>
+This table describes the option settings that correspond to the three<br>
+floating point semantic models: precise (the default), strict, and fast.<br>
+<br>
+<br>
+.. csv-table:: Floating Point Models<br>
+ :header: "Mode", "Precise", "Strict", "Fast"<br>
+ :widths: 25, 15, 15, 15<br>
+<br>
+ "except_behavior", "ignore", "strict", "ignore"<br>
+ "fenv_access", "off", "on", "off"<br>
+ "rounding_mode", "tonearest", "dynamic", "tonearest"<br>
+ "contract", "on", "off", "fast"<br>
+ "denormal_fp_math", "IEEE", "IEEE", "PreserveSign"<br>
+ "denormal_fp32_math", "IEEE","IEEE", "PreserveSign"<br>
+ "support_math_errno", "on", "on", "off"<br>
+ "no_honor_nans", "off", "off", "on"<br>
+ "no_honor_infinities", "off", "off", "on"<br>
+ "no_signed_zeros", "off", "off", "on"<br>
+ "allow_reciprocal", "off", "off", "on"<br>
+ "allow_approximate_fns", "off", "off", "on"<br>
+ "allow_reassociation", "off", "off", "on"<br>
<br>
.. option:: -ffast-math<br>
<br>
@@ -1385,7 +1427,7 @@ Note that floating-point operations performed as part of constant initialization<br>
and ``fast``.<br>
Details:<br>
<br>
- * ``precise`` Disables optimizations that are not value-safe on floating-point data, although FP contraction (FMA) is enabled (``-ffp-contract=fast``). This is the default behavior.<br>
+ * ``precise`` Disables optimizations that are not value-safe on floating-point data, although FP contraction (FMA) is enabled (``-ffp-contract=on``). This is the default behavior.<br>
* ``strict`` Enables ``-frounding-math`` and ``-ffp-exception-behavior=strict``, and disables contractions (FMA). All of the ``-ffast-math`` enablements are disabled.<br>
* ``fast`` Behaves identically to specifying both ``-ffast-math`` and ``ffp-contract=fast``<br>
<br>
<br>
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp<br>
index 4424d8e6f72c..a11a5423b0b9 100644<br>
--- a/clang/lib/Driver/ToolChains/Clang.cpp<br>
+++ b/clang/lib/Driver/ToolChains/Clang.cpp<br>
@@ -2525,10 +2525,9 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,<br>
<br>
llvm::DenormalMode DenormalFPMath = DefaultDenormalFPMath;<br>
llvm::DenormalMode DenormalFP32Math = DefaultDenormalFP32Math;<br>
- StringRef FPContract = "";<br>
+ StringRef FPContract = "on";<br>
bool StrictFPModel = false;<br>
<br>
-<br>
if (const Arg *A = Args.getLastArg(options::OPT_flimited_precision_EQ)) {<br>
CmdArgs.push_back("-mlimit-float-precision");<br>
CmdArgs.push_back(A->getValue());<br>
@@ -2551,7 +2550,6 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,<br>
SignedZeros = true;<br>
// -fno_fast_math restores default denormal and fpcontract handling<br>
DenormalFPMath = DefaultDenormalFPMath;<br>
- FPContract = "";<br>
StringRef Val = A->getValue();<br>
if (OFastEnabled && !Val.equals("fast")) {<br>
// Only -ffp-model=fast is compatible with OFast, ignore.<br>
@@ -2565,12 +2563,10 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,<br>
// ffp-model= is a Driver option, it is entirely rewritten into more<br>
// granular options before being passed into cc1.<br>
// Use the gcc option in the switch below.<br>
- if (!FPModel.empty() && !FPModel.equals(Val)) {<br>
+ if (!FPModel.empty() && !FPModel.equals(Val))<br>
D.Diag(clang::diag::warn_drv_overriding_flag_option)<br>
<< Args.MakeArgString("-ffp-model=" + FPModel)<br>
<< Args.MakeArgString("-ffp-model=" + Val);<br>
- FPContract = "";<br>
- }<br>
if (Val.equals("fast")) {<br>
optID = options::OPT_ffast_math;<br>
FPModel = Val;<br>
@@ -2578,13 +2574,14 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,<br>
} else if (Val.equals("precise")) {<br>
optID = options::OPT_ffp_contract;<br>
FPModel = Val;<br>
- FPContract = "fast";<br>
+ FPContract = "on";<br>
PreciseFPModel = true;<br>
} else if (Val.equals("strict")) {<br>
StrictFPModel = true;<br>
optID = options::OPT_frounding_math;<br>
FPExceptionBehavior = "strict";<br>
FPModel = Val;<br>
+ FPContract = "off";<br>
TrappingMath = true;<br>
} else<br>
D.Diag(diag::err_drv_unsupported_option_argument)<br>
@@ -2663,9 +2660,11 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,<br>
case options::OPT_ffp_contract: {<br>
StringRef Val = A->getValue();<br>
if (PreciseFPModel) {<br>
- // -ffp-model=precise enables ffp-contract=fast as a side effect<br>
- // the FPContract value has already been set to a string literal<br>
- // and the Val string isn't a pertinent value.<br>
+ // When -ffp-model=precise is seen on the command line,<br>
+ // the boolean PreciseFPModel is set to true which indicates<br>
+ // "the current option is actually PreciseFPModel". The optID<br>
+ // is changed to OPT_ffp_contract and FPContract is set to "on".<br>
+ // the argument Val string is "precise": it shouldn't be checked.<br>
;<br>
} else if (Val.equals("fast") || Val.equals("on") || Val.equals("off"))<br>
FPContract = Val;<br>
@@ -2762,7 +2761,7 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,<br>
// -fno_fast_math restores default denormal and fpcontract handling<br>
DenormalFPMath = DefaultDenormalFPMath;<br>
DenormalFP32Math = DefaultDenormalFP32Math;<br>
- FPContract = "";<br>
+ FPContract = "on";<br>
break;<br>
}<br>
if (StrictFPModel) {<br>
@@ -2773,7 +2772,7 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,<br>
!AssociativeMath && !ReciprocalMath &&<br>
SignedZeros && TrappingMath && RoundingFPMath &&<br>
DenormalFPMath != llvm::DenormalMode::getIEEE() &&<br>
- FPContract.empty())<br>
+ FPContract.equals("off"))<br>
// OK: Current Arg doesn't conflict with -ffp-model=strict<br>
;<br>
else {<br>
<br>
diff --git a/clang/test/CodeGen/ppc-emmintrin.c b/clang/test/CodeGen/ppc-emmintrin.c<br>
index 631b6c9d2614..c14b2dd210f8 100644<br>
--- a/clang/test/CodeGen/ppc-emmintrin.c<br>
+++ b/clang/test/CodeGen/ppc-emmintrin.c<br>
@@ -2,9 +2,9 @@<br>
// REQUIRES: powerpc-registered-target<br>
<br>
// RUN: %clang -S -emit-llvm -target powerpc64-unknown-linux-gnu -mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \<br>
-// RUN: -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-BE<br>
+// RUN: -ffp-contract=off -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-BE<br>
// RUN: %clang -S -emit-llvm -target powerpc64le-unknown-linux-gnu -mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \<br>
-// RUN: -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-LE<br>
+// RUN: -ffp-contract=off -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-LE<br>
<br>
// CHECK-BE-DAG: @_mm_movemask_pd.perm_mask = internal constant <4 x i32> <i32 -<a href="tel:(213)%20906-2144" value="+12139062144" target="_blank">2139062144</a>, i32 -<a href="tel:(213)%20906-2144" value="+12139062144" target="_blank">2139062144</a>, i32 -<a href="tel:(213)%20906-2144" value="+12139062144" target="_blank">2139062144</a>, i32 -<a href="tel:(213)%20907-8656" value="+12139078656" target="_blank">2139078656</a>>, align 16<br>
// CHECK-BE-DAG: @_mm_shuffle_epi32.permute_selectors = internal constant [4 x i32] [i32 66051, i32 67438087, i32 134810123, i32 202182159], align 4<br>
<br>
diff --git a/clang/test/CodeGen/ppc-xmmintrin.c b/clang/test/CodeGen/ppc-xmmintrin.c<br>
index e9466b32257f..d7499cbedc48 100644<br>
--- a/clang/test/CodeGen/ppc-xmmintrin.c<br>
+++ b/clang/test/CodeGen/ppc-xmmintrin.c<br>
@@ -2,9 +2,9 @@<br>
// REQUIRES: powerpc-registered-target<br>
<br>
// RUN: %clang -S -emit-llvm -target powerpc64-unknown-linux-gnu -mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \<br>
-// RUN: -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-BE<br>
+// RUN: -ffp-contract=off -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-BE<br>
// RUN: %clang -S -emit-llvm -target powerpc64le-unknown-linux-gnu -mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \<br>
-// RUN: -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-LE<br>
+// RUN: -ffp-contract=off -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-LE<br>
<br>
#include <xmmintrin.h><br>
<br>
<br>
diff --git a/clang/test/Driver/fp-model.c b/clang/test/Driver/fp-model.c<br>
index a3984acef62b..8ebbc1803c65 100644<br>
--- a/clang/test/Driver/fp-model.c<br>
+++ b/clang/test/Driver/fp-model.c<br>
@@ -27,9 +27,9 @@<br>
// RUN: | FileCheck --check-prefix=WARN5 %s<br>
// WARN5: warning: overriding '-ffp-model=strict' option with '-ffp-contract=fast' [-Woverriding-t-option]<br>
<br>
-// RUN: %clang -### -ffp-model=strict -ffp-contract=off -c %s 2>&1 \<br>
+// RUN: %clang -### -ffp-model=strict -ffp-contract=fast -c %s 2>&1 \<br>
// RUN: | FileCheck --check-prefix=WARN6 %s<br>
-// WARN6: warning: overriding '-ffp-model=strict' option with '-ffp-contract=off' [-Woverriding-t-option]<br>
+// WARN6: warning: overriding '-ffp-model=strict' option with '-ffp-contract=fast' [-Woverriding-t-option]<br>
<br>
// RUN: %clang -### -ffp-model=strict -ffp-contract=on -c %s 2>&1 \<br>
// RUN: | FileCheck --check-prefix=WARN7 %s<br>
@@ -100,13 +100,14 @@<br>
// RUN: %clang -### -nostdinc -ffp-model=precise -c %s 2>&1 \<br>
// RUN: | FileCheck --check-prefix=CHECK-FPM-PRECISE %s<br>
// CHECK-FPM-PRECISE: "-cc1"<br>
-// CHECK-FPM-PRECISE: "-ffp-contract=fast"<br>
+// CHECK-FPM-PRECISE: "-ffp-contract=on"<br>
// CHECK-FPM-PRECISE: "-fno-rounding-math"<br>
<br>
// RUN: %clang -### -nostdinc -ffp-model=strict -c %s 2>&1 \<br>
// RUN: | FileCheck --check-prefix=CHECK-FPM-STRICT %s<br>
// CHECK-FPM-STRICT: "-cc1"<br>
// CHECK-FPM-STRICT: "-ftrapping-math"<br>
+// CHECK-FPM-STRICT: "-ffp-contract=off"<br>
// CHECK-FPM-STRICT: "-frounding-math"<br>
// CHECK-FPM-STRICT: "-ffp-exception-behavior=strict"<br>
<br>
<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div>