[cfe-commits] [PATCH] Add -ffp-contract option.
Chandler Carruth
chandlerc at google.com
Thu Jul 5 17:27:59 PDT 2012
Arg, hit send too soon...
Index: include/clang/Basic/LangOptions.h
===================================================================
--- include/clang/Basic/LangOptions.h (revision 159591)
+++ include/clang/Basic/LangOptions.h (working copy)
@@ -56,6 +56,12 @@
SOB_Trapping // -ftrapv
};
+ enum FPContractModeTy {
+ FPC_Fast, // Aggressively fuse FP ops (E.g. FMA).
+ FPC_On, // Form fused FP ops according to FP_CONTRACT rules.
+ FPC_Off // Form fused FP ops only where result will not be
affected.
+ };
+
I would expect the reverse ordering. 0 -> off, and less-than or
greater-than having expected semantics.
Also, not sure if you want to switch in this patch, but the coding
conventions now clearly suggest naming this FPContractModeKind. Most
LangOpts just haven't been updated.
Index: lib/Frontend/CompilerInvocation.cpp
===================================================================
--- lib/Frontend/CompilerInvocation.cpp (revision 159591)
+++ lib/Frontend/CompilerInvocation.cpp (working copy)
@@ -759,6 +759,11 @@
Res.push_back("-ftrapv-handler", Opts.OverflowHandler);
break;
}
+ switch (Opts.getFPContractMode()) {
+ case LangOptions::FPC_Fast: Res.push_back("-ffp-contract=fast"); break;
+ case LangOptions::FPC_On: Res.push_back("-ffp-contract=on"); break;
+ case LangOptions::FPC_Off: Res.push_back("-ffp-contract=off"); break;
+ }
if (Opts.HeinousExtensions)
Res.push_back("-fheinous-gnu-extensions");
// Optimize is implicit.
@@ -1975,6 +1980,18 @@
Diags.Report(diag::err_drv_invalid_value)
<< Args.getLastArg(OPT_fvisibility)->getAsString(Args) << Vis;
+ if (Arg *A = Args.getLastArg(OPT_ffp_contract)) {
+ StringRef Val = A->getValue(Args);
+ if (Val == "fast")
+ Opts.setFPContractMode(LangOptions::FPC_Fast);
+ else if (Val == "on")
+ Opts.setFPContractMode(LangOptions::FPC_On);
+ else if (Val == "off")
+ Opts.setFPContractMode(LangOptions::FPC_Off);
+ else
+ Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args) <<
Val;
The frontend isn't supposed to use diagnostics designated for the driver
(err_drv_).
Index: lib/Driver/Tools.cpp
===================================================================
--- lib/Driver/Tools.cpp (revision 159591)
+++ lib/Driver/Tools.cpp (working copy)
@@ -1781,6 +1781,24 @@
!TrappingMath)
CmdArgs.push_back("-menable-unsafe-fp-math");
+
+ // Validate and pass through -fp-contract option.
+ if (Arg *A = Args.getLastArg(options::OPT_ffast_math,
+ options::OPT_ffp_contract)) {
+ if (A->getOption().getID() == options::OPT_ffp_contract) {
+ StringRef Val = A->getValue(Args);
+ if (Val == "fast" || Val == "on" || Val == "off") {
+ CmdArgs.push_back(Args.MakeArgString("-ffp-contract=" + Val));
+ } else {
+ D.Diag(diag::err_drv_unsupported_option_argument)
+ << A->getOption().getName() << Val;
+ }
+ } else { // A is OPT_ffast_math
I'd rather integrate this with the fast-math and friends below. It's a bit
unfortunate to check the arglist for an option in two separate places -- it
becomes easy for them to get out of sync.
Is it awkward to integrate this checking there?
In particular, it seems like there are several sub-flags of -ffast-math
that should still enable this...
On Thu, Jul 5, 2012 at 5:23 PM, Chandler Carruth <chandlerc at google.com>wrote:
> Couple of nit picks on the patch itself, no real issue with the plan:
>
> Index: test/CodeGen/fp-contract.c
> ===================================================================
> --- test/CodeGen/fp-contract.c (revision 0)
> +++ test/CodeGen/fp-contract.c (revision 0)
> @@ -0,0 +1,8 @@
> +// RUN: %clang_cc1 -O3 -ffp-contract=fast -triple=powerpc-apple-darwin10
> -S -o - %s | FileCheck %s
> +
> +float fma_test1(float a, float b, float c) {
> +// CHECK: fmadds
> + float x = a * b;
> + float y = x + c;
> + return y;
> +}
>
> I'd prefer to never check actual codegen from within the Clang test suite.
> I wonder if there is a way to get the backend libraries to print out their
> flags / state? Then we could just check that, and leave the actual
> functional test to the LLC-based tests in LLVM. Anyways, not needed for
> your patch -- this is a long standing deficit.
>
> Index: include/clang/Driver/Options.td
> ===================================================================
> --- include/clang/Driver/Options.td (revision 159591)
> +++ include/clang/Driver/Options.td (working copy)
> @@ -410,6 +410,12 @@
> def fno_honor_infinites : Flag<"-fno-honor-infinites">,
> Alias<fno_honor_infinities>;
> def ftrapping_math : Flag<"-ftrapping-math">, Group<f_Group>;
> def fno_trapping_math : Flag<"-fno-trapping-math">, Group<f_Group>;
> +def ffp_contract : Joined<"-ffp-contract=">, Group<f_Group>,
> + Flags<[CC1Option]>,
> + HelpText<"Form fused FP ops (e.g. FMAs): "
> + "fast (everywhere) | "
> + "on (according to FP_CONTRACT pragma,
> default) | "
> + "off (never fuse)">;
>
> The prevailing style in the options TD files is to maximize the columns
> w/o much regard for indentation:
>
> def ffp_contract: Joined<'-ffp-contract=">, Group<f_Group>,
> Flags<[CC1Option]>, HelpText<"...................."
> ".....">;
>
>
> On Mon, Jul 2, 2012 at 2:43 PM, Lang Hames <lhames at gmail.com> wrote:
>
>> Hi All,
>>
>> The attached patch adds a -ffp-contract=<style> option which (mostly)
>> follows the behavior of GCC's option. Styles are 'fast', for aggressive FMA
>> formation, 'on' for FP_CONTRACT compliance, and 'off' for no FMA formation
>> (in the future this might be relaxed to FMA formation in cases where the
>> result is provably unaffected).
>>
>> Feedback would be welcome. In particular, I have toggled the fp-contract
>> style based on the ffast-math option in Clang::ConstructJob(...) in
>> lib/Driver/Tools.cpp. Is that the right place to do that kind of option
>> fiddling? (It seems to be where the other fast-math related floating point
>> options get handled).
>>
>> Cheers,
>> Lang.
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120705/92b91d3f/attachment.html>
More information about the cfe-commits
mailing list