[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale added inline comments.


================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2408
+    case options::OPT_frounding_math:
+      // The default setting for frounding-math is True and ffast-math
+      // sets fno-rounding-math, but we only want to use constrained
----------------
Isn't the default `-fno-rounding-math`?


================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2416
+      RoundingFPMath = false;
+      RoundingMathPresent = false;
+      break;
----------------
Shouldn't this be set to `true` similarly to what you do for `TrappingMathPresent ` to track whether there is an explicit option related to rounding math?


================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2574
+    // FP Exception Behavior is also set to strict
+    assert(FPExceptionBehavior.equals("strict"));
+    CmdArgs.push_back("-ftrapping-math");
----------------
Running `clang -### -ftrapping-math -ffp-exception-behavior=ignore` lead to this assertion to fail. As far as I can see `TrappingMath` is not changed in the case FPExceptionBehavior is "ignore" or "maytrap".
Clearly in the "ignore" case it should be safe to just set `TrappingMath` to false, but I'm not sure about the "maytrap" case.
It seems that `-ffp-exception-behavior` is more general than `-f{,no-}trapping-math`, so it seems natural to me to see `ftrapping-math` and `foo-trapping-math` as aliases for `ffp-exception-behavior=strict` and `ffp-exception-behavior=ignore` respectively. If we agree on this, then I would expect the reasoning inside the compiler only in terms of `FPExceptionBehavior`.


================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2576
+    CmdArgs.push_back("-ftrapping-math");
+  } else if (TrappingMathPresent)
     CmdArgs.push_back("-fno-trapping-math");
----------------
With this change if I run `clang -### -ffast-math test.c` I don't see `-fno-trapping-math` passed to the CC1. 
This is changing the changes the value of the function level attribute "no-trapping-math" (see lib/CodeGen/CGCall.cpp : 1747).
Is this an intended change?

Moreover since with this patch the default value for trapping math changed, the "no-trapping-math" function level attribute is incorrect also for default case.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62731/new/

https://reviews.llvm.org/D62731