Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang] Macro for constant rounding mode #92699

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

spavloff
Copy link
Collaborator

The forthcoming C standard defines pragma FENV_ROUND to support constant rounding mode. It also requires some functions to be evaluated with such mode, N3096 7.6.2p4 states:

Within the scope of an FENV_ROUND pragma establishing a mode other
than FE_DYNAMIC ... invocations of functions indicated in the table
below, for which macro replacement has not been suppressed (7.1.4),
shall be evaluated according to the specified constant rounding mode
... . Invocations of functions for which macro replacement has been
suppressed and invocations of functions other than those indicated
in the table below shall not be affected by constant rounding modes
– they are affected by (and affect) only the dynamic mode.

The way this requirement is formulated indicates that it could be implemented using preprocessor facility. Such implementation would require a builtin macro that is set in the region where pragma FENV_ROUND is in effect and reflects constant rounding mode.

This change introduces macro ROUNDING_MODE, which is a string dependent on the constant rounding mode:

FE_TOWARDZERO        "_rtz"
FE_TONEAREST         "_rte"
FE_DOWNWARD          "_rtp"
FE_UPWARD            "_rtn"
FE_TONEARESTFROMZERO "_rta"
FE_DYNAMIC           empty string

All these values except "_rta" are OpenCL rounding mode modifiers. Default value, when no pragma FENV_ROUND is specified, is empty string. Concatenation of a function name with the builtin macro can be used to obtain name of the function variant for particular rounding mode, like "sin_rtz", or "__builtin_cos_rtd". The test "macro_rounding_mode.c" added in this change provides an example of possible use.

The macro is implemented in the same way as FLT_EVAL_METHOD, which also depends on the results of semantic analysis.

@spavloff spavloff self-assigned this May 19, 2024
Copy link

github-actions bot commented May 19, 2024

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

You can test this locally with the following command:
git-clang-format --diff 643f36184bd3d9a95cbfd608af6f1cccc69e0187 5e9223bb9a689952879dfe1dd37823544d192d49 -- clang/test/Preprocessor/macro_rounding_mode.c clang/include/clang/Lex/Preprocessor.h clang/include/clang/Parse/Parser.h clang/lib/Frontend/PrintPreprocessedOutput.cpp clang/lib/Lex/PPMacroExpansion.cpp clang/lib/Lex/Pragma.cpp clang/lib/Lex/Preprocessor.cpp clang/lib/Parse/ParsePragma.cpp clang/test/Parser/pragma-fenv_round.c
View the diff from clang-format here.
diff --git a/clang/lib/Lex/PPMacroExpansion.cpp b/clang/lib/Lex/PPMacroExpansion.cpp
index 0380fc4f68..bc4abbcf73 100644
--- a/clang/lib/Lex/PPMacroExpansion.cpp
+++ b/clang/lib/Lex/PPMacroExpansion.cpp
@@ -1662,7 +1662,7 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) {
       RoundingModeIdent = "_rtz";
       break;
     case LangOptions::RoundingMode::NearestTiesToEven:
-     RoundingModeIdent = "_rte";
+      RoundingModeIdent = "_rte";
       break;
     case LangOptions::RoundingMode::TowardPositive:
       RoundingModeIdent = "_rtp";
@@ -1801,8 +1801,7 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) {
 
         return false;
       });
-  } else if (II == Ident__has_cpp_attribute ||
-             II == Ident__has_c_attribute) {
+  } else if (II == Ident__has_cpp_attribute || II == Ident__has_c_attribute) {
     bool IsCXX = II == Ident__has_cpp_attribute;
     EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this, true,
         [&](Token &Tok, bool &HasLexedNextToken) -> int {
@@ -1832,8 +1831,7 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) {
                                    getLangOpts())
                     : 0;
         });
-  } else if (II == Ident__has_include ||
-             II == Ident__has_include_next) {
+  } else if (II == Ident__has_include || II == Ident__has_include_next) {
     // The argument to these two builtins should be a parenthesized
     // file name string literal using angle brackets (<>) or
     // double-quotes ("").

The forthcoming C standard defines pragma FENV_ROUND to support constant
rounding mode. It also requires some functions to be evaluated with such
mode, N3096 7.6.2p4 states:

    Within the scope of an FENV_ROUND pragma establishing a mode other
    than FE_DYNAMIC ... invocations of functions indicated in the table
    below, for which macro replacement has not been suppressed (7.1.4),
    shall be evaluated according to the specified constant rounding mode
    ... . Invocations of functions for which macro replacement has been
    suppressed and invocations of functions other than those indicated
    in the table below shall not be affected by constant rounding modes
    – they are affected by (and affect) only the dynamic mode.

The way this requirement is formulated indicates that it could be
implemented using preprocessor facility. Such implementation would
require a builtin macro that is set in the region where pragma
FENV_ROUND is in effect and reflects constant rounding mode.

This change introduces macro __ROUNDING_MODE__, which is a string
dependent on the constant rounding mode:

    FE_TOWARDZERO        "_rtz"
    FE_TONEAREST         "_rte"
    FE_DOWNWARD          "_rtp"
    FE_UPWARD            "_rtn"
    FE_TONEARESTFROMZERO "_rta"
    FE_DYNAMIC           empty string

All these values except "_rta" are OpenCL rounding mode modifiers.
Default value, when no pragma FENV_ROUND is specified, is empty string.
Concatenation of a function name with the builtin macro can be used to
obtain name of the function variant for particular rounding mode, like
"sin_rtz", or "__builtin_cos_rtd". The test "macro_rounding_mode.c"
added in this change provides an example of possible use.

The macro is implemented in the same way as FLT_EVAL_METHOD, which also
depends on the results of semantic analysis.
@@ -0,0 +1,55 @@
// RUN: %clang_cc1 -emit-llvm -triple i386-linux -Wno-unknown-pragmas %s -o - | FileCheck %s
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is a preprocessor testcase, can you just use -E?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this macro is managed by the code in Sema, because pragma FENV_ROUND is processed there. If only -E is specified, ROUNDING_MODE is always an empty string. We handle FLT_EVAL_METHOD similarly.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there some reason the preprocessor can't parse FENV_ROUND? Breaking code with -save-temps etc. seems bad.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parsing pragma FENV_ROUND has been moved to the preprocessor.

@spavloff spavloff marked this pull request as ready for review May 20, 2024 08:48
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels May 20, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented May 20, 2024

@llvm/pr-subscribers-clang

Author: Serge Pavlov (spavloff)

Changes

The forthcoming C standard defines pragma FENV_ROUND to support constant rounding mode. It also requires some functions to be evaluated with such mode, N3096 7.6.2p4 states:

Within the scope of an FENV_ROUND pragma establishing a mode other
than FE_DYNAMIC ... invocations of functions indicated in the table
below, for which macro replacement has not been suppressed (7.1.4),
shall be evaluated according to the specified constant rounding mode
... . Invocations of functions for which macro replacement has been
suppressed and invocations of functions other than those indicated
in the table below shall not be affected by constant rounding modes
– they are affected by (and affect) only the dynamic mode.

The way this requirement is formulated indicates that it could be implemented using preprocessor facility. Such implementation would require a builtin macro that is set in the region where pragma FENV_ROUND is in effect and reflects constant rounding mode.

This change introduces macro ROUNDING_MODE, which is a string dependent on the constant rounding mode:

FE_TOWARDZERO        "_rtz"
FE_TONEAREST         "_rte"
FE_DOWNWARD          "_rtp"
FE_UPWARD            "_rtn"
FE_TONEARESTFROMZERO "_rta"
FE_DYNAMIC           empty string

All these values except "_rta" are OpenCL rounding mode modifiers. Default value, when no pragma FENV_ROUND is specified, is empty string. Concatenation of a function name with the builtin macro can be used to obtain name of the function variant for particular rounding mode, like "sin_rtz", or "__builtin_cos_rtd". The test "macro_rounding_mode.c" added in this change provides an example of possible use.

The macro is implemented in the same way as FLT_EVAL_METHOD, which also depends on the results of semantic analysis.


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

5 Files Affected:

  • (modified) clang/include/clang/Lex/Preprocessor.h (+12)
  • (modified) clang/lib/Lex/PPMacroExpansion.cpp (+25)
  • (modified) clang/lib/Sema/Sema.cpp (+1)
  • (modified) clang/lib/Sema/SemaAttr.cpp (+1)
  • (added) clang/test/Preprocessor/macro_rounding_mode.c (+55)
diff --git a/clang/include/clang/Lex/Preprocessor.h b/clang/include/clang/Lex/Preprocessor.h
index c0850a8fa9f7f..295633b2e3c73 100644
--- a/clang/include/clang/Lex/Preprocessor.h
+++ b/clang/include/clang/Lex/Preprocessor.h
@@ -181,6 +181,7 @@ class Preprocessor {
   IdentifierInfo *Ident__is_target_variant_os;
   IdentifierInfo *Ident__is_target_variant_environment;
   IdentifierInfo *Ident__FLT_EVAL_METHOD__;        // __FLT_EVAL_METHOD
+  IdentifierInfo *Ident__ROUNDING_MODE__;          // __ROUNDING_MODE__
 
   // Weak, only valid (and set) while InMacroArgs is true.
   Token* ArgMacro;
@@ -201,6 +202,9 @@ class Preprocessor {
   LangOptions::FPEvalMethodKind TUFPEvalMethod =
       LangOptions::FPEvalMethodKind::FEM_UnsetOnCommandLine;
 
+  LangOptions::RoundingMode CurrentRoundingMode =
+      LangOptions::RoundingMode::Dynamic;
+
   // Next __COUNTER__ value, starts at 0.
   unsigned CounterValue = 0;
 
@@ -2356,6 +2360,14 @@ class Preprocessor {
     TUFPEvalMethod = Val;
   }
 
+  LangOptions::RoundingMode getCurrentRoundingMode() const {
+    return CurrentRoundingMode;
+  }
+
+  void setCurrentRoundingMode(LangOptions::RoundingMode RM) {
+    CurrentRoundingMode = RM;
+  }
+
   /// Retrieves the module that we're currently building, if any.
   Module *getCurrentModule();
 
diff --git a/clang/lib/Lex/PPMacroExpansion.cpp b/clang/lib/Lex/PPMacroExpansion.cpp
index 8af4a97d00cb8..519fbfd666375 100644
--- a/clang/lib/Lex/PPMacroExpansion.cpp
+++ b/clang/lib/Lex/PPMacroExpansion.cpp
@@ -344,6 +344,7 @@ void Preprocessor::RegisterBuiltinMacros() {
   Ident__COUNTER__ = RegisterBuiltinMacro(*this, "__COUNTER__");
   Ident_Pragma  = RegisterBuiltinMacro(*this, "_Pragma");
   Ident__FLT_EVAL_METHOD__ = RegisterBuiltinMacro(*this, "__FLT_EVAL_METHOD__");
+  Ident__ROUNDING_MODE__ = RegisterBuiltinMacro(*this, "__ROUNDING_MODE__");
 
   // C++ Standing Document Extensions.
   if (getLangOpts().CPlusPlus)
@@ -1654,6 +1655,30 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) {
       Diag(Tok, diag::err_illegal_use_of_flt_eval_macro);
       Diag(getLastFPEvalPragmaLocation(), diag::note_pragma_entered_here);
     }
+  } else if (II == Ident__ROUNDING_MODE__) {
+    switch (getCurrentRoundingMode()) {
+    case LangOptions::RoundingMode::TowardZero:
+      OS << "_rtz";
+      break;
+    case LangOptions::RoundingMode::NearestTiesToEven:
+      OS << "_rte";
+      break;
+    case LangOptions::RoundingMode::TowardPositive:
+      OS << "_rtp";
+      break;
+    case LangOptions::RoundingMode::TowardNegative:
+      OS << "_rtn";
+      break;
+    case LangOptions::RoundingMode::NearestTiesToAway:
+      OS << "_rta";
+      break;
+    case LangOptions::RoundingMode::Dynamic:
+      OS << "";
+      break;
+    default:
+      llvm_unreachable("unknown rounding mode");
+    }
+    Tok.setKind(tok::string_literal);
   } else if (II == Ident__COUNTER__) {
     // __COUNTER__ expands to a simple numeric value.
     OS << CounterValue++;
diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index f847c49920cf3..928a7953859c9 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -2725,6 +2725,7 @@ Sema::FPFeaturesStateRAII::~FPFeaturesStateRAII() {
   S.CurFPFeatures = OldFPFeaturesState;
   S.FpPragmaStack.CurrentValue = OldOverrides;
   S.PP.setCurrentFPEvalMethod(OldFPPragmaLocation, OldEvalMethod);
+  S.PP.setCurrentRoundingMode(S.CurFPFeatures.getConstRoundingMode());
 }
 
 bool Sema::isDeclaratorFunctionLike(Declarator &D) {
diff --git a/clang/lib/Sema/SemaAttr.cpp b/clang/lib/Sema/SemaAttr.cpp
index bb44531495a56..1faa5a879f030 100644
--- a/clang/lib/Sema/SemaAttr.cpp
+++ b/clang/lib/Sema/SemaAttr.cpp
@@ -1322,6 +1322,7 @@ void Sema::ActOnPragmaFEnvRound(SourceLocation Loc, llvm::RoundingMode FPR) {
   NewFPFeatures.setConstRoundingModeOverride(FPR);
   FpPragmaStack.Act(Loc, PSK_Set, StringRef(), NewFPFeatures);
   CurFPFeatures = NewFPFeatures.applyOverrides(getLangOpts());
+  PP.setCurrentRoundingMode(FPR);
 }
 
 void Sema::setExceptionMode(SourceLocation Loc,
diff --git a/clang/test/Preprocessor/macro_rounding_mode.c b/clang/test/Preprocessor/macro_rounding_mode.c
new file mode 100644
index 0000000000000..a18c72db9bed2
--- /dev/null
+++ b/clang/test/Preprocessor/macro_rounding_mode.c
@@ -0,0 +1,55 @@
+// RUN: %clang_cc1 -emit-llvm -triple i386-linux -Wno-unknown-pragmas %s -o - | FileCheck %s
+
+double sin(double);
+double sin_rte(double);
+double sin_rtz(double);
+double sin_rtp(double);
+double sin_rtn(double);
+double sin_rta(double);
+
+#define CONCAT(a, b) CONCAT_(a, b)
+#define CONCAT_(a, b) a##b
+#define ADD_ROUNDING_MODE_SUFFIX(func) CONCAT(func, __ROUNDING_MODE__)
+
+#define sin(x) ADD_ROUNDING_MODE_SUFFIX(sin)(x)
+
+double call_dyn(double x) {
+  return sin(x);
+}
+// CHECK-LABEL: define {{.*}} double @call_dyn(
+// CHECK:       call double @llvm.sin.f64(
+
+#pragma STDC FENV_ROUND FE_TOWARDZERO
+double call_tz(double x) {
+  return sin(x);
+}
+// CHECK-LABEL: define {{.*}} double @call_tz(
+// CHECK:       call double @sin_rtz(
+
+#pragma STDC FENV_ROUND FE_TONEAREST
+double call_te(double x) {
+  return sin(x);
+}
+// CHECK-LABEL: define {{.*}} double @call_te(
+// CHECK:       call double @sin_rte(
+
+#pragma STDC FENV_ROUND FE_DOWNWARD
+double call_tn(double x) {
+  return sin(x);
+}
+// CHECK-LABEL: define {{.*}} double @call_tn(
+// CHECK:       call double @sin_rtn(
+
+#pragma STDC FENV_ROUND FE_UPWARD
+double call_tp(double x) {
+  return sin(x);
+}
+// CHECK-LABEL: define {{.*}} double @call_tp(
+// CHECK:       call double @sin_rtp(
+
+#pragma STDC FENV_ROUND FE_TONEARESTFROMZERO
+double call_tea(double x) {
+  return sin(x);
+}
+// CHECK-LABEL: define {{.*}} double @call_tea(
+// CHECK:       call double @sin_rta(

As the macro __ROUNDING_MODE__ depends on the current static rounding
mode, which is managed by pragma FENV_ROUND, handling this pragma in
Parser is not possible anymore. Running clang with the option -E should
produce usable source code, where __ROUNDIND_MODE__ is substituted and
Parser is not called in this case. So the handler of pragma FENV_ROUND
is moved from Parser to Preprocessor.

Maintaining the rounding mode in Preprocessor also requires to keep
track of curly brace nesting level, because the pragma can be specified
inside a block, in this case its effect ends as the block is finished
and the previous static rounding mode must be established.
Copy link
Collaborator

@efriedma-quic efriedma-quic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This approach seems much better.

if (Result.is(tok::l_brace)) {
CurlyBraceLevel++;
} else if (Result.is(tok::r_brace)) {
if (!RoundingPragmas.empty() &&
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can move the !RoundingPragmas.empty() outside the if statement, so we can skip a little more of the code if the user isn't using any pragmas? I mean, it probably doesn't make a big difference, but Lex() is performance-sensitive.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A good idea, thank you.

Copy link
Collaborator

@efriedma-quic efriedma-quic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but maybe wait a few days to merge in case someone else has comments.

@spavloff
Copy link
Collaborator Author

Thanks! I will commit it in a couple of day, if no additional feedback is provided.

@jcranmer-intel
Copy link
Contributor

Overall, I'm not opposed to this patch.

This new macro should probably be mentioned somewhere in the clang user documentation.

The way this requirement is formulated indicates that it could be implemented using preprocessor facility. Such implementation would require a builtin macro that is set in the region where pragma FENV_ROUND is in effect and reflects constant rounding mode.

Has there been any discussion with gcc and libc implementations about how they plan to implement this requirement? I'm not entirely convinced this is the best approach, especially given that "printf and scanf families" is on the list of functions that need to be affected, and that's a decent amount of functions to have to wrap with round-mode-aware-variants.

This change introduces macro ROUNDING_MODE, which is a string dependent on the constant rounding mode

It expands to an identify, not a string literal, right?

@efriedma-quic
Copy link
Collaborator

Oh, I somehow thought the macro was part of the spec; reading again, I guess it isn't, it's just an attempt to implement the spec.

We probably want some feedback from libc implementers to check if this is what they want. I don't really want to end up in a situation where we end up implementing this, but then nobody ends up using it. Or we end up with two almost identical macros for the same thing.

@spavloff
Copy link
Collaborator Author

spavloff commented Jun 5, 2024

The discussion for the proposed mechanism is open here: https://discourse.llvm.org/t/rfc-calling-functions-if-pragma-fenv-round-is-present/79372

This change introduces macro ROUNDING_MODE, which is a string dependent on the constant rounding mode

It expands to an identify, not a string literal, right?

Exactly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants