[NFC] Refactor fast-math handling for clang driver

[andykaylor]

This refactors the fast-math handling in the clang driver, moving the settings into a lambda that is shared by the -ffp-model=fast and -ffast-math code. Previously the -ffp-model=fast handler changed the local option variable and fell through to the -ffast-math handler.

This refactoring is intended to prepare the way for decoupling the -ffp-model=fast settings from the -ffast-math settings and possibly introduce a less aggressive fp-model.

[llvmbot]

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-driver

Author: Andy Kaylor (andykaylor)

Changes

This refactors the fast-math handling in the clang driver, moving the settings into a lambda that is shared by the -ffp-model=fast and -ffast-math code. Previously the -ffp-model=fast handler changed the local option variable and fell through to the -ffast-math handler.

This refactoring is intended to prepare the way for decoupling the -ffp-model=fast settings from the -ffast-math settings and possibly introduce a less aggressive fp-model.

---

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

1 Files Affected:

• (modified) clang/lib/Driver/ToolChains/Clang.cpp (+22-18)

diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 942ebbc4106078..4459d86e77d5d9 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -2778,6 +2778,26 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
   LangOptions::ComplexRangeKind Range = LangOptions::ComplexRangeKind::CX_None;
   std::string ComplexRangeStr = "";
 
+  // Lambda to set fast-math options. This is also used by -ffp-model=fast
+  auto applyFastMath = [&]() {
+    HonorINFs = false;
+    HonorNaNs = false;
+    MathErrno = false;
+    AssociativeMath = true;
+    ReciprocalMath = true;
+    ApproxFunc = true;
+    SignedZeros = false;
+    TrappingMath = false;
+    RoundingFPMath = false;
+    FPExceptionBehavior = "";
+    // If fast-math is set then set the fp-contract mode to fast.
+    FPContract = "fast";
+    // ffast-math enables limited range rules for complex multiplication and
+    // division.
+    Range = LangOptions::ComplexRangeKind::CX_Limited;
+    SeenUnsafeMathModeOption = true;
+  };
+
   if (const Arg *A = Args.getLastArg(options::OPT_flimited_precision_EQ)) {
     CmdArgs.push_back("-mlimit-float-precision");
     CmdArgs.push_back(A->getValue());
@@ -2842,9 +2862,8 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
             << Args.MakeArgString("-ffp-model=" + FPModel)
             << Args.MakeArgString("-ffp-model=" + Val);
       if (Val.equals("fast")) {
-        optID = options::OPT_ffast_math;
         FPModel = Val;
-        FPContract = "fast";
+        applyFastMath();
       } else if (Val.equals("precise")) {
         optID = options::OPT_ffp_contract;
         FPModel = Val;
@@ -3061,22 +3080,7 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
         continue;
       [[fallthrough]];
     case options::OPT_ffast_math: {
-      HonorINFs = false;
-      HonorNaNs = false;
-      MathErrno = false;
-      AssociativeMath = true;
-      ReciprocalMath = true;
-      ApproxFunc = true;
-      SignedZeros = false;
-      TrappingMath = false;
-      RoundingFPMath = false;
-      FPExceptionBehavior = "";
-      // If fast-math is set then set the fp-contract mode to fast.
-      FPContract = "fast";
-      SeenUnsafeMathModeOption = true;
-      // ffast-math enables fortran rules for complex multiplication and
-      // division.
-      Range = LangOptions::ComplexRangeKind::CX_Limited;
+      applyFastMath();
       break;
     }
     case options::OPT_fno_fast_math:

[MaskRay]

Should fallthrough still be used?

[andykaylor]

This is falling through from OPT_Ofast to OPT_ffast_math. I think we still want that to happen. It's not obvious from the diff, but the "fp-model" handler and the "ffast-math" handler are in different switch statements.

[AaronBallman]

Oh! Thank you for mentioning they're in different switch statements, I had missed that as well. :-D

[zahiraam]

LGTM.

[AaronBallman]

Oh! Thank you for mentioning they're in different switch statements, I had missed that as well. :-D

[AaronBallman]

This seems like it's not an NFC change, right? We used to fail to set a whole pile of flags and now we're setting them correctly?

[andykaylor]

I'm pretty sure it is NFC. We have tests that verify this (clang/test/Driver/fp-model.c and clang/test/Driver/fast-math.c). I'm only changing where the local variables are set. The FPContract value that was being set here was also being set in the OPT_ffast_math handler. Now both places call the lambda for everything they set.

[andykaylor]

I just re-read your comment, and I think I see the confusion now. The previous code was not easy to follow. We were changing the value of optID here, so when we finished with this switch statement execution would continue on to the switch statement below where the "whole pile of flags" was being set by the OPT_ffast_math handler. Now I'm not changing the value of optID here and instead calling the lambda to set the pile of flags. In a future revision I'd like to add a parameter to the lambda to indicate that I want slightly less aggressive fast math settings.

I started out with a change that chained all the OPT_ffast_math, OPT_fno_fast_math, OPT_funsafe_math_optimizations, and OPT_fno_unsafe_math_optimizations into a pair of nested lambdas with a parameter for positive and negative versions, but that got way too convoluted to handle all the variations needed to make it NFC. I think that pointed to some things we're doing wrong, but I'll address those separately to keep the history clean. This seemed like a manageable place to start.