[NFC][Driver] Clean up RenderFloatingPointOptions()

[andykaylor]

This change refactors RenderFloatingPointOptions() to eliminate some
excessively complicated logic and a redundant switch statement. The
logic being simplified is an artifact of the original -ffp-model
implementation, and over time it has become unnecessary.

The handling of diagnostics related to the -ffp-contract option is
still a bit convoluted after this change. I will address that in a
subsequent patch because I think it will make sense to make some minor
changes to the driver behavior when that is cleaned up. The current
patch should not make any change to observable behavior of the driver.

[andykaylor]

Near as I can tell, the only thing PreciseFPModel was still being used for was to allow us to detect that we had gotten here as a result of the OPT_ffp_model_EQ handler above changing the value of optID and falling through to the second switch (optID) statement, and the only effect it had was to keep us from setting LastSeenFfpContractOption. Since I am eliminating the fallthrough and the redundant switch statement, this value is no longer needed.

[andykaylor]

Every place in the code that sets RoundingFPMath to true also sets RoundingMathPresent to true, so this variable was entirely redundant.

[github-actions]

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff 7ec698e6edf5add1f0b49b42fba707bea4b80225 615bdd2a310f75efaa670ac4fe3677ae17e110e8 -- clang/lib/Driver/ToolChains/Clang.cpp

View the diff from clang-format here.

diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 0a2ea96de7..657b55a6be 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -2800,7 +2800,8 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
   for (const Arg *A : Args) {
     switch (A->getOption().getID()) {
     // If this isn't an FP option skip the claim below
-    default: continue;
+    default:
+      continue;
 
     case options::OPT_fcx_limited_range:
       if (GccRangeComplexOption.empty()) {

[zahiraam]

LGTM. Thanks.

[zahiraam]

nit. no need for this line.

[zahiraam]

in the "way" we intend?

[llvmbot]

@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Andy Kaylor (andykaylor)

Changes

This change refactors RenderFloatingPointOptions() to eliminate some
excessively complicated logic and a redundant switch statement. The
logic being simplified is an artifact of the original -ffp-model
implementation, and over time it has become unnecessary.

The handling of diagnostics related to the -ffp-contract option is
still a bit convoluted after this change. I will address that in a
subsequent patch because I think it will make sense to make some minor
changes to the driver behavior when that is cleaned up. The current
patch should not make any change to observable behavior of the driver.

---

Full diff: https://github.com/llvm/llvm-project/pull/91017.diff

1 Files Affected:

• (modified) clang/lib/Driver/ToolChains/Clang.cpp (+16-39)

diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index f9ca76a5ac8007..0a2ea96de73824 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -2736,7 +2736,6 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
   bool TrappingMathPresent = false; // Is trapping-math in args, and not
                                     // overriden by ffp-exception-behavior?
   bool RoundingFPMath = false;
-  bool RoundingMathPresent = false; // Is rounding-math in args?
   // -ffp-model values: strict, fast, precise
   StringRef FPModel = "";
   // -ffp-exception-behavior options: strict, maytrap, ignore
@@ -2799,11 +2798,10 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
   }
 
   for (const Arg *A : Args) {
-    auto optID = A->getOption().getID();
-    bool PreciseFPModel = false;
-    switch (optID) {
-    default:
-      break;
+    switch (A->getOption().getID()) {
+    // If this isn't an FP option skip the claim below
+    default: continue;
+
     case options::OPT_fcx_limited_range:
       if (GccRangeComplexOption.empty()) {
         if (Range != LangOptions::ComplexRangeKind::CX_Basic)
@@ -2895,7 +2893,6 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
       AssociativeMath = false;
       ReciprocalMath = false;
       SignedZeros = true;
-      // -fno_fast_math restores default denormal and fpcontract handling
       FPContract = "on";
 
       StringRef Val = A->getValue();
@@ -2906,10 +2903,6 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
         break;
       }
       StrictFPModel = false;
-      PreciseFPModel = true;
-      // ffp-model= is a Driver option, it is entirely rewritten into more
-      // granular options before being passed into cc1.
-      // Use the gcc option in the switch below.
       if (!FPModel.empty() && !FPModel.equals(Val))
         D.Diag(clang::diag::warn_drv_overriding_option)
             << Args.MakeArgString("-ffp-model=" + FPModel)
@@ -2918,27 +2911,20 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
         FPModel = Val;
         applyFastMath();
       } else if (Val.equals("precise")) {
-        optID = options::OPT_ffp_contract;
         FPModel = Val;
         FPContract = "on";
-        PreciseFPModel = true;
       } else if (Val.equals("strict")) {
         StrictFPModel = true;
-        optID = options::OPT_frounding_math;
         FPExceptionBehavior = "strict";
         FPModel = Val;
         FPContract = "off";
         TrappingMath = true;
+        RoundingFPMath = true;
       } else
         D.Diag(diag::err_drv_unsupported_option_argument)
             << A->getSpelling() << Val;
       break;
     }
-    }
-
-    switch (optID) {
-    // If this isn't an FP option skip the claim below
-    default: continue;
 
     // Options controlling individual features
     case options::OPT_fhonor_infinities:    HonorINFs = true;         break;
@@ -2982,12 +2968,10 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
 
     case options::OPT_frounding_math:
       RoundingFPMath = true;
-      RoundingMathPresent = true;
       break;
 
     case options::OPT_fno_rounding_math:
       RoundingFPMath = false;
-      RoundingMathPresent = false;
       break;
 
     case options::OPT_fdenormal_fp_math_EQ:
@@ -3010,13 +2994,8 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
     // Validate and pass through -ffp-contract option.
     case options::OPT_ffp_contract: {
       StringRef Val = A->getValue();
-      if (PreciseFPModel) {
-        // -ffp-model=precise enables ffp-contract=on.
-        // -ffp-model=precise sets PreciseFPModel to on and Val to
-        // "precise". FPContract is set.
-        ;
-      } else if (Val.equals("fast") || Val.equals("on") || Val.equals("off") ||
-                 Val.equals("fast-honor-pragmas")) {
+      if (Val.equals("fast") || Val.equals("on") || Val.equals("off") ||
+          Val.equals("fast-honor-pragmas")) {
         FPContract = Val;
         LastSeenFfpContractOption = Val;
       } else
@@ -3025,13 +3004,6 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
       break;
     }
 
-    // Validate and pass through -ffp-model option.
-    case options::OPT_ffp_model_EQ:
-      // This should only occur in the error case
-      // since the optID has been replaced by a more granular
-      // floating point option.
-      break;
-
     // Validate and pass through -ffp-exception-behavior option.
     case options::OPT_ffp_exception_behavior_EQ: {
       StringRef Val = A->getValue();
@@ -3152,6 +3124,12 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
       }
       break;
     }
+    // The StrictFPModel local variable is needed to report warnings
+    // in the way we intend. If -ffp-model=strict has been used, we
+    // want to report a warning for the next option encountered that
+    // takes us out of the settings described by fp-model=strict, but
+    // we don't want to continue issuing warnings for other conflicting
+    // options after that.
     if (StrictFPModel) {
       // If -ffp-model=strict has been specified on command line but
       // subsequent options conflict then emit warning diagnostic.
@@ -3225,11 +3203,10 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
   if (!FPContract.empty())
     CmdArgs.push_back(Args.MakeArgString("-ffp-contract=" + FPContract));
 
-  if (!RoundingFPMath)
-    CmdArgs.push_back(Args.MakeArgString("-fno-rounding-math"));
-
-  if (RoundingFPMath && RoundingMathPresent)
+  if (RoundingFPMath)
     CmdArgs.push_back(Args.MakeArgString("-frounding-math"));
+  else
+    CmdArgs.push_back(Args.MakeArgString("-fno-rounding-math"));
 
   if (!FPExceptionBehavior.empty())
     CmdArgs.push_back(Args.MakeArgString("-ffp-exception-behavior=" +