[clang] abd0905 - Revert "Revert "Change clang option -ffp-model=precise to select ffp-contract=on""
Blower, Melanie I via cfe-commits
cfe-commits at lists.llvm.org
Wed Feb 26 07:51:19 PST 2020
Rumeet, Thanks for the additional info. At some point we'd like to re-commit this patch. LNT testing showed execution time regressions on arch including x86 and aarch. I didn't capture the test names each time but it appeared to be approximately the same tests regressing everywhere. Andy Kaylor took a quick look at the x86 regressions and he said it looked like the loop unroller was making a bad decision. We are working out what to do now. FYI these are the x86 tests that failed on the LNT bot:
/home/ssglocal/clang-cmake-x86_64-sde-avx512-linux/clang-cmake-x86_64-sde-avx512-linux/test/sandbox/build/report.simple.csv
Importing 'report.json'
Import succeeded.
--- Tested: 2560 tests --
FAIL: MultiSource/Applications/oggenc/oggenc.execution_time (513 of 2560)
FAIL: MultiSource/Benchmarks/DOE-ProxyApps-C++/CLAMR/CLAMR.execution_time (514 of 2560)
FAIL: MultiSource/Benchmarks/DOE-ProxyApps-C++/HPCCG/HPCCG.execution_time (515 of 2560)
FAIL: MultiSource/Benchmarks/DOE-ProxyApps-C++/miniFE/miniFE.execution_time (516 of 2560)
FAIL: SingleSource/Benchmarks/Linpack/linpack-pc.execution_time (517 of 2560)
FAIL: SingleSource/Benchmarks/Misc-C++/Large/sphereflake.execution_time (518 of 2560)
FAIL: SingleSource/Benchmarks/Polybench/datamining/correlation/correlation.execution_time (519 of 2560)
FAIL: SingleSource/Benchmarks/Polybench/datamining/covariance/covariance.execution_time (520 of 2560)
FAIL: SingleSource/Benchmarks/Polybench/linear-algebra/kernels/2mm/2mm.execution_time (521 of 2560)
FAIL: SingleSource/Benchmarks/Polybench/linear-algebra/kernels/3mm/3mm.execution_time (522 of 2560)
FAIL: SingleSource/Benchmarks/Polybench/linear-algebra/kernels/atax/atax.execution_time (523 of 2560)
FAIL: SingleSource/Benchmarks/Polybench/linear-algebra/kernels/bicg/bicg.execution_time (524 of 2560)
FAIL: SingleSource/Benchmarks/Polybench/linear-algebra/kernels/gemver/gemver.execution_time (525 of 2560)
FAIL: SingleSource/Benchmarks/Polybench/linear-algebra/kernels/gesummv/gesummv.execution_time (526 of 2560)
FAIL: SingleSource/Benchmarks/Polybench/linear-algebra/kernels/symm/symm.execution_time (527 of 2560)
FAIL: SingleSource/Benchmarks/Polybench/linear-algebra/kernels/trisolv/trisolv.execution_time (528 of 2560)
FAIL: SingleSource/Benchmarks/Polybench/linear-algebra/kernels/trmm/trmm.execution_time (529 of 2560)
FAIL: SingleSource/Benchmarks/Polybench/linear-algebra/solvers/gramschmidt/gramschmidt.execution_time (530 of 2560)
FAIL: SingleSource/Benchmarks/Polybench/stencils/adi/adi.execution_time (531 of 2560)
FAIL: SingleSource/UnitTests/Vector/SSE/sse_expandfft.execution_time (532 of 2560)
FAIL: SingleSource/UnitTests/Vector/SSE/sse_stepfft.execution_time (533 of 2560)
> -----Original Message-----
> From: cfe-commits <cfe-commits-bounces at lists.llvm.org> On Behalf Of via cfe-
> commits
> Sent: Tuesday, February 25, 2020 6:24 PM
> To: cfe-commits at lists.llvm.org
> Subject: cfe-commits Digest, Vol 152, Issue 938
>
> Send cfe-commits mailing list submissions to
> cfe-commits at lists.llvm.org
>
> To subscribe or unsubscribe via the World Wide Web, visit
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> or, via email, send a message with subject or body 'help' to
> cfe-commits-request at lists.llvm.org
>
> You can reach the person managing the list at
> cfe-commits-owner at lists.llvm.org
>
> When replying, please edit your Subject line so it is more specific than "Re:
> Contents of cfe-commits digest..."
>
>
> Today's Topics:
>
> 1. [PATCH] D74912: [AArch64][SVE] Add SVE2 intrinsics for bit
> permutation & table lookup
> (Eli Friedman via Phabricator via cfe-commits)
> 2. Re: [clang] abd0905 - Revert "Revert "Change clang option
> -ffp-model=precise to select ffp-contract=on""
> (Rumeet Dhindsa via cfe-commits)
>
>
> ----------------------------------------------------------------------
>
> Message: 1
> Date: Tue, 25 Feb 2020 23:14:47 +0000 (UTC)
> From: Eli Friedman via Phabricator via cfe-commits
> <cfe-commits at lists.llvm.org>
> To: kerry.mclaughlin at arm.com, sander.desmalen at arm.com,
> andrzej.warzynski at arm.com, dancgr at gmail.com,
> cameron.mcinally at nyu.edu,
> efriedma at quicinc.com, rengolin at gmail.com
> Cc: hanna.kruppe at gmail.com, schuett at gmail.com, jun.l at samsung.com,
> llvm-commits at lists.llvm.org, kanheim at a-bix.com,
> cfe-commits at lists.llvm.org, snobl at codasip.com,
> simon.moll at emea.nec.com
> Subject: [PATCH] D74912: [AArch64][SVE] Add SVE2 intrinsics for bit
> permutation & table lookup
> Message-ID: <8ad204f6f5d906623adfa37862e78ee3 at localhost.localdomain>
> Content-Type: text/plain; charset=us-ascii
>
> efriedma accepted this revision.
> efriedma added a comment.
> This revision is now accepted and ready to land.
>
> LGTM
>
>
> CHANGES SINCE LAST ACTION
> https://reviews.llvm.org/D74912/new/
>
> https://reviews.llvm.org/D74912
>
>
>
>
>
> ------------------------------
>
> Message: 2
> Date: Tue, 25 Feb 2020 15:21:39 -0800
> From: Rumeet Dhindsa via cfe-commits <cfe-commits at lists.llvm.org>
> To: Melanie Blower <melanie.blower at intel.com>, Melanie Blower
> <llvmlistbot at llvm.org>
> Cc: cfe-commits at lists.llvm.org
> Subject: Re: [clang] abd0905 - Revert "Revert "Change clang option
> -ffp-model=precise to select ffp-contract=on""
> Message-ID:
> <CANx258uRL9BxY0Y8K=1bGgF0+q_NCnRtXqewoJimeK4hG1Uiuw at m
> ail.gmail.com>
> Content-Type: text/plain; charset="utf-8"
>
> 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/abd09053bc7aa6144873c196a7
> > d50aa6ce6ca430
> > DIFF:
> > https://github.com/llvm/llvm-
> project/commit/abd09053bc7aa6144873c196a7
> > d50aa6ce6ca430.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.html>
>
> ------------------------------
>
> Subject: Digest Footer
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
> ------------------------------
>
> End of cfe-commits Digest, Vol 152, Issue 938
> *********************************************
More information about the cfe-commits
mailing list