[cfe-commits] r138380 - in /cfe/trunk: include/clang/Basic/SourceManager.h lib/Basic/SourceManager.cpp lib/Lex/TokenLexer.cpp

Argyrios Kyrtzidis akyrtzi at gmail.com
Tue Aug 23 14:02:41 PDT 2011


Author: akirtzidis
Date: Tue Aug 23 16:02:41 2011
New Revision: 138380

URL: http://llvm.org/viewvc/llvm-project?rev=138380&view=rev
Log:
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".

Modified:
    cfe/trunk/include/clang/Basic/SourceManager.h
    cfe/trunk/lib/Basic/SourceManager.cpp
    cfe/trunk/lib/Lex/TokenLexer.cpp

Modified: cfe/trunk/include/clang/Basic/SourceManager.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/SourceManager.h?rev=138380&r1=138379&r2=138380&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/SourceManager.h (original)
+++ cfe/trunk/include/clang/Basic/SourceManager.h Tue Aug 23 16:02:41 2011
@@ -906,6 +906,25 @@
     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.
   //===--------------------------------------------------------------------===//

Modified: cfe/trunk/lib/Basic/SourceManager.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/SourceManager.cpp?rev=138380&r1=138379&r2=138380&view=diff
==============================================================================
--- cfe/trunk/lib/Basic/SourceManager.cpp (original)
+++ cfe/trunk/lib/Basic/SourceManager.cpp Tue Aug 23 16:02:41 2011
@@ -834,10 +834,11 @@
   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);

Modified: cfe/trunk/lib/Lex/TokenLexer.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/TokenLexer.cpp?rev=138380&r1=138379&r2=138380&view=diff
==============================================================================
--- cfe/trunk/lib/Lex/TokenLexer.cpp (original)
+++ cfe/trunk/lib/Lex/TokenLexer.cpp Tue Aug 23 16:02:41 2011
@@ -669,33 +669,51 @@
                                             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 @@
                                             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);
+  }
 }





More information about the cfe-commits mailing list