[NFC] Optimize file kind determination #139492
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There are checks in clang codebase that determine the type of source file, associated with a given location - specifically, if it is an ordonary file or comes from sources like command-line options or a built-in definitions. These checks often rely on calls to `getPresumedLoc`, which is relatively expensive. In certain cases, these checks are combined, leading to repeated calculations of the costly function negatively affecting compile time. This change tries to optimize such checks. It must fix compile time regression introduced in llvm#137306.
spavloff
requested review from
nikic,
a-tarasyuk,
jmmartinez,
AaronBallman and
lamb-j
spavloff
requested review from
daniel-grumberg and
QuietMisdreavus
as code owners
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Serge Pavlov (spavloff) ChangesThere are checks in clang codebase that determine the type of source file, associated with a given location - specifically, if it is an ordonary file or comes from sources like command-line options or a built-in definitions. These checks often rely on calls to This change tries to optimize such checks. It must fix compile time regression introduced in #137306. Full diff: https://github.com/llvm/llvm-project/pull/139492.diff 4 Files Affected:
diff --git a/clang/include/clang/Basic/SourceManager.h b/clang/include/clang/Basic/SourceManager.h
index e0f1ea435d54e..8859865a1ee7e 100644
--- a/clang/include/clang/Basic/SourceManager.h
+++ b/clang/include/clang/Basic/SourceManager.h
@@ -1529,6 +1529,15 @@ class SourceManager : public RefCountedBase<SourceManager> {
return Filename == "<scratch space>";
}
+ /// Returns whether \p Loc is located in a built-ins or command line sources.
+ bool isInPredefinedFile(SourceLocation Loc) const {
+ PresumedLoc Presumed = getPresumedLoc(Loc);
+ if (Presumed.isInvalid())
+ return false;
+ StringRef Filename(Presumed.getFilename());
+ return Filename == "<built-in>" || Filename == "<command line>";
+ }
+
/// Returns if a SourceLocation is in a system header.
bool isInSystemHeader(SourceLocation Loc) const {
if (Loc.isInvalid())
diff --git a/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp b/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
index 6f42b36bd36a4..764c345a9db99 100644
--- a/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
+++ b/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
@@ -305,8 +305,7 @@ class MacroCallback : public PPCallbacks {
auto DefLoc = MI->getDefinitionLoc();
- if (SM.isWrittenInBuiltinFile(DefLoc) ||
- SM.isWrittenInCommandLineFile(DefLoc))
+ if (SM.isInPredefinedFile(DefLoc))
continue;
auto AssociatedModuleMacros = MD.getModuleMacros();
diff --git a/clang/lib/Frontend/PrintPreprocessedOutput.cpp b/clang/lib/Frontend/PrintPreprocessedOutput.cpp
index 2ae355fb33885..22ba4cee182af 100644
--- a/clang/lib/Frontend/PrintPreprocessedOutput.cpp
+++ b/clang/lib/Frontend/PrintPreprocessedOutput.cpp
@@ -569,8 +569,7 @@ void PrintPPOutputPPCallbacks::MacroDefined(const Token &MacroNameTok,
SourceLocation DefLoc = MI->getDefinitionLoc();
if (DirectivesOnly && !MI->isUsed()) {
SourceManager &SM = PP.getSourceManager();
- if (SM.isWrittenInBuiltinFile(DefLoc) ||
- SM.isWrittenInCommandLineFile(DefLoc))
+ if (SM.isInPredefinedFile(DefLoc))
return;
}
MoveToLine(DefLoc, /*RequireStartOfLine=*/true);
diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp
index 384d167cbcf88..c2bab9118234c 100644
--- a/clang/lib/Lex/PPDirectives.cpp
+++ b/clang/lib/Lex/PPDirectives.cpp
@@ -374,9 +374,8 @@ bool Preprocessor::CheckMacroName(Token &MacroNameTok, MacroUse isDefineUndef,
// Macro names with reserved identifiers are accepted if built-in or passed
// through the command line (the later may be present if -dD was used to
// generate the preprocessed file).
- bool IsBuiltinOrCmd = SourceMgr.isWrittenInBuiltinFile(MacroNameLoc) ||
- SourceMgr.isWrittenInCommandLineFile(MacroNameLoc);
- if (!IsBuiltinOrCmd && !SourceMgr.isInSystemHeader(MacroNameLoc)) {
+ if (!SourceMgr.isInPredefinedFile(MacroNameLoc) &&
+ !SourceMgr.isInSystemHeader(MacroNameLoc)) {
MacroDiag D = MD_NoWarn;
if (isDefineUndef == MU_Define) {
D = shouldWarnOnMacroDef(*this, II);
@@ -1706,8 +1705,7 @@ void Preprocessor::HandleDigitDirective(Token &DigitTok) {
// If a filename was present, read any flags that are present.
if (ReadLineMarkerFlags(IsFileEntry, IsFileExit, FileKind, *this))
return;
- if (!SourceMgr.isWrittenInBuiltinFile(DigitTok.getLocation()) &&
- !SourceMgr.isWrittenInCommandLineFile(DigitTok.getLocation()))
+ if (!SourceMgr.isInPredefinedFile(DigitTok.getLocation()))
Diag(StrTok, diag::ext_pp_gnu_line_directive);
// Exiting to an empty string means pop to the including file, so leave
|
cor3ntin
approved these changes
May 12, 2025
Co-authored-by: cor3ntin <[email protected]>
jmmartinez
approved these changes
May 12, 2025
There was a problem hiding this comment.
Thanks @spavloff ! I'm sorry I didn't address the remarks earlier, I was out-of-office and didn't see the notifications.
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/66/builds/13818 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There are checks in clang codebase that determine the type of source file, associated with a given location - specifically, if it is an ordonary file or comes from sources like command-line options or a built-in definitions. These checks often rely on calls to
getPresumedLoc, which is relatively expensive. In certain cases, these checks are combined, leading to repeated calculations of the costly function negatively affecting compile time.This change tries to optimize such checks. It must fix compile time regression introduced in #137306.