[clang] Change representation of CurLexerKind

[serge-sans-paille]

Previous representation used an enumeration combined to a switch to dispatch to the appropriate lexer.

Use function pointer so that the dispatching is just an indirect call, which is actually better because lexing is a costly task compared to a function call.

This also makes the code slightly cleaner, speedup on compile time tracker are consistent and range form -0.05% to -0.20% for NewPM-O0-g, see

    https://llvm-compile-time-tracker.com/compare.php?from=f9906508bc4f05d3950e2219b4c56f6c078a61ef&to=608c85ec1283638db949d73e062bcc3355001ce4&stat=instructions:u

Considering just the preprocessing task, preprocessing the sqlite amalgametion takes -0.6% instructions (according to valgrind --tool=callgrind)

[llvmbot]

@llvm/pr-subscribers-clang

Author: None (serge-sans-paille)

Changes

Previous representation used an enumeration combined to a switch to dispatch to the appropriate lexer.

Use function pointer so that the dispatching is just an indirect call, which is actually better because lexing is a costly task compared to a function call.

This also makes the code slightly cleaner, speedup on compile time tracker are consistent and range form -0.05% to -0.20% for NewPM-O0-g, see

    https://llvm-compile-time-tracker.com/compare.php?from=f9906508bc4f05d3950e2219b4c56f6c078a61ef&to=608c85ec1283638db949d73e062bcc3355001ce4&stat=instructions:u

Considering just the preprocessing task, preprocessing the sqlite amalgametion takes -0.6% instructions (according to valgrind --tool=callgrind)

---

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

2 Files Affected:

• (modified) clang/include/clang/Lex/Preprocessor.h (+24-9)
• (modified) clang/lib/Lex/Preprocessor.cpp (+3-38)

diff --git a/clang/include/clang/Lex/Preprocessor.h b/clang/include/clang/Lex/Preprocessor.h
index 18d88407ae12c90..c9ecb0844526eeb 100644
--- a/clang/include/clang/Lex/Preprocessor.h
+++ b/clang/include/clang/Lex/Preprocessor.h
@@ -751,13 +751,8 @@ class Preprocessor {
   std::unique_ptr<TokenLexer> CurTokenLexer;
 
   /// The kind of lexer we're currently working with.
-  enum CurLexerKind {
-    CLK_Lexer,
-    CLK_TokenLexer,
-    CLK_CachingLexer,
-    CLK_DependencyDirectivesLexer,
-    CLK_LexAfterModuleImport
-  } CurLexerKind = CLK_Lexer;
+  typedef bool (*CurLexerKindType)(Preprocessor &, Token &);
+  CurLexerKindType CurLexerKind = &CLK_Lexer;
 
   /// If the current lexer is for a submodule that is being built, this
   /// is that submodule.
@@ -767,7 +762,7 @@ class Preprocessor {
   /// \#included, and macros currently being expanded from, not counting
   /// CurLexer/CurTokenLexer.
   struct IncludeStackInfo {
-    enum CurLexerKind           CurLexerKind;
+    CurLexerKindType            CurLexerKind;
     Module                     *TheSubmodule;
     std::unique_ptr<Lexer>      TheLexer;
     PreprocessorLexer          *ThePPLexer;
@@ -776,7 +771,7 @@ class Preprocessor {
 
     // The following constructors are completely useless copies of the default
     // versions, only needed to pacify MSVC.
-    IncludeStackInfo(enum CurLexerKind CurLexerKind, Module *TheSubmodule,
+    IncludeStackInfo(CurLexerKindType CurLexerKind, Module *TheSubmodule,
                      std::unique_ptr<Lexer> &&TheLexer,
                      PreprocessorLexer *ThePPLexer,
                      std::unique_ptr<TokenLexer> &&TheTokenLexer,
@@ -2899,6 +2894,26 @@ class Preprocessor {
   /// \return true iff this PP is currently in a "-Wunsafe-buffer-usage"
   ///          opt-out region
   bool isPPInSafeBufferOptOutRegion(SourceLocation &StartLoc);
+
+private:
+  /// Helper functions to forward lexing to the actual lexer. They all share the
+  /// same signature.
+  static bool CLK_Lexer(Preprocessor &P, Token &Result) {
+    return P.CurLexer->Lex(Result);
+  }
+  static bool CLK_TokenLexer(Preprocessor &P, Token &Result) {
+    return P.CurTokenLexer->Lex(Result);
+  }
+  static bool CLK_CachingLexer(Preprocessor &P, Token &Result) {
+    P.CachingLex(Result);
+    return true;
+  }
+  static bool CLK_DependencyDirectivesLexer(Preprocessor &P, Token &Result) {
+    return P.CurLexer->LexDependencyDirectiveToken(Result);
+  }
+  static bool CLK_LexAfterModuleImport(Preprocessor &P, Token &Result) {
+    return P.LexAfterModuleImport(Result);
+  }
 };
 
 /// Abstract base class that describes a handler that will receive
diff --git a/clang/lib/Lex/Preprocessor.cpp b/clang/lib/Lex/Preprocessor.cpp
index ede4c51487ffbe7..a1ddb7732b9b857 100644
--- a/clang/lib/Lex/Preprocessor.cpp
+++ b/clang/lib/Lex/Preprocessor.cpp
@@ -643,23 +643,7 @@ void Preprocessor::SkipTokensWhileUsingPCH() {
   while (true) {
     bool InPredefines =
         (CurLexer && CurLexer->getFileID() == getPredefinesFileID());
-    switch (CurLexerKind) {
-    case CLK_Lexer:
-      CurLexer->Lex(Tok);
-     break;
-    case CLK_TokenLexer:
-      CurTokenLexer->Lex(Tok);
-      break;
-    case CLK_CachingLexer:
-      CachingLex(Tok);
-      break;
-    case CLK_DependencyDirectivesLexer:
-      CurLexer->LexDependencyDirectiveToken(Tok);
-      break;
-    case CLK_LexAfterModuleImport:
-      LexAfterModuleImport(Tok);
-      break;
-    }
+    (void)CurLexerKind(*this, Tok);
     if (Tok.is(tok::eof) && !InPredefines) {
       ReachedMainFileEOF = true;
       break;
@@ -882,27 +866,8 @@ void Preprocessor::Lex(Token &Result) {
   ++LexLevel;
 
   // We loop here until a lex function returns a token; this avoids recursion.
-  bool ReturnedToken;
-  do {
-    switch (CurLexerKind) {
-    case CLK_Lexer:
-      ReturnedToken = CurLexer->Lex(Result);
-      break;
-    case CLK_TokenLexer:
-      ReturnedToken = CurTokenLexer->Lex(Result);
-      break;
-    case CLK_CachingLexer:
-      CachingLex(Result);
-      ReturnedToken = true;
-      break;
-    case CLK_DependencyDirectivesLexer:
-      ReturnedToken = CurLexer->LexDependencyDirectiveToken(Result);
-      break;
-    case CLK_LexAfterModuleImport:
-      ReturnedToken = LexAfterModuleImport(Result);
-      break;
-    }
-  } while (!ReturnedToken);
+  while (!CurLexerKind(*this, Result))
+    ;
 
   if (Result.is(tok::unknown) && TheModuleLoader.HadFatalFailure)
     return;

[github-actions]

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff d0f6825da2d155ccfc4d07c51ae8676a861791ea dfa80ef2ac912fb61c0a3bb348f6a63c49a12dbf -- clang/include/clang/Lex/Preprocessor.h clang/lib/Lex/PPCaching.cpp clang/lib/Lex/PPLexerChange.cpp clang/lib/Lex/Preprocessor.cpp

View the diff from clang-format here.

diff --git a/clang/include/clang/Lex/Preprocessor.h b/clang/include/clang/Lex/Preprocessor.h
index 4ec21a8b6be2..0407c7ef8d53 100644
--- a/clang/include/clang/Lex/Preprocessor.h
+++ b/clang/include/clang/Lex/Preprocessor.h
@@ -762,7 +762,7 @@ private:
   /// \#included, and macros currently being expanded from, not counting
   /// CurLexer/CurTokenLexer.
   struct IncludeStackInfo {
-    LexerCallback               CurLexerCallback;
+    LexerCallback CurLexerCallback;
     Module                     *TheSubmodule;
     std::unique_ptr<Lexer>      TheLexer;
     PreprocessorLexer          *ThePPLexer;

[serge-sans-paille]

Updated compile time metrics: https://llvm-compile-time-tracker.com/compare.php?from=f9906508bc4f05d3950e2219b4c56f6c078a61ef&to=2631a5163de552a8e482f5cbc51525fb8bb5082f&stat=instructions:u

[cor3ntin]

This is excellent!
I made some naming suggestion. Now that we don't have an enum i think it is clearer to drop the "kind" terminology

[serge-sans-paille]

Agreed. Sorry I pushed --force my change out of habit :-/

[serge-sans-paille]

As expected, the formatter iw now red, otherwise looks good (to me at least), waiting for official approval :-)

[AaronBallman]

LGTM! It's always great to improve compile-time performance!