From 2797df6a24e9065b37022f922da813a133cfaac2 Mon Sep 17 00:00:00 2001 From: Argyrios Kyrtzidis Date: Tue, 23 Aug 2011 21:02:41 +0000 Subject: [PATCH] Amend r138129 (reduction of SLocEntries) which introduced performance regression due to increased calls to SourceManager::getFileID. (rdar://9992664) Use a slightly different approach that is more efficient both in terms of speed (no extra getFileID calls) and in SLocEntries reduction. Comparing pre-r138129 and this patch we get: For compiling SemaExpr.cpp reduction of SLocEntries by 26%. For the boost enum library: -SLocEntries -34% (note that this was -5% for r138129) -Memory consumption -50% -PCH size -31% Reduced SLocEntries also benefit the hot function SourceManager::getFileID, evident by the reduced "FileID scans". llvm-svn: 138380 --- clang/include/clang/Basic/SourceManager.h | 19 +++++++ clang/lib/Basic/SourceManager.cpp | 3 +- clang/lib/Lex/TokenLexer.cpp | 64 ++++++++++++++++------- 3 files changed, 67 insertions(+), 19 deletions(-) diff --git a/clang/include/clang/Basic/SourceManager.h b/clang/include/clang/Basic/SourceManager.h index 8799106a0397..79eba4f3537f 100644 --- a/clang/include/clang/Basic/SourceManager.h +++ b/clang/include/clang/Basic/SourceManager.h @@ -906,6 +906,25 @@ public: return false; } + /// \brief Return true if both \arg LHS and \arg RHS are in the local source + /// location address space or the loaded one. If it's true and + /// \arg RelativeOffset is non-null, it will be set to the offset of \arg RHS + /// relative to \arg LHS. + bool isInSameSLocAddrSpace(SourceLocation LHS, SourceLocation RHS, + int *RelativeOffset) const { + unsigned LHSOffs = LHS.getOffset(), RHSOffs = RHS.getOffset(); + bool LHSLoaded = LHSOffs >= CurrentLoadedOffset; + bool RHSLoaded = RHSOffs >= CurrentLoadedOffset; + + if (LHSLoaded == RHSLoaded) { + if (RelativeOffset) + *RelativeOffset = RHSOffs - LHSOffs; + return true; + } + + return false; + } + //===--------------------------------------------------------------------===// // Queries about the code at a SourceLocation. //===--------------------------------------------------------------------===// diff --git a/clang/lib/Basic/SourceManager.cpp b/clang/lib/Basic/SourceManager.cpp index bf2c8be46795..66ae4861a6a1 100644 --- a/clang/lib/Basic/SourceManager.cpp +++ b/clang/lib/Basic/SourceManager.cpp @@ -834,10 +834,11 @@ SourceManager::getDecomposedSpellingLocSlowCase(const SrcMgr::SLocEntry *E, SourceLocation Loc; do { Loc = E->getExpansion().getSpellingLoc(); + Loc = Loc.getFileLocWithOffset(Offset); FID = getFileID(Loc); E = &getSLocEntry(FID); - Offset += Loc.getOffset()-E->getOffset(); + Offset = Loc.getOffset()-E->getOffset(); } while (!Loc.isFileID()); return std::make_pair(FID, Offset); diff --git a/clang/lib/Lex/TokenLexer.cpp b/clang/lib/Lex/TokenLexer.cpp index 85c03f4e96bb..80f6ad14496a 100644 --- a/clang/lib/Lex/TokenLexer.cpp +++ b/clang/lib/Lex/TokenLexer.cpp @@ -669,33 +669,51 @@ static void updateConsecutiveMacroArgTokens(SourceManager &SM, Token *&begin_tokens, Token * end_tokens) { assert(begin_tokens < end_tokens); - Token &FirstTok = *begin_tokens; - FileID SpellFID = SM.getFileID(FirstTok.getLocation()); - // Look for the first token that is not from the same FileID. - Token *NextFIDTok = begin_tokens + 1; - for (; NextFIDTok < end_tokens; ++NextFIDTok) - if (!SM.isInFileID(NextFIDTok->getLocation(), SpellFID)) + SourceLocation FirstLoc = begin_tokens->getLocation(); + SourceLocation CurLoc = FirstLoc; + + // Compare the source location offset of tokens and group together tokens that + // are close, even if their locations point to different FileIDs. e.g. + // + // |bar | foo | cake | (3 tokens from 3 consecutive FileIDs) + // ^ ^ + // |bar foo cake| (one SLocEntry chunk for all tokens) + // + // we can perform this "merge" since the token's spelling location depends + // on the relative offset. + + Token *NextTok = begin_tokens + 1; + for (; NextTok < end_tokens; ++NextTok) { + int RelOffs; + if (!SM.isInSameSLocAddrSpace(CurLoc, NextTok->getLocation(), &RelOffs)) + break; // Token from different local/loaded location. + // Check that token is not before the previous token or more than 50 + // "characters" away. + if (RelOffs < 0 || RelOffs > 50) break; + CurLoc = NextTok->getLocation(); + } // For the consecutive tokens, find the length of the SLocEntry to contain // all of them. - unsigned FirstOffs, LastOffs; - SM.isInFileID(FirstTok.getLocation(), SpellFID, &FirstOffs); - SM.isInFileID((NextFIDTok-1)->getLocation(), SpellFID, &LastOffs); - unsigned FullLength = (LastOffs - FirstOffs) + (NextFIDTok-1)->getLength(); + Token &LastConsecutiveTok = *(NextTok-1); + int LastRelOffs; + SM.isInSameSLocAddrSpace(FirstLoc, LastConsecutiveTok.getLocation(), + &LastRelOffs); + unsigned FullLength = LastRelOffs + LastConsecutiveTok.getLength(); // Create a macro expansion SLocEntry that will "contain" all of the tokens. SourceLocation Expansion = - SM.createMacroArgExpansionLoc(FirstTok.getLocation(), InstLoc,FullLength); + SM.createMacroArgExpansionLoc(FirstLoc, InstLoc,FullLength); // Change the location of the tokens from the spelling location to the new // expanded location. - for (; begin_tokens < NextFIDTok; ++begin_tokens) { + for (; begin_tokens < NextTok; ++begin_tokens) { Token &Tok = *begin_tokens; - unsigned Offs; - SM.isInFileID(Tok.getLocation(), SpellFID, &Offs); - Tok.setLocation(Expansion.getFileLocWithOffset(Offs - FirstOffs)); + int RelOffs; + SM.isInSameSLocAddrSpace(FirstLoc, Tok.getLocation(), &RelOffs); + Tok.setLocation(Expansion.getFileLocWithOffset(RelOffs)); } } @@ -710,9 +728,19 @@ void TokenLexer::updateLocForMacroArgTokens(SourceLocation ArgIdSpellLoc, Token *end_tokens) { SourceManager &SM = PP.getSourceManager(); - SourceLocation curInst = + SourceLocation InstLoc = getExpansionLocForMacroDefLoc(ArgIdSpellLoc); - while (begin_tokens < end_tokens) - updateConsecutiveMacroArgTokens(SM, curInst, begin_tokens, end_tokens); + while (begin_tokens < end_tokens) { + // If there's only one token just create a SLocEntry for it. + if (end_tokens - begin_tokens == 1) { + Token &Tok = *begin_tokens; + Tok.setLocation(SM.createMacroArgExpansionLoc(Tok.getLocation(), + InstLoc, + Tok.getLength())); + return; + } + + updateConsecutiveMacroArgTokens(SM, InstLoc, begin_tokens, end_tokens); + } }