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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits