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