Revert "Set FLT_EVAL_METHOD to -1 when fast-math is enabled."

[zahiraam]

Summary

Setting FLT_EVAL_METHOD to -1 with fast-math will set __GLIBC_FLT_EVAL_METHOD to 2 and long double ends up being used for
float_t and double_t. This creates some ABI breakage with various C libraries.

See details here: https://github.com/llvm/llvm-project/issues/60781

This reverts commit bbf0d1932a3c1be970ed8a580e51edf571b80fd5.

[zahiraam]

Tagging @davemgreen, @kito-cheng, @bjope and @Wilco1 for awareness.

[aaron.ballman]

Thank you for working on this! Because this is fixing an ABI break, if we can land this in time for the release candidate I think we should try to add this to 16.0 to limit damage. CC @tstellar for his opinion.

This doesn't seem to fully revert bbf0d1932a3c1be970ed8a580e51edf571b80fd5; that commit changed clang/lib/Sema/Sema.cpp but this commit does not back those changes out. Is that intentional?

clang/test/Preprocessor/flt_eval_macro.cpp

66–67

This line is no longer needed because it will never be checked.

[zahiraam]

In D145704#4182268, @aaron.ballman wrote:

Thank you for working on this! Because this is fixing an ABI break, if we can land this in time for the release candidate I think we should try to add this to 16.0 to limit damage. CC @tstellar for his opinion.

This doesn't seem to fully revert bbf0d1932a3c1be970ed8a580e51edf571b80fd5; that commit changed clang/lib/Sema/Sema.cpp but this commit does not back those changes out. Is that intentional?

Yes that makes sense because the changes in Sema were moved to lib/lex/Preprocessor.cpp in this patch: https://reviews.llvm.org/D128814

[aaron.ballman]

In D145704#4182341, @zahiraam wrote:

In D145704#4182268, @aaron.ballman wrote:

Thank you for working on this! Because this is fixing an ABI break, if we can land this in time for the release candidate I think we should try to add this to 16.0 to limit damage. CC @tstellar for his opinion.

This doesn't seem to fully revert bbf0d1932a3c1be970ed8a580e51edf571b80fd5; that commit changed clang/lib/Sema/Sema.cpp but this commit does not back those changes out. Is that intentional?

Yes that makes sense because the changes in Sema were moved to lib/lex/Preprocessor.cpp in this patch: https://reviews.llvm.org/D128814

Ah, thank you for the explanation, that makes sense.

This variant comes with a .patch file added to the PR; that should be deleted in the next round. Aside from that, the changes LGTM, but I'd like to give precommit CI a chance to complete and some time for the folks impacted by the changes to weigh in before accepting.

[aaron.ballman]

In D145704#4182364, @aaron.ballman wrote:

This variant comes with a .patch file added to the PR; that should be deleted in the next round. Aside from that, the changes LGTM, but I'd like to give precommit CI a chance to complete and some time for the folks impacted by the changes to weigh in before accepting.

Heh you caught the .patch file and already fixed it, thanks!

[tstellar]

We're holding -rc4 until this is merged, so it would be great if we could merge it on Friday.

[aaron.ballman]

LGTM! If you don't hear otherwise from @kito-cheng, @bjope, @Wilco1 in the next ~4 hours, feel free to land.

[zahiraam]

In D145704#4184584, @aaron.ballman wrote:

LGTM! If you don't hear otherwise from @kito-cheng, @bjope, @Wilco1 in the next ~4 hours, feel free to land.

Great! Thanks @aaron.ballman and @tstellar.

[bjope]

I have no objections about doing this revert.

clang/include/clang/Lex/Preprocessor.h

class Preprocessor {
   SourceLocation DATELoc, TIMELoc;
 
   // FEM_UnsetOnCommandLine means that an explicit evaluation method was
   // not specified on the command line. The target is queried to set the
   // default evaluation method.
   LangOptions::FPEvalMethodKind CurrentFPEvalMethod =
       LangOptions::FPEvalMethodKind::FEM_UnsetOnCommandLine;
 
-  // Keeps the value of the last evaluation method before a
-  // `pragma float_control (precise,off) is applied.
-  LangOptions::FPEvalMethodKind LastFPEvalMethod =
-      LangOptions::FPEvalMethodKind::FEM_UnsetOnCommandLine;
-
   // The most recent pragma location where the floating point evaluation
   // method was modified. This is used to determine whether the
   // 'pragma clang fp eval_method' was used whithin the current scope.
   SourceLocation LastFPEvalPragmaLocation;
 
   LangOptions::FPEvalMethodKind TUFPEvalMethod =
       LangOptions::FPEvalMethodKind::FEM_UnsetOnCommandLine;
 
public:
   LangOptions::FPEvalMethodKind getTUFPEvalMethod() const {
     return TUFPEvalMethod;
   }
 
   SourceLocation getLastFPEvalPragmaLocation() const {
     return LastFPEvalPragmaLocation;
   }
 
-  LangOptions::FPEvalMethodKind getLastFPEvalMethod() const {
-    return LastFPEvalMethod;
-  }
-
-  void setLastFPEvalMethod(LangOptions::FPEvalMethodKind Val) {
-    LastFPEvalMethod = Val;
-  }
-
   void setCurrentFPEvalMethod(SourceLocation PragmaLoc,
                               LangOptions::FPEvalMethodKind Val) {
     assert(Val != LangOptions::FEM_UnsetOnCommandLine &&
            "FPEvalMethod should never be set to FEM_UnsetOnCommandLine");
     // This is the location of the '#pragma float_control" where the
     // execution state is modifed.
     LastFPEvalPragmaLocation = PragmaLoc;
     CurrentFPEvalMethod = Val;

clang/lib/Lex/PPMacroExpansion.cpp

if (getPreprocessorOpts().SourceDateEpoch) {
         Result = "??? ??? ?? ??:??:?? ????\n";
       }
     }
     // Surround the string with " and strip the trailing newline.
     OS << '"' << StringRef(Result).drop_back() << '"';
     Tok.setKind(tok::string_literal);
   } else if (II == Ident__FLT_EVAL_METHOD__) {
     // __FLT_EVAL_METHOD__ is set to the default value.
-    if (getTUFPEvalMethod() ==
-        LangOptions::FPEvalMethodKind::FEM_Indeterminable) {
-      // This is possible if `AllowFPReassoc` or `AllowReciprocal` is enabled.
-      // These modes can be triggered via the command line option `-ffast-math`
-      // or via a `pragam float_control`.
-      // __FLT_EVAL_METHOD__ expands to -1.
-      // The `minus` operator is the next token we read from the stream.
-      auto Toks = std::make_unique<Token[]>(1);
-      OS << "-";
-      Tok.setKind(tok::minus);
-      // Push the token `1` to the stream.
-      Token NumberToken;
-      NumberToken.startToken();
-      NumberToken.setKind(tok::numeric_constant);
-      NumberToken.setLiteralData("1");
-      NumberToken.setLength(1);
-      Toks[0] = NumberToken;
-      EnterTokenStream(std::move(Toks), 1, /*DisableMacroExpansion*/ false,
-                       /*IsReinject*/ false);
-    } else {
-      OS << getTUFPEvalMethod();
-      // __FLT_EVAL_METHOD__ expands to a simple numeric value.
-      Tok.setKind(tok::numeric_constant);
-      if (getLastFPEvalPragmaLocation().isValid()) {
-        // The program is ill-formed. The value of __FLT_EVAL_METHOD__ is
-        // altered by the pragma.
-        Diag(Tok, diag::err_illegal_use_of_flt_eval_macro);
-        Diag(getLastFPEvalPragmaLocation(), diag::note_pragma_entered_here);
-      }
-    }
+      // The program is ill-formed. The value of __FLT_EVAL_METHOD__ is altered
+      // by the pragma.
   } else if (II == Ident__COUNTER__) {
     // __COUNTER__ expands to a simple numeric value.
     OS << CounterValue++;
     Tok.setKind(tok::numeric_constant);
   } else if (II == Ident__has_feature) {
     EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this, false,
       [this](Token &Tok, bool &HasLexedNextToken) -> int {
         IdentifierInfo *II = ExpectFeatureIdentifierInfo(Tok, *this,

clang/lib/Lex/Preprocessor.cpp

void Preprocessor::Initialize(const TargetInfo &Target,
   setTUFPEvalMethod(getTargetInfo().getFPEvalMethod());
 
   if (getLangOpts().getFPEvalMethod() == LangOptions::FEM_UnsetOnCommandLine)
     // Use setting from TargetInfo.
     setCurrentFPEvalMethod(SourceLocation(), Target.getFPEvalMethod());
   else
     // Set initial value of __FLT_EVAL_METHOD__ from the command line.
     setCurrentFPEvalMethod(SourceLocation(), getLangOpts().getFPEvalMethod());
-  // When `-ffast-math` option is enabled, it triggers several driver math
-  // options to be enabled. Among those, only one the following two modes
-  // affect the eval-method:  reciprocal or reassociate.
-  if (getLangOpts().AllowFPReassoc || getLangOpts().AllowRecip)
-    setCurrentFPEvalMethod(SourceLocation(), LangOptions::FEM_Indeterminable);
 }
 
 void Preprocessor::InitializeForModelFile() {
   NumEnteredSourceFiles = 0;
 
   // Reset pragmas
   PragmaHandlersBackup = std::move(PragmaHandlers);
   PragmaHandlers = std::make_unique<PragmaNamespace>(StringRef());

clang/lib/Sema/SemaAttr.cpp

if ((Action == PSK_Push_Set || Action == PSK_Push || Action == PSK_Pop) &&
     return;
   }
   switch (Value) {
   default:
     llvm_unreachable("invalid pragma float_control kind");
   case PFC_Precise:
     NewFPFeatures.setFPPreciseEnabled(true);
     FpPragmaStack.Act(Loc, Action, StringRef(), NewFPFeatures);
-    if (PP.getCurrentFPEvalMethod() ==
-            LangOptions::FPEvalMethodKind::FEM_Indeterminable &&
-        PP.getLastFPEvalPragmaLocation().isValid())
-      // A preceding `pragma float_control(precise,off)` has changed
-      // the value of the evaluation method.
-      // Set it back to its old value.
-      PP.setCurrentFPEvalMethod(SourceLocation(), PP.getLastFPEvalMethod());
     break;
   case PFC_NoPrecise:
     if (CurFPFeatures.getExceptionMode() == LangOptions::FPE_Strict)
       Diag(Loc, diag::err_pragma_fc_noprecise_requires_noexcept);
     else if (CurFPFeatures.getAllowFEnvAccess())
       Diag(Loc, diag::err_pragma_fc_noprecise_requires_nofenv);
     else
       NewFPFeatures.setFPPreciseEnabled(false);
     FpPragmaStack.Act(Loc, Action, StringRef(), NewFPFeatures);
-    PP.setLastFPEvalMethod(PP.getCurrentFPEvalMethod());
-    // `AllowFPReassoc` or `AllowReciprocal` option is enabled.
-    PP.setCurrentFPEvalMethod(
-        Loc, LangOptions::FPEvalMethodKind::FEM_Indeterminable);
     break;
   case PFC_Except:
     if (!isPreciseFPEnabled())
       Diag(Loc, diag::err_pragma_fc_except_requires_precise);
     else
       NewFPFeatures.setSpecifiedExceptionModeOverride(LangOptions::FPE_Strict);
     FpPragmaStack.Act(Loc, Action, StringRef(), NewFPFeatures);
     break;
   case PFC_NoExcept:
     NewFPFeatures.setSpecifiedExceptionModeOverride(LangOptions::FPE_Ignore);
     FpPragmaStack.Act(Loc, Action, StringRef(), NewFPFeatures);
     break;
   case PFC_Push:
     FpPragmaStack.Act(Loc, Sema::PSK_Push_Set, StringRef(), NewFPFeatures);
     break;
   case PFC_Pop:
     if (FpPragmaStack.Stack.empty()) {
       Diag(Loc, diag::warn_pragma_pop_failed) << "float_control"
                                               << "stack empty";
       return;
     }
     FpPragmaStack.Act(Loc, Action, StringRef(), NewFPFeatures);
     NewFPFeatures = FpPragmaStack.CurrentValue;
-    if (CurFPFeatures.getAllowFPReassociate() ||
-        CurFPFeatures.getAllowReciprocal())
-      // Since we are popping the pragma, we don't want to be passing
-      // a location here.
-      PP.setCurrentFPEvalMethod(SourceLocation(),
-                                CurFPFeatures.getFPEvalMethod());
     break;
   }
   CurFPFeatures = NewFPFeatures.applyOverrides(getLangOpts());
 }
 
 void Sema::ActOnPragmaMSPointersToMembers(
     LangOptions::PragmaMSPointersToMembersKind RepresentationMethod,
     SourceLocation PragmaLoc) {

clang/test/CodeGen/X86/fexcess-precision.c

 // CHECK-CONTRACT-DBL-NEXT:    ret i32 1
 //
 // CHECK-CONTRACT-EXT-LABEL: @getFEM(
 // CHECK-CONTRACT-EXT-NEXT:  entry:
 // CHECK-CONTRACT-EXT-NEXT:    ret i32 2
 //
 // CHECK-UNSAFE-LABEL: @getFEM(
 // CHECK-UNSAFE-NEXT:  entry:
-// CHECK-UNSAFE-NEXT:    ret i32 -1
+// CHECK-UNSAFE-NEXT:    ret i32 0
 //
 int getFEM() {
   return __FLT_EVAL_METHOD__;
 }

clang/test/CodeGen/eval-method-fast-math.cpp

-// RUN: %clang_cc1 -fexperimental-strict-floating-point  \
-// RUN: -triple x86_64-linux-gnu -emit-llvm -o - %s  \
-// RUN: | FileCheck %s -check-prefixes=CHECK
-
-// RUN: %clang_cc1 -triple i386--linux -emit-llvm -o - %s \
-// RUN: | FileCheck %s -check-prefixes=CHECK-EXT
-
-// RUN: %clang_cc1 -fexperimental-strict-floating-point  \
-// RUN: -mreassociate -freciprocal-math -ffp-contract=fast \
-// RUN: -ffast-math -triple x86_64-linux-gnu \
-// RUN: -emit-llvm -o - %s \
-// RUN: | FileCheck %s -check-prefixes=CHECK-FAST
-
-// RUN: %clang_cc1 -triple i386--linux -mreassociate -freciprocal-math \
-// RUN: -ffp-contract=fast -ffast-math -emit-llvm -o - %s \
-// RUN: | FileCheck %s -check-prefixes=CHECK-FAST
-
-float a = 1.0f, b = 2.0f, c = 3.0f;
-#pragma float_control(precise, off)
-float res2 = a + b + c;
-int val3 = __FLT_EVAL_METHOD__;
-#pragma float_control(precise, on)
-float res3 = a + b + c;
-int val4 = __FLT_EVAL_METHOD__;
-
-// CHECK: @val3 = global i32 -1
-// CHECK: @val4 = global i32 0
-
-// CHECK-EXT: @val3 = global i32 -1
-// CHECK-EXT: @val4 = global i32 2
-
-// CHECK-FAST: @val3 = global i32 -1
-// CHECK-FAST: @val4 = global i32 -1
-
-float res;
-int add(float a, float b, float c) {
-  // CHECK: fadd float
-  // CHECK: load float, ptr
-  // CHECK: fadd float
-  // CHECK: store float
-  // CHECK: ret i32 0
-  res = a + b + c;
-  return __FLT_EVAL_METHOD__;
-}
-
-int add_precise(float a, float b, float c) {
-#pragma float_control(precise, on)
-  // CHECK: fadd float
-  // CHECK: load float, ptr
-  // CHECK: fadd float
-  // CHECK: store float
-  // CHECK: ret i32 0
-  res = a + b + c;
-  return __FLT_EVAL_METHOD__;
-}
-
-#pragma float_control(push)
-#pragma float_control(precise, on)
-int add_precise_1(float a, float b, float c) {
-  // CHECK: fadd float
-  // CHECK: load float, ptr
-  // CHECK: fadd float
-  // CHECK: store float
-  // CHECK: ret i32 0
-  res = a + b + c;
-  return __FLT_EVAL_METHOD__;
-}
-#pragma float_control(pop)
-
-int add_not_precise(float a, float b, float c) {
-  // Fast-math is enabled with this pragma.
-#pragma float_control(precise, off)
-  // CHECK: fadd fast float
-  // CHECK: load float, ptr
-  // CHECK: fadd fast float
-  // CHECK: float {{.*}}, ptr
-  // CHECK: ret i32 -1
-  res = a + b + c;
-  return __FLT_EVAL_METHOD__;
-}
-
-#pragma float_control(push)
-// Fast-math is enabled with this pragma.
-#pragma float_control(precise, off)
-int add_not_precise_1(float a, float b, float c) {
-  // CHECK: fadd fast float
-  // CHECK: load float, ptr
-  // CHECK: fadd fast float
-  // CHECK: float {{.*}}, ptr
-  // CHECK: ret i32 -1
-  res = a + b + c;
-  return __FLT_EVAL_METHOD__;
-}
-#pragma float_control(pop)
-
-int getFPEvalMethod() {
-  // CHECK: ret i32 0
-  return __FLT_EVAL_METHOD__;
-}
-
-float res1;
-int whatever(float a, float b, float c) {
-#pragma float_control(precise, off)
-  // CHECK: load float, ptr
-  // CHECK: fadd fast float
-  // CHECK: store float {{.*}}, ptr
-  // CHECK: store i32 -1
-  // CHECK: store i32 0
-  // CHECK: ret i32 -1
-  res1 = a + b + c;
-  int val1 = __FLT_EVAL_METHOD__;
-  {
-#pragma float_control(precise, on)
-    int val2 = __FLT_EVAL_METHOD__;
-  }
-  return __FLT_EVAL_METHOD__;
-}

clang/test/Preprocessor/flt_eval_macro.cpp

 
 // RUN: %clang_cc1 -E -dM -triple=arm64_32-apple-ios  %s -o - \
 // RUN:   | FileCheck %s  -strict-whitespace
 
 // RUN: %clang_cc1 -E -dM -triple=arm64_32-apple-ios -target-feature -sse \
 // RUN:   %s -o - | FileCheck %s  -strict-whitespace
 
 // RUN: %clang_cc1 -E -dM -triple=x86_64-apple-macos13.0 -ffast-math \
-// RUN:   %s -o - | FileCheck %s -check-prefix=CHECK-MINUS-ONE -strict-whitespace
+// RUN:   %s -o - | FileCheck %s -check-prefix=CHECK -strict-whitespace
 
 // RUN: %clang_cc1 -E -dM -triple i386-pc-windows -target-cpu pentium4 %s -o - \
 // RUN:   | FileCheck %s  -strict-whitespace
 
 // RUN: %clang_cc1 -E -dM -triple i386-pc-windows -target-cpu pentium4 \
 // RUN:   -target-feature -sse %s -o - | FileCheck -check-prefix=EXT %s \
 // RUN:   -strict-whitespace
 

 #define Name "Eight"
 #elif __GLIBC_FLT_EVAL_METHOD == 129
 #define Name "Nine"
 #else
 #error "Unknown __GLIBC_FLT_EVAL_METHOD"
 #endif
 
 int foo() {
   // CHECK: #define Name "One"
-  // CHECK-MINUS-ONE: #define Name "MinusOne"
   // EXT: #define Name "Three"
   return Name;
 }
 
 #pragma fp eval_method(double)
 
 #if __FLT_EVAL_METHOD__ == 3
 #define Val "Unset"
 #elif __FLT_EVAL_METHOD__ == 0