[clang] [NFC] Add assertion to ensure FiniteMathOnly is in sync with HonorINFs and HonorNANs. (PR #97342)

Zahira Ammarguellat via cfe-commits cfe-commits at lists.llvm.org
Tue Jul 9 12:18:14 PDT 2024


https://github.com/zahiraam updated https://github.com/llvm/llvm-project/pull/97342

>From aea6519809340024226d587303e26c800c1a3756 Mon Sep 17 00:00:00 2001
From: Zahira Ammarguellat <zahira.ammarguellat at intel.com>
Date: Mon, 1 Jul 2024 12:56:07 -0700
Subject: [PATCH 1/3] [NFC] Add assertion to ensure that FiniteMathOnly toggles
 with the values of HonorINFs and HonorNANs.

---
 clang/include/clang/Basic/LangOptions.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/clang/include/clang/Basic/LangOptions.h b/clang/include/clang/Basic/LangOptions.h
index 91f1c2f2e6239..e866902743d2d 100644
--- a/clang/include/clang/Basic/LangOptions.h
+++ b/clang/include/clang/Basic/LangOptions.h
@@ -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");
     setNoSignedZero(LO.NoSignedZero);
     setAllowReciprocal(LO.AllowRecip);
     setAllowApproxFunc(LO.ApproxFunc);

>From 3a6cc060a0b91e2d960ea92a1d307a0bae3893f4 Mon Sep 17 00:00:00 2001
From: Zahira Ammarguellat <zahira.ammarguellat at intel.com>
Date: Mon, 8 Jul 2024 13:42:57 -0700
Subject: [PATCH 2/3] Fixed assertion as recommeded by reviwer and fix LIT
 tests.

---
 clang/include/clang/Basic/LangOptions.h            |  4 ++--
 clang/lib/Driver/ToolChains/Clang.cpp              | 12 +++++++++++-
 clang/test/CodeGen/fp-floatcontrol-stack.cpp       |  2 +-
 clang/test/Sema/warn-infinity-nan-disabled-lnx.cpp |  9 +++++----
 clang/test/Sema/warn-infinity-nan-disabled-win.cpp |  9 +++++----
 5 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/clang/include/clang/Basic/LangOptions.h b/clang/include/clang/Basic/LangOptions.h
index e866902743d2d..e1adcb3a95f18 100644
--- a/clang/include/clang/Basic/LangOptions.h
+++ b/clang/include/clang/Basic/LangOptions.h
@@ -819,8 +819,8 @@ class FPOptions {
     // 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");
+    assert((LO.FiniteMathOnly == (LO.NoHonorInfs && LO.NoHonorNaNs)) &&
+           "inf/nan inconsistent internal state");
     setNoSignedZero(LO.NoSignedZero);
     setAllowReciprocal(LO.AllowRecip);
     setAllowApproxFunc(LO.ApproxFunc);
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index aa285c39f14b4..1b799fb31bb7f 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -3295,7 +3295,17 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
   }
 
   // Handle __FINITE_MATH_ONLY__ similarly.
-  if (!HonorINFs && !HonorNaNs)
+  bool InfValues, NanValues = true;
+  auto processArg = [&](const auto *Arg) {
+    if (StringRef(Arg->getValue()) == "-menable-no-nans")
+      NanValues = false;
+    if (StringRef(Arg->getValue()) == "-menable-no-infs")
+      InfValues = false;
+  };
+  for (auto *Arg : Args.filtered(options::OPT_Xclang)) {
+    processArg(Arg);
+  }
+  if ((!HonorINFs && !HonorNaNs)  || (!NanValues && !InfValues))
     CmdArgs.push_back("-ffinite-math-only");
 
   if (const Arg *A = Args.getLastArg(options::OPT_mfpmath_EQ)) {
diff --git a/clang/test/CodeGen/fp-floatcontrol-stack.cpp b/clang/test/CodeGen/fp-floatcontrol-stack.cpp
index 090da25d21207..237c9d4f9a37e 100644
--- a/clang/test/CodeGen/fp-floatcontrol-stack.cpp
+++ b/clang/test/CodeGen/fp-floatcontrol-stack.cpp
@@ -1,7 +1,7 @@
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -ffp-contract=on -DDEFAULT=1 -emit-llvm -o - %s | FileCheck --check-prefix=CHECK-DDEFAULT %s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -ffp-contract=on -DEBSTRICT=1 -ffp-exception-behavior=strict -emit-llvm -o - %s | FileCheck --check-prefix=CHECK-DEBSTRICT %s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -DFAST=1 -ffast-math -ffp-contract=fast -emit-llvm -o - %s | FileCheck --check-prefix=CHECK-FAST %s
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -ffp-contract=on -DNOHONOR=1 -menable-no-infs -menable-no-nans -emit-llvm -o - %s | FileCheck --check-prefix=CHECK-NOHONOR %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -ffp-contract=on -DNOHONOR=1 -ffinite-math-only -menable-no-infs -menable-no-nans -emit-llvm -o - %s | FileCheck --check-prefix=CHECK-NOHONOR %s
 
 #define FUN(n) \
   (float z) { return n * z + n; }
diff --git a/clang/test/Sema/warn-infinity-nan-disabled-lnx.cpp b/clang/test/Sema/warn-infinity-nan-disabled-lnx.cpp
index 03a432e05851d..a4ba1fd0c97d3 100644
--- a/clang/test/Sema/warn-infinity-nan-disabled-lnx.cpp
+++ b/clang/test/Sema/warn-infinity-nan-disabled-lnx.cpp
@@ -1,10 +1,11 @@
 // RUN: %clang_cc1 -x c++ -verify=no-inf-no-nan \
-// RUN: -triple powerpc64le-unknown-unknown %s -menable-no-infs \
-// RUN: -menable-no-nans -std=c++23
+// RUN: -triple powerpc64le-unknown-unknown %s -ffinite-math-only \
+// RUN: -menable-no-infs -menable-no-nans -std=c++23
 
 // RUN: %clang_cc1 -x c++ -verify=no-inf-no-nan \
-// RUN: -triple powerpc64le-unknown-unknown %s -menable-no-infs \
-// RUN: -menable-no-nans -funsafe-math-optimizations -std=c++23
+// RUN: -triple powerpc64le-unknown-unknown %s -ffinite-math-only \
+// RUN: -menable-no-infs -menable-no-nans -funsafe-math-optimizations \
+// RUN: -std=c++23
 
 // RUN: %clang_cc1 -x c++ -verify=no-fast -triple powerpc64le-unknown-unknown \
 // RUN: %s -std=c++23
diff --git a/clang/test/Sema/warn-infinity-nan-disabled-win.cpp b/clang/test/Sema/warn-infinity-nan-disabled-win.cpp
index 51f9d325619ba..9248ee4c2e72a 100644
--- a/clang/test/Sema/warn-infinity-nan-disabled-win.cpp
+++ b/clang/test/Sema/warn-infinity-nan-disabled-win.cpp
@@ -2,12 +2,13 @@
 // on Windows the NAN macro is defined using INFINITY. See below.
 
 // RUN: %clang_cc1 -x c++ -verify=no-inf-no-nan \
-// RUN: -triple powerpc64le-unknown-unknown %s -menable-no-infs \
-// RUN: -menable-no-nans -std=c++23
+// RUN: -triple powerpc64le-unknown-unknown %s -ffinite-math-only \
+// RUN: -menable-no-infs -menable-no-nans -std=c++23
 
 // RUN: %clang_cc1 -x c++ -verify=no-inf-no-nan \
-// RUN: -triple powerpc64le-unknown-unknown %s -menable-no-infs \
-// RUN: -menable-no-nans -funsafe-math-optimizations -std=c++23
+// RUN: -triple powerpc64le-unknown-unknown %s -ffinite-math-only \
+// RUN: -menable-no-infs -menable-no-nans -funsafe-math-optimizations \
+// RUN: -std=c++23
 
 // RUN: %clang_cc1 -x c++ -verify=no-fast -triple powerpc64le-unknown-unknown \
 // RUN: %s -std=c++23

>From 1363e32f3d119f1e23911704325126edc6ddddaf Mon Sep 17 00:00:00 2001
From: Zahira Ammarguellat <zahira.ammarguellat at intel.com>
Date: Tue, 9 Jul 2024 12:17:16 -0700
Subject: [PATCH 3/3] Removed FiniteMathOnly and fixed LIT tests.

---
 clang/lib/Driver/ToolChains/Clang.cpp              | 2 +-
 clang/lib/Frontend/InitPreprocessor.cpp            | 2 +-
 clang/test/Sema/warn-infinity-nan-disabled-lnx.cpp | 8 ++++----
 clang/test/Sema/warn-infinity-nan-disabled-win.cpp | 7 +++----
 4 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 1b799fb31bb7f..65257e3c4c9d9 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -3305,7 +3305,7 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
   for (auto *Arg : Args.filtered(options::OPT_Xclang)) {
     processArg(Arg);
   }
-  if ((!HonorINFs && !HonorNaNs)  || (!NanValues && !InfValues))
+  if ((!HonorINFs && !HonorNaNs) || (!NanValues && !InfValues))
     CmdArgs.push_back("-ffinite-math-only");
 
   if (const Arg *A = Args.getLastArg(options::OPT_mfpmath_EQ)) {
diff --git a/clang/lib/Frontend/InitPreprocessor.cpp b/clang/lib/Frontend/InitPreprocessor.cpp
index 5e52555c6fee9..e7e02e9ddf96e 100644
--- a/clang/lib/Frontend/InitPreprocessor.cpp
+++ b/clang/lib/Frontend/InitPreprocessor.cpp
@@ -1311,7 +1311,7 @@ static void InitializePredefinedMacros(const TargetInfo &TI,
   if (!LangOpts.MathErrno)
     Builder.defineMacro("__NO_MATH_ERRNO__");
 
-  if (LangOpts.FastMath || LangOpts.FiniteMathOnly)
+  if (LangOpts.FastMath || (LangOpts.NoHonorInfs && LangOpts.NoHonorNaNs))
     Builder.defineMacro("__FINITE_MATH_ONLY__", "1");
   else
     Builder.defineMacro("__FINITE_MATH_ONLY__", "0");
diff --git a/clang/test/Sema/warn-infinity-nan-disabled-lnx.cpp b/clang/test/Sema/warn-infinity-nan-disabled-lnx.cpp
index a4ba1fd0c97d3..cb719c3a519da 100644
--- a/clang/test/Sema/warn-infinity-nan-disabled-lnx.cpp
+++ b/clang/test/Sema/warn-infinity-nan-disabled-lnx.cpp
@@ -1,10 +1,10 @@
 // RUN: %clang_cc1 -x c++ -verify=no-inf-no-nan \
-// RUN: -triple powerpc64le-unknown-unknown %s -ffinite-math-only \
-// RUN: -menable-no-infs -menable-no-nans -std=c++23
+// RUN: -triple powerpc64le-unknown-unknown %s \
+// RUN: -ffinite-math-only -std=c++23
 
 // RUN: %clang_cc1 -x c++ -verify=no-inf-no-nan \
-// RUN: -triple powerpc64le-unknown-unknown %s -ffinite-math-only \
-// RUN: -menable-no-infs -menable-no-nans -funsafe-math-optimizations \
+// RUN: -triple powerpc64le-unknown-unknown %s \
+// RUN: -ffinite-math-only -funsafe-math-optimizations \
 // RUN: -std=c++23
 
 // RUN: %clang_cc1 -x c++ -verify=no-fast -triple powerpc64le-unknown-unknown \
diff --git a/clang/test/Sema/warn-infinity-nan-disabled-win.cpp b/clang/test/Sema/warn-infinity-nan-disabled-win.cpp
index 9248ee4c2e72a..4bf3355926634 100644
--- a/clang/test/Sema/warn-infinity-nan-disabled-win.cpp
+++ b/clang/test/Sema/warn-infinity-nan-disabled-win.cpp
@@ -2,12 +2,11 @@
 // on Windows the NAN macro is defined using INFINITY. See below.
 
 // RUN: %clang_cc1 -x c++ -verify=no-inf-no-nan \
-// RUN: -triple powerpc64le-unknown-unknown %s -ffinite-math-only \
-// RUN: -menable-no-infs -menable-no-nans -std=c++23
+// RUN: -triple powerpc64le-unknown-unknown %s -ffinite-math-only -std=c++23
 
 // RUN: %clang_cc1 -x c++ -verify=no-inf-no-nan \
-// RUN: -triple powerpc64le-unknown-unknown %s -ffinite-math-only \
-// RUN: -menable-no-infs -menable-no-nans -funsafe-math-optimizations \
+// RUN: -triple powerpc64le-unknown-unknown %s \
+// RUN: -ffinite-math-only -funsafe-math-optimizations \
 // RUN: -std=c++23
 
 // RUN: %clang_cc1 -x c++ -verify=no-fast -triple powerpc64le-unknown-unknown \



More information about the cfe-commits mailing list