Skip to content

[clang] Avoid unnecessary call to clang::NamespaceDecl::isRedundantInlineQualifierFor(). #115196

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

Merged
merged 1 commit into from
Nov 8, 2024

Conversation

ZequanWu
Copy link
Contributor

@ZequanWu ZequanWu commented Nov 6, 2024

We observed 2X slowdown in lldb's expression evaluation with #109147 in some cases. It turns out that calling isRedundantInlineQualifierFor is quite expensive. Using short-circuit evaluation in the if statement to avoid unnecessary calls to that function.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Nov 6, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 6, 2024

@llvm/pr-subscribers-clang

Author: Zequan Wu (ZequanWu)

Changes

We observed 2X slowdown in lldb's expression evaluation with #109147. It turns out that calling isRedundantInlineQualifierFor is quite expensive. Using short-circuit evaluation in the if statement to avoid unnecessary calls to that function.


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

1 Files Affected:

  • (modified) clang/lib/AST/Decl.cpp (+2-3)
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 8204e3509dd563..a65dc85e04a83d 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -1738,13 +1738,12 @@ void NamedDecl::printNestedNameSpecifier(raw_ostream &OS,
 
     // Suppress inline namespace if it doesn't make the result ambiguous.
     if (Ctx->isInlineNamespace() && NameInScope) {
-      bool isRedundant =
-          cast<NamespaceDecl>(Ctx)->isRedundantInlineQualifierFor(NameInScope);
       if (P.SuppressInlineNamespace ==
               PrintingPolicy::SuppressInlineNamespaceMode::All ||
           (P.SuppressInlineNamespace ==
                PrintingPolicy::SuppressInlineNamespaceMode::Redundant &&
-           isRedundant)) {
+           cast<NamespaceDecl>(Ctx)->isRedundantInlineQualifierFor(
+               NameInScope))) {
         continue;
       }
     }

@labath
Copy link
Collaborator

labath commented Nov 7, 2024

This makes sense to me, but I'll leave the review to someone from the clang team.

@labath labath removed their request for review November 7, 2024 09:33
@ZequanWu ZequanWu merged commit e579632 into llvm:main Nov 8, 2024
9 of 11 checks passed
Groverkss pushed a commit to iree-org/llvm-project that referenced this pull request Nov 15, 2024
…lineQualifierFor(). (llvm#115196)

We observed 2X slowdown in lldb's expression evaluation with
llvm#109147 in some cases. It turns
out that calling `isRedundantInlineQualifierFor` is quite expensive.
Using short-circuit evaluation in the if statement to avoid unnecessary
calls to that function.
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.

4 participants