-
Notifications
You must be signed in to change notification settings - Fork 13.3k
[clang] Macro for constant rounding mode #92699
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
base: main
Are you sure you want to change the base?
Conversation
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.
15b6edb
to
f8cd253
Compare
@@ -0,0 +1,55 @@ | |||
// RUN: %clang_cc1 -emit-llvm -triple i386-linux -Wno-unknown-pragmas %s -o - | FileCheck %s |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
@llvm/pr-subscribers-clang Author: Serge Pavlov (spavloff) ChangesThe 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:
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:
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:
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.
There was a problem hiding this 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.
clang/lib/Lex/Preprocessor.cpp
Outdated
if (Result.is(tok::l_brace)) { | ||
CurlyBraceLevel++; | ||
} else if (Result.is(tok::r_brace)) { | ||
if (!RoundingPragmas.empty() && |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this 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.
Thanks! I will commit it in a couple of day, if no additional feedback is provided. |
Overall, I'm not opposed to this patch. This new macro should probably be mentioned somewhere in the clang user documentation.
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 "
It expands to an identify, not a string literal, right? |
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. |
The discussion for the proposed mechanism is open here: https://discourse.llvm.org/t/rfc-calling-functions-if-pragma-fenv-round-is-present/79372
Exactly. |
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:
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:
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.