Change clang option -ffp-model=precise to select ffp-contract=on

[mibintc]

Summary

This patch changes the ffp-model=precise to enables -ffp-contract=on (previously -ffp-model=precise enabled -ffp-contract=fast). This is a follow-up to Andy Kaylor's comments in the llvm-dev discussion "Floating Point semantic modes". From the same email thread, I put Andy's distillation of floating point options and floating point modes into UsersManual.rst

[lebedev.ri]

clang/docs/UsersManual.rst

1459

I'm confused. Where in this patch the This patch establishes the default option for -ffp-model to select "precise". happens? LHS of diff says it is already default

[rjmccall]

I agree that the default mode should be standards-compliant, which I believe permits fp-contract=on (contraction within expressions) but not fp-contract=fast (arbitrary contraction).

[andrew.w.kaylor]

clang/docs/UsersManual.rst

1459

The comments said that it was the default, but the actual default was something that didn't quite match any of the fp-models -- precise but with fp contraction off.

clang/lib/Driver/ToolChains/Clang.cpp

2666–2672

I think this always gets changed to fast, on, or off below, but making it empty here looks wrong.

2783–2787

Does this mean that "-ffp-model=precise -ffp-contract=off" will leave FP contraction on? That doesn't seem right.

2890–2895

We should never get here with FPContract empty.

[mibintc]

some replies to Andy. I'll upload another patch here which passed check-all locally. then i'll re-commit it.

clang/docs/UsersManual.rst

1459

yes you're right. sorry for the confusion. i changed the title and description

clang/lib/Driver/ToolChains/Clang.cpp

2783–2787

No. When -ffp-model=precise is on the command line, we end up here because the code above changes optID to ffp_contract, and sets FPContract to "on". The Val string will be "precise", and it shouldn't be checked. I'll change the comment.

[andrew.w.kaylor]

lgtm

[mstorsjo]

FYI this caused failed builds for aarch64 (when building with -fno-signed-zeros), hitting unreachable statements in TargetLowering::getNegatedExpression - see https://bugs.llvm.org/show_bug.cgi?id=44892. I guess that's a bug in the aarch64 backend and not with this change in itself, but it was only uncovered by this change...

[xbolva00]

Looks like this patch regressed the test-suite:
https://lnt.llvm.org/db_default/v4/nts/recent_activity

Now 45min+

[jsji]

This is also breaking test-suites in our internal buildbots ..

This change is actually changing the default behavior: if user does NOT supply -ffp-contract, it becomes -ffp-contract=on.

So this means any code without explicitly add -ffp-contract=off will have -ffp-contract=on.
eg: -O0 will also generate FMA.

Is this intended behavior change? @mibintc @andrew.w.kaylor

[mibintc]

In D74436#1875273, @jsji wrote:

This is also breaking test-suites in our internal buildbots ..

This change is actually changing the default behavior: if user does NOT supply -ffp-contract, it becomes -ffp-contract=on.

So this means any code without explicitly add -ffp-contract=off will have -ffp-contract=on.
eg: -O0 will also generate FMA.

Is this intended behavior change? @mibintc @andrew.w.kaylor

You're right, -O0 shouldn't generate FMA. I'm preparing to revert this now -- just verifying the build.

[nemanjai]

You're right, -O0 shouldn't generate FMA. I'm preparing to revert this now -- just verifying the build.

Perhaps this should be
off with no optimization
on with -O1/-O2/-O3/-Os/-Oz
fast with fast math

Just a suggestion, I'm not sure whether that would be the best breakdown. Perhaps we can also see what the defaults are for GCC and unify with those?

[mibintc]

@jsji Will your backend tolerate -ffp-contract=on if optimizations are not disabled, e.g. -O2? We are proposing that the default floating point model be -ffp-model=precise, and with precise model, the ffp-contract=on. However you are right we don't want the frontend to create FMA when optimizations are disabled -O0

[thakis]

The revert of this breaks tests everywhere, as far as I can tell.

[andrew.w.kaylor]

In D74436#1875386, @thakis wrote:

The revert of this breaks tests everywhere, as far as I can tell.

It looks like something strange happened with the revert:

clang-11: warning: overriding '-ffp-model=strict' option with '-ffp-model=strict' [-Woverriding-t-option]

I believe the problem is that the original change that was being reverted contained this:

clang/lib/Driver/ToolChains/Clang.cpp 
@@ -2768,7 +2766,7 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
        !AssociativeMath && !ReciprocalMath &&
        SignedZeros && TrappingMath && RoundingFPMath &&
        DenormalFPMath != llvm::DenormalMode::getIEEE() &&
+        FPContract.empty())
-        (FPContract.equals("off") || FPContract.empty()))

But sometime in the land-revert-land-revert cycle the line above that changed, causing the merge to miss this change in the most recent revert. I see that @MaskRay has since re-landed this change set, but it's going to cause problems for PowerPC. If someone needs to revert this yet again, I think it can be safely done by recovering the change above.

Apologies for the mess!

[jsji]

In D74436#1875320, @mibintc wrote:

@jsji Will your backend tolerate -ffp-contract=on if optimizations are not disabled, e.g. -O2? We are proposing that the default floating point model be -ffp-model=precise, and with precise model, the ffp-contract=on. However you are right we don't want the frontend to create FMA when optimizations are disabled -O0

We normally default to -ffp-contract=on only for -O3 and above (for legacy compilers XL/gcc on PowerPC).
But yes, I think we are OK to accept -ffp-contract=on (not -ffp-contract=fast) at -O2 if there are strong justifications to do so on clang.

[andrew.w.kaylor]

In D74436#1875320, @mibintc wrote:

However you are right we don't want the frontend to create FMA when optimizations are disabled -O0

I've been discussing this with @scanon on the cfe-dev mailing list, and he has convinced me that we should create FMA options at -O0 if the fp-contract setting is "on" -- including when it is on by default. The reasoning that persuaded me was, "preserving FMA formation at O0 _helps_ debuggability, because it means that numerical behavior is more likely to match what a user observed at Os, allowing them to debug the problem."

[rjmccall]

Yeah, I agree with Steve here. The great virtue of -ffp-contract=on over the more aggressive modes is that FMA formation is purely local to a single source expression, which means there's really no obstacle to treating it as implementation-guaranteed semantics. Such behavior should be done consistently across compiler settings rather than potentially making the numeric result of an expression subject to optimizer settings and choices; the latter is fine for "fast" modes but not really acceptable as the default compiler behavior. If a backend needs to legalize the intrinsic back to separate multiply + add instructions, that's okay, but it should do so consistently rather than only under a specific optimization level.

[jsji]

We normally default to -ffp-contract=on only for -O3 and above (for legacy compilers XL/gcc on PowerPC).

Correction: XL on PowerPC default to generate FMA at -O0 as well. The corresponding option is -qfloat=maf.
https://www.ibm.com/support/knowledgecenter/SSXVZZ_16.1.1/com.ibm.xlcpp1611.lelinux.doc/compiler_ref/opt_float.html

[andrew.w.kaylor]

In D74436#1875315, @nemanjai wrote:

You're right, -O0 shouldn't generate FMA. I'm preparing to revert this now -- just verifying the build.

Perhaps this should be
off with no optimization
on with -O1/-O2/-O3/-Os/-Oz
fast with fast math

Just a suggestion, I'm not sure whether that would be the best breakdown. Perhaps we can also see what the defaults are for GCC and unify with those?

GCC doesn't support "on" so I'm not sure it's possible to distinguish their intentions. I think it would be better to define what we think is best for clang and encourage GCC to unify with that.

[xbolva00]

Perhaps we can also see what the defaults are for GCC and unify with those?

+1

[fhahn]

IIUC the patch is currently reverted (in 78654e8511cf16d49f6680d782f3771a767ba942), due to ~20 llvm-test-suite failures.

The failures on X86 are listed below. Unfortunately most tests just compare the hash of the output, so it is not obvious in what way the results are changed. Any ideas on how to move forward on this? I'll update the diff with a rebased version on current trunk, which I used to reproduce the failures.

Failed Tests (22):
  test-suite :: MultiSource/Applications/oggenc/oggenc.test
  test-suite :: MultiSource/Benchmarks/DOE-ProxyApps-C++/CLAMR/CLAMR.test
  test-suite :: MultiSource/Benchmarks/DOE-ProxyApps-C++/HPCCG/HPCCG.test
  test-suite :: MultiSource/Benchmarks/DOE-ProxyApps-C++/miniFE/miniFE.test
  test-suite :: MultiSource/Benchmarks/Rodinia/srad/srad.test
  test-suite :: SingleSource/Benchmarks/Linpack/linpack-pc.test
  test-suite :: SingleSource/Benchmarks/Misc-C++/Large/sphereflake.test
  test-suite :: SingleSource/Benchmarks/Polybench/datamining/correlation/correlation.test
  test-suite :: SingleSource/Benchmarks/Polybench/datamining/covariance/covariance.test
  test-suite :: SingleSource/Benchmarks/Polybench/linear-algebra/kernels/2mm/2mm.test
  test-suite :: SingleSource/Benchmarks/Polybench/linear-algebra/kernels/3mm/3mm.test
  test-suite :: SingleSource/Benchmarks/Polybench/linear-algebra/kernels/atax/atax.test
  test-suite :: SingleSource/Benchmarks/Polybench/linear-algebra/kernels/bicg/bicg.test
  test-suite :: SingleSource/Benchmarks/Polybench/linear-algebra/kernels/gemver/gemver.test
  test-suite :: SingleSource/Benchmarks/Polybench/linear-algebra/kernels/gesummv/gesummv.test
  test-suite :: SingleSource/Benchmarks/Polybench/linear-algebra/kernels/symm/symm.test
  test-suite :: SingleSource/Benchmarks/Polybench/linear-algebra/kernels/trisolv/trisolv.test
  test-suite :: SingleSource/Benchmarks/Polybench/linear-algebra/kernels/trmm/trmm.test
  test-suite :: SingleSource/Benchmarks/Polybench/linear-algebra/solvers/gramschmidt/gramschmidt.test
  test-suite :: SingleSource/Benchmarks/Polybench/stencils/adi/adi.test
  test-suite :: SingleSource/UnitTests/Vector/SSE/Vector-sse.expandfft.test
  test-suite :: SingleSource/UnitTests/Vector/SSE/Vector-sse.stepfft.test

[fhahn]

Rebase

[lebedev.ri]

In D74436#2190422, @fhahn wrote:

IIUC the patch is currently reverted (in 78654e8511cf16d49f6680d782f3771a767ba942), due to ~20 llvm-test-suite failures.

The failures on X86 are listed below. Unfortunately most tests just compare the hash of the output, so it is not obvious in what way the results are changed.

Presumably it means that whatever optimization options those test specify, are now either more strict or less strict.
I think this is really missing some codegen tests that show how codegen actually changes by this.

Any ideas on how to move forward on this? I'll update the diff with a rebased version on current trunk, which I used to reproduce the failures.

Failed Tests (22):
  test-suite :: MultiSource/Applications/oggenc/oggenc.test
  test-suite :: MultiSource/Benchmarks/DOE-ProxyApps-C++/CLAMR/CLAMR.test
  test-suite :: MultiSource/Benchmarks/DOE-ProxyApps-C++/HPCCG/HPCCG.test
  test-suite :: MultiSource/Benchmarks/DOE-ProxyApps-C++/miniFE/miniFE.test
  test-suite :: MultiSource/Benchmarks/Rodinia/srad/srad.test
  test-suite :: SingleSource/Benchmarks/Linpack/linpack-pc.test
  test-suite :: SingleSource/Benchmarks/Misc-C++/Large/sphereflake.test
  test-suite :: SingleSource/Benchmarks/Polybench/datamining/correlation/correlation.test
  test-suite :: SingleSource/Benchmarks/Polybench/datamining/covariance/covariance.test
  test-suite :: SingleSource/Benchmarks/Polybench/linear-algebra/kernels/2mm/2mm.test
  test-suite :: SingleSource/Benchmarks/Polybench/linear-algebra/kernels/3mm/3mm.test
  test-suite :: SingleSource/Benchmarks/Polybench/linear-algebra/kernels/atax/atax.test
  test-suite :: SingleSource/Benchmarks/Polybench/linear-algebra/kernels/bicg/bicg.test
  test-suite :: SingleSource/Benchmarks/Polybench/linear-algebra/kernels/gemver/gemver.test
  test-suite :: SingleSource/Benchmarks/Polybench/linear-algebra/kernels/gesummv/gesummv.test
  test-suite :: SingleSource/Benchmarks/Polybench/linear-algebra/kernels/symm/symm.test
  test-suite :: SingleSource/Benchmarks/Polybench/linear-algebra/kernels/trisolv/trisolv.test
  test-suite :: SingleSource/Benchmarks/Polybench/linear-algebra/kernels/trmm/trmm.test
  test-suite :: SingleSource/Benchmarks/Polybench/linear-algebra/solvers/gramschmidt/gramschmidt.test
  test-suite :: SingleSource/Benchmarks/Polybench/stencils/adi/adi.test
  test-suite :: SingleSource/UnitTests/Vector/SSE/Vector-sse.expandfft.test
  test-suite :: SingleSource/UnitTests/Vector/SSE/Vector-sse.stepfft.test

[lebedev.ri]

In D74436#2190492, @lebedev.ri wrote:

<...>

And by codegen changes i mostly mean newly-set/now-unset fp fast-math instruction flags.

[xbolva00]

Status?

[mibintc]

I rebased and enhanced the test case clang/test/CodeGen/ffp-contract-option.c to show the effect of various ffp-contract={on,fast,off} * ffast-math=on,off in response to the request from @lebedev.ri ; sorry for leaving this hanging for so long, I wasn't sure what to do about the optimization discrepancies but I have a plan now. We think it could be caused by a particular nuance on Broadwell.

[mibintc]

There are 3 clang settings for ffp-contract (on, off, fast) but the FMF bits have only "allow contract". Clang sets the "allow contract" bit in the IRBuilder only when ffp-contract=fast

[mibintc]

In D74436#2190566, @lebedev.ri wrote:

In D74436#2190492, @lebedev.ri wrote:

<...>

And by codegen changes i mostly mean newly-set/now-unset fp fast-math instruction flags.

I added some codegen tests to show you what ffp-contract is doing: the tests added are: ffp-contract={on,off,fast} * {ffast-math * fno-fast-math}

[mibintc]

I created https://reviews.llvm.org/D102861 to make changes to the failing LNT tests. Hoping to push this commit after the test changes in D102862 are approved.

[mibintc]

Hoping @lebedev.ri will take a look since he requested changes, thanks!

[lebedev.ri]

No real comments from me.
I assume, the errors are because -ffp-contract=on actually results in *less* error?

[mibintc]

In D74436#2785730, @lebedev.ri wrote:

No real comments from me.
I assume, the errors are because -ffp-contract=on actually results in *less* error?

Yes, FMA improves accuracy, thank you!

[xbolva00]

clang/docs/UsersManual.rst

1501

Maybe you should add short info to Release Notes as well?

[mibintc]

clang/docs/UsersManual.rst

1501

Maybe you should add short info to Release Notes as well?

Good idea thanks. Can I do that in a separate patch?

[xbolva00]

clang/docs/UsersManual.rst

1501

Yes, maybe wait a few days in case of the revert of this patch.

[mibintc]

The bot is showing a fail due to this patch, see https://lab.llvm.org/buildbot#builders/110/builds/4007

It looks like my updates to LNT earlier this week haven't been migrated to the bot, is that right?

See https://reviews.llvm.org/rTccc86f938839d02de3a6564b2d80fb47a60aa069

In the json log I can see ffp-contract=off but it should be overridden to ffp-contract=off on the command line due to the LNT change.

Is there something else I need to do to update those sources?

[mibintc]

I’ve been trying to commit this patch. However, when I commit, the tests fail some 21 floating point tests in LNT. Also there is both performance improvement as well as regression which shouldn’t happen because the sum total of my clang patch + my LNT patch should effectively be a no-op.

It appears that the correction I made to LNT is not effective, but I’ve tested the patch on my Linux box and it does work. My patch unconditionally overrides the command line to restore the original LNT behavior. I verified the commit revision to LNT and the bot is running my patched version of LNT. It looks like the bot is using a different rule to build the test cases.

It’s hard to see exactly what the bot is doing, there isn’t enough information. It would be nice if the bot Makefile was built with ‘verbose’ so I could see all the compile commands that the bot is using.

One of the bot failures is here: http://lnt.llvm.org/db_default/v4/nts/147167

I can see the bot owner is Yin Yang, I will reach out.

[mibintc]

I had to revert again, it's still failing on Intel buildbots. Cannot reproduce on Intel-internal Broadwell server.

[haowei]

We are seeing float related unit tests failing in Fuchsia after this patch. What flags should we add to our build to match llvm's previous float point behavior?

[mibintc]

In D74436#2917357, @haowei wrote:

We are seeing float related unit tests failing in Fuchsia after this patch. What flags should we add to our build to match llvm's previous float point behavior?

This option: ffp-contract=off

[xbolva00]

Seems like lnt related things are stable now so maybe it is a good time to update release notes?

[hans]

In D74436#2917395, @mibintc wrote:

In D74436#2917357, @haowei wrote:

We are seeing float related unit tests failing in Fuchsia after this patch. What flags should we add to our build to match llvm's previous float point behavior?

This option: ffp-contract=off

We're seeing float related tests fail in Chromium too (crbug.com/1235145). I haven't read the background llvm-dev thread about this, but is changing the default behavior here really intentional? Does it match what the standards require and what other compilers do?

[fhahn]

It looks like this patch is also causing mis-compares in both SPEC2006 and SPEC2017 on AArch64, e.g. in External/SPEC/CFP2006/453.povray/.

[haowei]

Can we revert this patch please? If I understood correctly, this patch changed the default behavior of clang when generating code for float point arithmetic. After this patch -ffp-contract was set to on if this flag was not specified. In theory this makes clang use FMA instructions that should be slightly more efficient, but there are cases in the field that certain programs relies on the stable rounding behavior of float point arithmetic, and this patch broke that. And in fact we already seen breakages in Fuchsia, Chromium and SPEC benchmark suite and there might be other people seeing the same breakages but still working on bisecting this breaking change.

Even if this breaking change is reasonable to proceed and receive condense from major clang users, this patch lacks enough test coverage for codegen of float point arithmetic on major CPU platforms without related flag specified. Nobody will discover this is a default behavior breaking patch by just reading this patch. This should be reverted and reworked.

[hans]

Also +@tstellar since I believe this went in before the 13 branch point.

[mibintc]

There was a long discussion on cfe-dev about this issue approximately January-February 2020 including @scanon @andrew.w.kaylor and many others. While there wasn't 100% consensus to move to ffp-contract=on there are many reasons to do it including better numerical results and debuggability. I'm hoping that @andrew.w.kaylor @kbsmith1 @rjmccall and others will join in here.

[probinson]

I see that the UserManual has been updated, but this behavior change should probably also be mentioned in the Release Notes.

[tstellar]

Can someone file a bug for this and put release-13.0.0 in the blocks field so we can track it?

[haowei]

In D74436#2926385, @tstellar wrote:

Can someone file a bug for this and put release-13.0.0 in the blocks field so we can track it?

Filed https://bugs.llvm.org/show_bug.cgi?id=51346 .

[zahiraam]

Filed bug to edit the release not as suggested by @xbolva00 here: https://bugs.llvm.org/show_bug.cgi?id=51347

[andrew.w.kaylor]

FWIW, fp-contract=on has been the documented default for clang since version 5.

https://releases.llvm.org/5.0.1/tools/clang/docs/ClangCommandLineReference.html#cmdoption-clang-ffp-contract

This change just brought the behavior into conformance with the documentation.

[zahiraam]

This patch had to be reverted because SPEC2006 and SPEC2017 are failing.
More information here:
https://bugs.llvm.org/show_bug.cgi?id=51346

clang/docs/UsersManual.rst

 and the precompiled header cannot be generated after headers have been
 installed.
 
 .. _controlling-fp-behavior:
 
 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
 
    Enable fast-math mode.  This option lets the
    compiler make aggressive, potentially-lossy assumptions about
    floating-point math.  These include:
 
    * Floating-point math obeys regular algebraic rules for real numbers (e.g.

 .. option:: -ffp-model=<value>
 
    Specify floating point behavior. ``-ffp-model`` is an umbrella
    option that encompasses functionality provided by other, single
    purpose, floating point options.  Valid values are: ``precise``, ``strict``,
    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. Enables ``STDC FENV_ACCESS``: by default ``FENV_ACCESS`` is disabled. This option setting behaves as though ``#pragma STDC FENV_ACESS ON`` appeared at the top of the source file.
    * ``fast`` Behaves identically to specifying both ``-ffast-math`` and ``ffp-contract=fast``
 
    Note: If your command line specifies multiple instances
    of the ``-ffp-model`` option, or if your command line option specifies
    ``-ffp-model`` and later on the command line selects a floating point
    option that has the effect of negating part of the  ``ffp-model`` that
    has been selected, then the compiler will issue a diagnostic warning

clang/lib/Driver/ToolChains/Clang.cpp

static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
   StringRef FPExceptionBehavior = "";
   const llvm::DenormalMode DefaultDenormalFPMath =
       TC.getDefaultDenormalModeForType(Args, JA);
   const llvm::DenormalMode DefaultDenormalFP32Math =
       TC.getDefaultDenormalModeForType(Args, JA, &llvm::APFloat::IEEEsingle());
 
   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());
   }
 
   for (const Arg *A : Args) {
     auto optID = A->getOption().getID();
     bool PreciseFPModel = false;
     switch (optID) {
     default:
       break;
     case options::OPT_ffp_model_EQ: {
       // If -ffp-model= is seen, reset to fno-fast-math
       HonorINFs = true;
       HonorNaNs = true;
       // Turning *off* -ffast-math restores the toolchain default.
       MathErrno = TC.IsMathErrnoDefault();
       AssociativeMath = false;
       ReciprocalMath = false;
       SignedZeros = true;
       // -fno_fast_math restores default denormal and fpcontract handling
-      FPContract = "";
+      FPContract = "on";
       DenormalFPMath = llvm::DenormalMode::getIEEE();
 
       // FIXME: The target may have picked a non-IEEE default mode here based on
       // -cl-denorms-are-zero. Should the target consider -fp-model interaction?
       DenormalFP32Math = llvm::DenormalMode::getIEEE();
 
       StringRef Val = A->getValue();
       if (OFastEnabled && !Val.equals("fast")) {
           // Only -ffp-model=fast is compatible with OFast, ignore.
         D.Diag(clang::diag::warn_drv_overriding_flag_option)
           << Args.MakeArgString("-ffp-model=" + Val)
           << "-Ofast";
         break;
       }
       StrictFPModel = false;
       PreciseFPModel = true;
       // 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;
-        FPContract = "fast";
+        FPContract = Val;
       } 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;
case options::OPT_fdenormal_fp_math_f32_EQ:
             << A->getAsString(Args) << A->getValue();
       }
       break;
 
     // Validate and pass through -ffp-contract option.
     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;
       else
         D.Diag(diag::err_drv_unsupported_option_argument)
            << A->getOption().getName() << Val;
       break;
     }
case options::OPT_fno_fast_math:
       AssociativeMath = false;
       ReciprocalMath = false;
       SignedZeros = true;
       TrappingMath = false;
       RoundingFPMath = false;
       // -fno_fast_math restores default denormal and fpcontract handling
       DenormalFPMath = DefaultDenormalFPMath;
       DenormalFP32Math = llvm::DenormalMode::getIEEE();
-      FPContract = "";
+      FPContract = "on";
       break;
     }
     if (StrictFPModel) {
       // If -ffp-model=strict has been specified on command line but
       // subsequent options conflict then emit warning diagnostic.
-      if (HonorINFs && HonorNaNs &&
-        !AssociativeMath && !ReciprocalMath &&
+      if (HonorINFs && HonorNaNs && !AssociativeMath && !ReciprocalMath &&
           SignedZeros && TrappingMath && RoundingFPMath &&
-        (FPContract.equals("off") || FPContract.empty()) &&
           DenormalFPMath == llvm::DenormalMode::getIEEE() &&
-        DenormalFP32Math == llvm::DenormalMode::getIEEE())
+          DenormalFP32Math == llvm::DenormalMode::getIEEE() &&
+          FPContract.equals("off"))
         // OK: Current Arg doesn't conflict with -ffp-model=strict
         ;
       else {
         StrictFPModel = false;
         FPModel = "";
         D.Diag(clang::diag::warn_drv_overriding_flag_option)
             << "-ffp-model=strict" <<
             ((A->getNumValues() == 0) ?  A->getSpelling()

clang/test/CodeGen/ffp-contract-option.c

-// RUN: %clang_cc1 -O3 -ffp-contract=fast -triple=aarch64-apple-darwin -S -o - %s | FileCheck %s
+// RUN: %clang_cc1 -O3 -ffp-contract=fast -triple=aarch64-apple-darwin -S -o - %s | FileCheck --check-prefix=CHECK-FMADD %s
 // REQUIRES: aarch64-registered-target
 
 float fma_test1(float a, float b, float c) {
-// CHECK: fmadd
-  float x = a * b;
-  float y = x + c;
-  return y;
+// CHECK-FMADD: fmadd
 }
+
+// RUN: %clang_cc1 -triple=x86_64 %s -emit-llvm -o - \
+// RUN:| FileCheck --check-prefix=CHECK-DEFAULT %s
+//
+// RUN: %clang_cc1 -triple=x86_64 -ffp-contract=off %s -emit-llvm -o - \
+// RUN:| FileCheck --check-prefix=CHECK-DEFAULT %s
+// RUN: %clang_cc1 -triple=x86_64 -ffp-contract=on %s -emit-llvm -o - \
+// RUN:| FileCheck --check-prefix=CHECK-ON %s
+// RUN: %clang_cc1 -triple=x86_64 -ffp-contract=fast %s -emit-llvm -o - \
+// RUN:| FileCheck --check-prefix=CHECK-CONTRACTFAST %s
+//
+// RUN: %clang_cc1 -triple=x86_64 -ffast-math %s -emit-llvm -o - \
+// RUN:| FileCheck --check-prefix=CHECK-DEFAULTFAST %s
+// RUN: %clang_cc1 -triple=x86_64 -ffast-math -ffp-contract=off %s -emit-llvm -o - \
+// RUN:| FileCheck --check-prefix=CHECK-DEFAULTFAST %s
+// RUN: %clang_cc1 -triple=x86_64 -ffast-math -ffp-contract=on %s -emit-llvm -o - \
+// RUN:| FileCheck --check-prefix=CHECK-ONFAST %s
+// RUN: %clang_cc1 -triple=x86_64 -ffast-math -ffp-contract=fast %s -emit-llvm -o - \
+// RUN:| FileCheck --check-prefix=CHECK-FASTFAST %s
+float mymuladd( float x, float y, float z ) {
+  return x * y + z;
+  // CHECK-DEFAULT: = fmul float
+  // CHECK-DEFAULT: = fadd float
+  
+  // CHECK-ON: = call float @llvm.fmuladd.f32
+ 
+  // CHECK-CONTRACTFAST: = fmul contract float
+  // CHECK-CONTRACTFAST: = fadd contract float
+
+  // CHECK-DEFAULTFAST: = fmul reassoc nnan ninf nsz arcp afn float
+  // CHECK-DEFAULTFAST: = fadd reassoc nnan ninf nsz arcp afn float
+
+  // CHECK-ONFAST: = call reassoc nnan ninf nsz arcp afn float @llvm.fmuladd.f32
+
+  // CHECK-FASTFAST: = fmul fast float
+  // CHECK-FASTFAST: = fadd fast float
+}

clang/test/CodeGen/ppc-emmintrin.c

 // NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
 // 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
 // CHECK-BE-DAG: @_mm_shufflehi_epi16.permute_selectors = internal constant [4 x i16] [i16 2057, i16 2571, i16 3085, i16 3599], align 2
 // CHECK-BE-DAG: @_mm_shufflelo_epi16.permute_selectors = internal constant [4 x i16] [i16 1, i16 515, i16 1029, i16 1543], align 2
 
 // CHECK-LE-DAG: @_mm_movemask_pd.perm_mask = internal constant <4 x i32> <i32 -2139094976, i32 -2139062144, i32 -2139062144, i32 -2139062144>, align 16
 // CHECK-LE-DAG: @_mm_shuffle_epi32.permute_selectors = internal constant [4 x i32] [i32 50462976, i32 117835012, i32 185207048, i32 252579084], align 4

clang/test/CodeGen/ppc-xmmintrin.c

 // NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
 // 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 -x c++ -fsyntax-only -target powerpc64-unknown-linux-gnu -mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
 // RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns
 // 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
 // RUN: %clang -x c++ -fsyntax-only -target powerpc64le-unknown-linux-gnu -mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
 // RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns
 
 #include <xmmintrin.h>
 
 __m128 res, m1, m2;
 __m64 res64, ms[2];
 float fs[4];

clang/test/Driver/fp-model.c

 // Test that incompatible combinations of -ffp-model= options
 // and other floating point options get a warning diagnostic.
-//
-// REQUIRES: clang-driver
 
-// RUN: %clang -### -ffp-model=fast -ffp-contract=off -c %s 2>&1 \
+// RUN: %clang -target x86_64 -### -ffp-model=fast -ffp-contract=off -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=WARN %s
 // WARN: warning: overriding '-ffp-model=fast' option with '-ffp-contract=off' [-Woverriding-t-option]
 
-// RUN: %clang -### -ffp-model=fast -ffp-contract=on -c %s 2>&1 \
+// RUN: %clang -target x86_64 -### -ffp-model=fast -ffp-contract=on -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=WARN1 %s
 // WARN1: warning: overriding '-ffp-model=fast' option with '-ffp-contract=on' [-Woverriding-t-option]
 
-// RUN: %clang -### -ffp-model=strict -fassociative-math -c %s 2>&1 \
+// RUN: %clang -target x86_64 -### -ffp-model=strict -fassociative-math -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=WARN2 %s
 // WARN2: warning: overriding '-ffp-model=strict' option with '-fassociative-math' [-Woverriding-t-option]
 
-// RUN: %clang -### -ffp-model=strict -ffast-math -c %s 2>&1 \
+// RUN: %clang -target x86_64 -### -ffp-model=strict -ffast-math -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=WARN3 %s
 // WARN3: warning: overriding '-ffp-model=strict' option with '-ffast-math' [-Woverriding-t-option]
 
-// RUN: %clang -### -ffp-model=strict -ffinite-math-only -c %s 2>&1 \
+// RUN: %clang -target x86_64 -### -ffp-model=strict -ffinite-math-only -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=WARN4 %s
 // WARN4: warning: overriding '-ffp-model=strict' option with '-ffinite-math-only' [-Woverriding-t-option]
 
-// RUN: %clang -### -ffp-model=strict -ffp-contract=fast -c %s 2>&1 \
+// RUN: %clang -target x86_64 -### -ffp-model=strict -ffp-contract=fast -c %s 2>&1 \
 // 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=on -c %s 2>&1 \
+// RUN: %clang -target x86_64 -### -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=fast' [-Woverriding-t-option]
+
+// RUN: %clang -target x86_64 -### -ffp-model=strict -ffp-contract=on -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=WARN7 %s
 // WARN7: warning: overriding '-ffp-model=strict' option with '-ffp-contract=on' [-Woverriding-t-option]
 
-// RUN: %clang -### -ffp-model=strict -fno-honor-infinities -c %s 2>&1 \
+// RUN: %clang -target x86_64 -### -ffp-model=strict -fno-honor-infinities -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=WARN8 %s
 // WARN8: warning: overriding '-ffp-model=strict' option with '-fno-honor-infinities' [-Woverriding-t-option]
 
-// RUN: %clang -### -ffp-model=strict -fno-honor-nans -c %s 2>&1 \
+// RUN: %clang -target x86_64 -### -ffp-model=strict -fno-honor-nans -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=WARN9 %s
 // WARN9: warning: overriding '-ffp-model=strict' option with '-fno-honor-nans' [-Woverriding-t-option]
 
-// RUN: %clang -### -ffp-model=strict -fno-rounding-math -c %s 2>&1 \
+// RUN: %clang -target x86_64 -### -ffp-model=strict -fno-rounding-math -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=WARNa %s
 // WARNa: warning: overriding '-ffp-model=strict' option with '-fno-rounding-math' [-Woverriding-t-option]
 
-// RUN: %clang -### -ffp-model=strict -fno-signed-zeros -c %s 2>&1 \
+// RUN: %clang -target x86_64 -### -ffp-model=strict -fno-signed-zeros -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=WARNb %s
 // WARNb: warning: overriding '-ffp-model=strict' option with '-fno-signed-zeros' [-Woverriding-t-option]
 
-// RUN: %clang -### -ffp-model=strict -fno-trapping-math -c %s 2>&1 \
+// RUN: %clang -target x86_64 -### -ffp-model=strict -fno-trapping-math -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=WARNc %s
 // WARNc: warning: overriding '-ffp-model=strict' option with '-fno-trapping-math' [-Woverriding-t-option]
 
-// RUN: %clang -### -ffp-model=strict -freciprocal-math -c %s 2>&1 \
+// RUN: %clang -target x86_64 -### -ffp-model=strict -freciprocal-math -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=WARNd %s
 // WARNd: warning: overriding '-ffp-model=strict' option with '-freciprocal-math' [-Woverriding-t-option]
 
-// RUN: %clang -### -ffp-model=strict -funsafe-math-optimizations -c %s 2>&1 \
+// RUN: %clang -target x86_64 -### -ffp-model=strict -funsafe-math-optimizations -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=WARNe %s
 // WARNe: warning: overriding '-ffp-model=strict' option with '-funsafe-math-optimizations' [-Woverriding-t-option]
 
-// RUN: %clang -### -ffp-model=strict -Ofast -c %s 2>&1 \
+// RUN: %clang -target x86_64 -### -ffp-model=strict -Ofast -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=WARNf %s
 // WARNf: warning: overriding '-ffp-model=strict' option with '-Ofast' [-Woverriding-t-option]
 
-// RUN: %clang -### -ffp-model=strict -fdenormal-fp-math=preserve-sign,preserve-sign -c %s 2>&1 \
+// RUN: %clang -target x86_64 -### -ffp-model=strict -fdenormal-fp-math=preserve-sign,preserve-sign -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=WARN10 %s
 // WARN10: warning: overriding '-ffp-model=strict' option with '-fdenormal-fp-math=preserve-sign,preserve-sign' [-Woverriding-t-option]
 
-// RUN: %clang -### -c %s 2>&1 \
+// RUN: %clang -target x86_64 -### -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NOROUND %s
 // CHECK-NOROUND: "-cc1"
 // CHECK-NOROUND: "-fno-rounding-math"
 
-// RUN: %clang -### -frounding-math -c %s 2>&1 \
+// RUN: %clang -target x86_64 -### -frounding-math -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-ROUND --implicit-check-not ffp-exception-behavior=strict %s
 // CHECK-ROUND: "-cc1"
 // CHECK-ROUND: "-frounding-math"
 
-// RUN: %clang -### -ftrapping-math -c %s 2>&1 \
+// RUN: %clang -target x86_64 -### -ftrapping-math -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-TRAP %s
 // CHECK-TRAP: "-cc1"
 // CHECK-TRAP: "-ffp-exception-behavior=strict"
 
-// RUN: %clang -### -nostdinc -ffp-model=fast -c %s 2>&1 \
+// RUN: %clang -target x86_64 -### -nostdinc -ffp-model=fast -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FPM-FAST %s
 // CHECK-FPM-FAST: "-cc1"
 // CHECK-FPM-FAST: "-menable-no-infs"
 // CHECK-FPM-FAST: "-menable-no-nans"
 // CHECK-FPM-FAST: "-menable-unsafe-fp-math"
 // CHECK-FPM-FAST: "-fno-signed-zeros"
 // CHECK-FPM-FAST: "-mreassociate"
 // CHECK-FPM-FAST: "-freciprocal-math"
 // CHECK-FPM-FAST: "-ffp-contract=fast"
 // CHECK-FPM-FAST: "-fno-rounding-math"
 // CHECK-FPM-FAST: "-ffast-math"
 // CHECK-FPM-FAST: "-ffinite-math-only"
 
-// RUN: %clang -### -nostdinc -ffp-model=precise -c %s 2>&1 \
+// RUN: %clang -target x86_64 -### -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: %clang -target x86_64 -### -nostdinc -ffp-model=strict -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FPM-STRICT %s
 // CHECK-FPM-STRICT: "-cc1"
+// CHECK-FPM-STRICT: "-fmath-errno"
+// CHECK-FPM-STRICT: "-ffp-contract=off"
 // CHECK-FPM-STRICT: "-frounding-math"
 // CHECK-FPM-STRICT: "-ffp-exception-behavior=strict"
 
 
-// RUN: %clang -### -nostdinc -ffp-exception-behavior=strict -c %s 2>&1 \
+// RUN: %clang -target x86_64 -### -nostdinc -ffp-exception-behavior=strict -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FEB-STRICT %s
 // CHECK-FEB-STRICT: "-cc1"
 // CHECK-FEB-STRICT: "-fno-rounding-math"
 // CHECK-FEB-STRICT: "-ffp-exception-behavior=strict"
 
-// RUN: %clang -### -nostdinc -ffp-exception-behavior=maytrap -c %s 2>&1 \
+// RUN: %clang -target x86_64 -### -nostdinc -ffp-exception-behavior=maytrap -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FEB-MAYTRAP %s
 // CHECK-FEB-MAYTRAP: "-cc1"
 // CHECK-FEB-MAYTRAP: "-fno-rounding-math"
 // CHECK-FEB-MAYTRAP: "-ffp-exception-behavior=maytrap"
 
-// RUN: %clang -### -nostdinc -ffp-exception-behavior=ignore -c %s 2>&1 \
+// RUN: %clang -target x86_64 -### -nostdinc -ffp-exception-behavior=ignore -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FEB-IGNORE %s
 // CHECK-FEB-IGNORE: "-cc1"
 // CHECK-FEB-IGNORE: "-fno-rounding-math"
 // CHECK-FEB-IGNORE: "-ffp-exception-behavior=ignore"
-