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