[clang] [NFC] Add assertion to ensure FiniteMathOnly is in sync with HonorINFs and HonorNANs. (PR #97342)
Aaron Ballman via cfe-commits
cfe-commits at lists.llvm.org
Wed Jul 3 04:38:25 PDT 2024
================
@@ -816,6 +816,11 @@ class FPOptions {
setAllowFPReassociate(LO.AllowFPReassoc);
setNoHonorNaNs(LO.NoHonorNaNs);
setNoHonorInfs(LO.NoHonorInfs);
+ // Ensure that if FiniteMathOnly is enabled, NoHonorNaNs and NoHonorInfs are
+ // also enabled. This is because FiniteMathOnly mode assumes no NaNs or Infs
+ // are present in computations.
+ if (!LO.NoHonorInfs || !LO.NoHonorInfs)
+ assert(!LO.FiniteMathOnly && "FiniteMathOnly implies NoHonorInfs");
----------------
AaronBallman wrote:
> There is quite a bit of LIT tests that have that "logic bug" then. They are using -Xclang -menable-no-infs -Xclang -menable-no-nan without the -ffinite-math-only option.
I didn't mean the logic bug was on the user end; I meant that our internal state has the logic bug because `!FiniteMathOnly` implies that infinities are supported and so it logically conflicts with `!HonorInfs`.
> Getting rid of FiniteMathOnly is fine by me.
FiniteMathOnly = !HonorINFs && !HonorNaNs which would mean -menable-no-infs -menable-non-nans. I would think using one or the other of these options could still mean that input values may be either INF or NAN or we can produce them, in which case we wouldn't be in finite math mode.
I can see the logic there and I think that's defensible, but because `FiniteMathOnly` determines the value `__FINITE_MATH_ONLY__` expands to, we should probably see how that flag is being used in the wild to see whether folks expect it to be `1` or `0` when infinity is enabled but NAN is disabled (for example).
https://github.com/llvm/llvm-project/pull/97342
More information about the cfe-commits
mailing list