-
Notifications
You must be signed in to change notification settings - Fork 12.3k
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
[ExprConst] Handle floating- and char literals in FastEvaluateAsRValue #118294
Conversation
|
@llvm/pr-subscribers-clang Author: Timm Baeder (tbaederr) ChangesThis is part of a three-patch series that results in some nice (but not substantial) compile-time improvements: http://llvm-compile-time-tracker.com/compare.php?from=fe1c4f0106fe4fd6d61c38ba46e71fda8f4d1573&to=0824d621b2c035a3befb564153b31309a9a79d97&stat=instructions%3Au The results for just this patch are here: http://llvm-compile-time-tracker.com/compare.php?from=fe1c4f0106fe4fd6d61c38ba46e71fda8f4d1573&to=6f7f51b476a37dc7c80427fede077e6798a83be8&stat=instructions:u Full diff: https://github.com/llvm/llvm-project/pull/118294.diff 1 Files Affected:
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index bb5ab67328fbc6..fa0661f65fceaf 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -16473,7 +16473,7 @@ static bool FastEvaluateAsRValue(const Expr *Exp, Expr::EvalResult &Result,
const ASTContext &Ctx, bool &IsConst) {
// Fast-path evaluations of integer literals, since we sometimes see files
// containing vast quantities of these.
- if (const IntegerLiteral *L = dyn_cast<IntegerLiteral>(Exp)) {
+ if (const auto *L = dyn_cast<IntegerLiteral>(Exp)) {
Result.Val = APValue(APSInt(L->getValue(),
L->getType()->isUnsignedIntegerType()));
IsConst = true;
@@ -16486,6 +16486,18 @@ static bool FastEvaluateAsRValue(const Expr *Exp, Expr::EvalResult &Result,
return true;
}
+ if (const auto *FL = dyn_cast<FloatingLiteral>(Exp)) {
+ Result.Val = APValue(FL->getValue());
+ IsConst = true;
+ return true;
+ }
+
+ if (const auto *L = dyn_cast<CharacterLiteral>(Exp)) {
+ Result.Val = APValue(Ctx.MakeIntValue(L->getValue(), L->getType()));
+ IsConst = true;
+ return true;
+ }
+
if (const auto *CE = dyn_cast<ConstantExpr>(Exp)) {
if (CE->hasAPValueResult()) {
APValue APV = CE->getAPValueResult();
|
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.
Seems fine, though I wonder at what point we might want to rewrite this to be a switch statement on the StmtClass because we’re checking for like 5 different expressions here at this point
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
llvm#118294) This is part of a three-patch series that results in some nice (but not substantial) compile-time improvements: http://llvm-compile-time-tracker.com/compare.php?from=fe1c4f0106fe4fd6d61c38ba46e71fda8f4d1573&to=0824d621b2c035a3befb564153b31309a9a79d97&stat=instructions%3Au The results for just this patch are here: http://llvm-compile-time-tracker.com/compare.php?from=fe1c4f0106fe4fd6d61c38ba46e71fda8f4d1573&to=6f7f51b476a37dc7c80427fede077e6798a83be8&stat=instructions:u
This is part of a three-patch series that results in some nice (but not substantial) compile-time improvements: http://llvm-compile-time-tracker.com/compare.php?from=fe1c4f0106fe4fd6d61c38ba46e71fda8f4d1573&to=0824d621b2c035a3befb564153b31309a9a79d97&stat=instructions%3Au
The results for just this patch are here: http://llvm-compile-time-tracker.com/compare.php?from=fe1c4f0106fe4fd6d61c38ba46e71fda8f4d1573&to=6f7f51b476a37dc7c80427fede077e6798a83be8&stat=instructions:u