[PATCH] D30582: [Driver] Restructure handling of -ffast-math and similar options
Renato Golin via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Mar 15 04:47:03 PDT 2017
rengolin added inline comments.
================
Comment at: lib/Driver/ToolChains/Clang.cpp:2452
+ if (!HonorInfs && !HonorNans && !MathErrno && AssociativeMath &&
+ ReciprocalMath && !SignedZeros && !TrappingMath && FpContract == "fast")
+ CmdArgs.push_back("-ffast-math");
----------------
rengolin wrote:
> This is technically correct, but users will be confused if they choose `-ffast-math -ffp-contract=on` and not see `-ffast-math` coming out on the other side.
>
> Also, `fp-contract=on` doesn't preclude `-ffast-math` for the languages that support it, so I wouldn't add `FpContract` to this list at all.
I've been thinking a bit more about this, and I started wondering, why do we even need to pass `-ffast-math` down?
If all the others are already being passed, shouldn't this flag be redundant?
Finally, we could possibly add instead `&& FpContract != "off"`. Would that be better?
================
Comment at: lib/Driver/ToolChains/Clang.cpp:2347
+ // Validate and pass through -fp-contract option.
+ case options::OPT_ffp_contract: {
+ StringRef Val = A->getValue();
----------------
Also, when `-ffast-math` is selected, and `-ffp-contract=on`, we should actually change it to `fast`, no?
================
Comment at: lib/Driver/ToolChains/Clang.cpp:2356
+ }
+
+ case options::OPT_ffinite_math_only:
----------------
Missing break?
Repository:
rL LLVM
https://reviews.llvm.org/D30582
More information about the cfe-commits
mailing list