rengolin added inline comments.

================
Comment at: lib/Driver/ToolChains/Clang.cpp:2320
+
+  for (Arg *A : Args)
+  {
----------------
format


================
Comment at: lib/Driver/ToolChains/Clang.cpp:2324
+    {
+    // Options controlling individual features
+    case options::OPT_fhonor_infinities:    HonorInfs = true;        break;
----------------
default should be the first item, for readability.


================
Comment at: lib/Driver/ToolChains/Clang.cpp:2382
+      if (!OFastEnabled)
+        continue;
+    case options::OPT_ffast_math:
----------------
Use `LLVM_FALLTHROUGH`


================
Comment at: lib/Driver/ToolChains/Clang.cpp:2452
+  if (!HonorInfs && !HonorNans && !MathErrno && AssociativeMath &&
+      ReciprocalMath && !SignedZeros && !TrappingMath && FpContract == "fast")
+    CmdArgs.push_back("-ffast-math");
----------------
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.


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

Reply via email to