Skip to content

Commit

Permalink
[Tooling/Inclusion] Refactor to use tables for compile time
Browse files Browse the repository at this point in the history 
The macro expansion takes 13s and generates an 1.5M obj file, table uses
2s and 680k .o file.

Sanitizers take multiple minutes to compile the old version, while
having negligible overhead on the new version.

No change in functionality.
  • Loading branch information
@d0k
d0k committed Oct 24, 2023
1 parent febc4ff commit 4674e30
Showing 1 changed file with 24 additions and 4 deletions.
28 changes: 24 additions & 4 deletions clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,20 +55,26 @@ static const SymbolHeaderMapping *getMappingPerLang(Lang L) {
}

static int countSymbols(Lang Language) {
llvm::DenseSet<llvm::StringRef> Set;
#define SYMBOL(Name, NS, Header) Set.insert(#NS #Name);
ArrayRef<const char*> Symbols;
#define SYMBOL(Name, NS, Header) #NS #Name,
switch (Language) {
case Lang::C:
static constexpr const char *CSymbols[] = {
#include "CSymbolMap.inc"
};
Symbols = CSymbols;
break;
case Lang::CXX:
static constexpr const char *CXXSymbols[] = {
#include "StdSpecialSymbolMap.inc"
#include "StdSymbolMap.inc"
#include "StdTsSymbolMap.inc"
};
Symbols = CXXSymbols;
break;
}
#undef SYMBOL
return Set.size();
return llvm::DenseSet<StringRef>(Symbols.begin(), Symbols.end()).size();
}

static int initialize(Lang Language) {
Expand Down Expand Up @@ -127,15 +133,29 @@ static int initialize(Lang Language) {
NSSymbolMap &NSSymbols = AddNS(QName.take_front(NSLen));
NSSymbols.try_emplace(QName.drop_front(NSLen), SymIndex);
};
#define SYMBOL(Name, NS, Header) Add(#NS #Name, strlen(#NS), #Header);

struct Symbol {
const char *QName;
unsigned NSLen;
const char *HeaderName;
};
#define SYMBOL(Name, NS, Header) {#NS #Name, StringRef(#NS).size(), #Header},
switch (Language) {
case Lang::C:
static constexpr Symbol CSymbols[] = {
#include "CSymbolMap.inc"
};
for (const Symbol &S : CSymbols)
Add(S.QName, S.NSLen, S.HeaderName);
break;
case Lang::CXX:
static constexpr Symbol CXXSymbols[] = {
#include "StdSpecialSymbolMap.inc"
#include "StdSymbolMap.inc"
#include "StdTsSymbolMap.inc"
};
for (const Symbol &S : CXXSymbols)
Add(S.QName, S.NSLen, S.HeaderName);
break;
}
#undef SYMBOL
Expand Down

2 comments on commit 4674e30

@mikaelholmen
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,

When compiling this patch with gcc we get lots of warnings like

../../clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp:144:65: warning: narrowing conversion of 'llvm::StringRef(((const char*)"std::")).llvm::StringRef::size()' from 'size_t' {aka 'long unsigned int'} to 'unsigned int' [-Wnarrowing]
  144 | #define SYMBOL(Name, NS, Header) {#NS #Name, StringRef(#NS).size(), #Header},
        |                                              ~~~~~~~~~~~~~~~~~~~^~
../../clang/lib/Tooling/Inclusions/Stdlib/StdSymbolMap.inc:2755:1: note: in expansion of macro 'SYMBOL'
 2755 | SYMBOL(string, std::, <string>)
      | ^~~~~~

I guess because we write size_t in "unsigned NSLen".
Anything to worry about?

@d0k
Copy link
Member Author

@d0k d0k commented on 4674e30 Oct 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,

When compiling this patch with gcc we get lots of warnings like

../../clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp:144:65: warning: narrowing conversion of 'llvm::StringRef(((const char*)"std::")).llvm::StringRef::size()' from 'size_t' {aka 'long unsigned int'} to 'unsigned int' [-Wnarrowing]
  144 | #define SYMBOL(Name, NS, Header) {#NS #Name, StringRef(#NS).size(), #Header},
        |                                              ~~~~~~~~~~~~~~~~~~~^~
../../clang/lib/Tooling/Inclusions/Stdlib/StdSymbolMap.inc:2755:1: note: in expansion of macro 'SYMBOL'
 2755 | SYMBOL(string, std::, <string>)
      | ^~~~~~

I guess because we write size_t in "unsigned NSLen". Anything to worry about?

It's fine, the strings are all constants and the size will never exceed unsigned. We can add an explicit cast to get rid of the warnings.

Please sign in to comment.