[clang] abd0905 - Revert "Revert "Change clang option -ffp-model=precise to select ffp-contract=on""

Rumeet Dhindsa via cfe-commits cfe-commits at lists.llvm.org
Tue Feb 25 15:21:39 PST 2020


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 at 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 at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200225/73d37ad5/attachment-0001.html>


More information about the cfe-commits mailing list