[cfe-commits] r99012 - in /cfe/trunk: lib/CodeGen/CGCXX.cpp lib/CodeGen/CGDebugInfo.cpp lib/CodeGen/CGDecl.cpp lib/CodeGen/CodeGenModule.cpp lib/CodeGen/CodeGenModule.h lib/CodeGen/Mangle.h test/CodeGenCXX/attr.cpp

[John McCall]

On Mar 23, 2010, at 2:54 PM, Chris Lattner wrote:

> 
> On Mar 19, 2010, at 4:29 PM, John McCall wrote:
> 
>> Author: rjmccall
>> Date: Fri Mar 19 18:29:14 2010
>> New Revision: 99012
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=99012&view=rev
>> Log:
>> Change CodeGenModule to rely on the Module's symbol table instead of
>> shadowing it in the GlobalDeclMap.  Eliminates the string-uniquing
>> requirement for mangled names, which should help C++ codegen times a little.
>> Forces us to do string lookups instead of pointer lookups, which might hurt
>> codegen times a little across the board.  We'll see how it plays out.
>> 
>> Removing the string-uniquing requirement implicitly fixes any bugs like
>> PR6635 which arose from the fact that we had multiple uniquing tables for
>> different kinds of identifiers.
> 
> Hi John,
> 
> Why not get the best of both worlds and map from StringMapEntry*'s instead of the string itself?  The stringmap entry can be obtained from the LLVM Module, no?

Not as far as I can tell;  you can ask a module for its ValueSymbolTable, but you can't ask a ValueSymbolTable for its StringMap<Value*> or any other private data.  It looks like I could reinterpret_cast to StringMap, but I suspect you're not suggesting that. :)  I could certainly expand the ValueSymbolTable API to give direct access to the StringMap;  that breaks encapsulation but otherwise isn't too horrible.

I don't think keying off StringMapEntry* helps at all unless we shove a StringMapEntry* into the IdentifierInfo or something.  The loss of efficiency is that we're currently doing string lookups where (in C but not C++) we used to get away with pointer hashes because the lexer was uniquing identifier strings for us.  Keying off the module's StringMapEntry would require us to look that up in the module's ValueSymbolTable, which is basically exactly what we're doing now.  That said, we sometimes have to do extra redundant lookups because of ValueSymbolTable has such a constrained API, and being able to pass around the StringMapEntry would help us avoid that.

John.