[PATCH] D80428: [clang] Optimize SourceManager::getFileIDLocal [WIP]

[Marco Elver via Phabricator via cfe-commits]

melver abandoned this revision.
melver added a comment.

More background: https://github.com/ClangBuiltLinux/linux/issues/1032

This approach likely doesn't yield too much benefit. Too much variability is 
observed when compiling Clang with either Clang or GCC.

Since Nick has a better proposal at the above link, let's abandon this.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80428/new/

https://reviews.llvm.org/D80428



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80428: [clang] Optimize SourceManager::getFileIDLocal [WIP]

[Marco Elver via Phabricator via cfe-commits]

melver updated this revision to Diff 265780.
melver retitled this revision from "[clang] Optimize getFileIDLocal [WIP]" to 
"[clang] Optimize SourceManager::getFileIDLocal [WIP]".
melver edited the summary of this revision.
melver added a comment.

Improve code-gen further -- another ~5%.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80428/new/

https://reviews.llvm.org/D80428

Files:
  clang/include/clang/Basic/SourceManager.h
  clang/lib/Basic/SourceManager.cpp


Index: clang/lib/Basic/SourceManager.cpp
===
--- clang/lib/Basic/SourceManager.cpp
+++ clang/lib/Basic/SourceManager.cpp
@@ -885,7 +885,7 @@
 bool Invalid = false;
 unsigned MiddleIndex = (GreaterIndex-LessIndex)/2+LessIndex;
 unsigned MidOffset = getLocalSLocEntry(MiddleIndex, ).getOffset();
-if (Invalid)
+if (LLVM_UNLIKELY(Invalid))
   return FileID::get(0);
 
 ++NumProbes;
@@ -896,13 +896,15 @@
   GreaterIndex = MiddleIndex;
   continue;
 }
+// Otherwise, move the low-side up to the middle index. Used in next
+// iteration if !isOffsetInFileID() below.
+LessIndex = MiddleIndex;
 
 // If the middle index contains the value, succeed and return.
 // FIXME: This could be made faster by using a function that's aware of
 // being in the local area.
-if (isOffsetInFileID(FileID::get(MiddleIndex), SLocOffset)) {
-  FileID Res = FileID::get(MiddleIndex);
-
+FileID Res = FileID::get(MiddleIndex);
+if (LLVM_UNLIKELY(isOffsetInFileID(Res, SLocOffset))) {
   // If this isn't a macro expansion, remember it.  We have good locality
   // across FileID lookups.
   if (!LocalSLocEntryTable[MiddleIndex].isExpansion())
@@ -910,9 +912,6 @@
   NumBinaryProbes += NumProbes;
   return Res;
 }
-
-// Otherwise, move the low-side up to the middle index.
-LessIndex = MiddleIndex;
   }
 }
 
Index: clang/include/clang/Basic/SourceManager.h
===
--- clang/include/clang/Basic/SourceManager.h
+++ clang/include/clang/Basic/SourceManager.h
@@ -439,8 +439,8 @@
   /// SourceManager keeps an array of these objects, and they are uniquely
   /// identified by the FileID datatype.
   class SLocEntry {
-unsigned Offset : 31;
-unsigned IsExpansion : 1;
+unsigned Offset;
+bool IsExpansion;
 union {
   FileInfo File;
   ExpansionInfo Expansion;
@@ -465,7 +465,6 @@
 }
 
 static SLocEntry get(unsigned Offset, const FileInfo ) {
-  assert(!(Offset & (1u << 31)) && "Offset is too large");
   SLocEntry E;
   E.Offset = Offset;
   E.IsExpansion = false;
@@ -474,7 +473,6 @@
 }
 
 static SLocEntry get(unsigned Offset, const ExpansionInfo ) {
-  assert(!(Offset & (1u << 31)) && "Offset is too large");
   SLocEntry E;
   E.Offset = Offset;
   E.IsExpansion = true;


Index: clang/lib/Basic/SourceManager.cpp
===
--- clang/lib/Basic/SourceManager.cpp
+++ clang/lib/Basic/SourceManager.cpp
@@ -885,7 +885,7 @@
 bool Invalid = false;
 unsigned MiddleIndex = (GreaterIndex-LessIndex)/2+LessIndex;
 unsigned MidOffset = getLocalSLocEntry(MiddleIndex, ).getOffset();
-if (Invalid)
+if (LLVM_UNLIKELY(Invalid))
   return FileID::get(0);
 
 ++NumProbes;
@@ -896,13 +896,15 @@
   GreaterIndex = MiddleIndex;
   continue;
 }
+// Otherwise, move the low-side up to the middle index. Used in next
+// iteration if !isOffsetInFileID() below.
+LessIndex = MiddleIndex;
 
 // If the middle index contains the value, succeed and return.
 // FIXME: This could be made faster by using a function that's aware of
 // being in the local area.
-if (isOffsetInFileID(FileID::get(MiddleIndex), SLocOffset)) {
-  FileID Res = FileID::get(MiddleIndex);
-
+FileID Res = FileID::get(MiddleIndex);
+if (LLVM_UNLIKELY(isOffsetInFileID(Res, SLocOffset))) {
   // If this isn't a macro expansion, remember it.  We have good locality
   // across FileID lookups.
   if (!LocalSLocEntryTable[MiddleIndex].isExpansion())
@@ -910,9 +912,6 @@
   NumBinaryProbes += NumProbes;
   return Res;
 }
-
-// Otherwise, move the low-side up to the middle index.
-LessIndex = MiddleIndex;
   }
 }
 
Index: clang/include/clang/Basic/SourceManager.h
===
--- clang/include/clang/Basic/SourceManager.h
+++ clang/include/clang/Basic/SourceManager.h
@@ -439,8 +439,8 @@
   /// SourceManager keeps an array of these objects, and they are uniquely
   /// identified by the FileID datatype.
   class SLocEntry {
-unsigned Offset : 31;
-unsigned IsExpansion : 1;
+unsigned Offset;
+bool IsExpansion;
 union {
   FileInfo File;
   ExpansionInfo Expansion;
@@ -465,7