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

David Blaikie via cfe-commits cfe-commits at lists.llvm.org
Sat Mar 21 22:07:57 PDT 2020


Probably worth a bit more detail when recommitting a patch that was
reverted - in the future please include more detail about the reason for
the original revert and the specific changes to the previous version that
address that (so someone who might've reviewed the previous patch can more
quickly examine/assess the new changes compared to the previous version)

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, i32 -2139062144, i32 -2139062144, i32 -2139078656>, 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/20200321/5bbb7a42/attachment-0001.html>


More information about the cfe-commits mailing list