[cfe-commits] r147617 - in /cfe/trunk: include/clang/AST/DeclBase.h include/clang/Serialization/ASTReader.h lib/AST/DeclBase.cpp lib/Serialization/ASTReaderDecl.cpp lib/Serialization/ASTWriter.cpp

Benjamin Kramer benny.kra at googlemail.com
Thu Jan 5 15:47:51 PST 2012


On 05.01.2012, at 23:27, Douglas Gregor wrote:

> Author: dgregor
> Date: Thu Jan  5 16:27:05 2012
> New Revision: 147617
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=147617&view=rev
> Log:
> When we deserialize a declaration from a module file, allocate extra
> storage for the global declaration ID. Declarations that are parsed
> (rather than deserialized) are unaffected, so the number of
> declarations that pay this cost tends to be relatively small (since
> relatively few declarations are ever deserialized).
> 
> This replaces a largish DenseMap within the AST reader. It's not
> strictly a win in terms of memory use---not every declaration was
> added to that DenseMap in the first place---but it's cleaner to have
> this information available for every deserialized declaration, so that
> future clients can rely on it.
> 
> 
> Modified:
>    cfe/trunk/include/clang/AST/DeclBase.h
>    cfe/trunk/include/clang/Serialization/ASTReader.h
>    cfe/trunk/lib/AST/DeclBase.cpp
>    cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
>    cfe/trunk/lib/Serialization/ASTWriter.cpp
> 
> Modified: cfe/trunk/include/clang/AST/DeclBase.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=147617&r1=147616&r2=147617&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/AST/DeclBase.h (original)
> +++ cfe/trunk/include/clang/AST/DeclBase.h Thu Jan  5 16:27:05 2012
> @@ -532,6 +532,14 @@
>   /// a precompiled header or module) rather than having been parsed.
>   bool isFromASTFile() const { return FromASTFile; }
> 
> +  /// \brief Retrieve the global declaration ID associated with this 
> +  /// declaration, which specifies where in the 
> +  unsigned getGlobalID() const { 
> +    if (isFromASTFile())
> +      return *((const unsigned*)this - 1);
> +    return 0;
> +  }
> +              
>   unsigned getIdentifierNamespace() const {
>     return IdentifierNamespace;
>   }
> 
> Modified: cfe/trunk/include/clang/Serialization/ASTReader.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTReader.h?rev=147617&r1=147616&r2=147617&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Serialization/ASTReader.h (original)
> +++ cfe/trunk/include/clang/Serialization/ASTReader.h Thu Jan  5 16:27:05 2012
> @@ -675,13 +675,6 @@
>   /// \brief Keeps track of the elements added to PendingDeclChains.
>   llvm::SmallSet<serialization::DeclID, 16> PendingDeclChainsKnown;
> 
> -  /// \brief Reverse mapping from declarations to their global declaration IDs.
> -  /// 
> -  /// FIXME: This data structure is currently only used for ObjCInterfaceDecls
> -  /// and ObjCProtocolDecls to support declaration merging. If we must have 
> -  /// this for other declarations, allocate it along with the Decl itself.
> -  llvm::DenseMap<Decl *, serialization::GlobalDeclID> DeclToID;
> -  
>   typedef llvm::DenseMap<Decl *, llvm::SmallVector<serialization::DeclID, 2> >
>     MergedDeclsMap;
> 
> 
> Modified: cfe/trunk/lib/AST/DeclBase.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclBase.cpp?rev=147617&r1=147616&r2=147617&view=diff
> ==============================================================================
> --- cfe/trunk/lib/AST/DeclBase.cpp (original)
> +++ cfe/trunk/lib/AST/DeclBase.cpp Thu Jan  5 16:27:05 2012
> @@ -44,7 +44,20 @@
> void *Decl::AllocateDeserializedDecl(const ASTContext &Context, 
>                                      unsigned ID,
>                                      unsigned Size) {
> -  return Context.Allocate(Size);
> +  // Allocate an extra pointer's worth of storage, which ensures that 
> +  //   (1) We have enough storage to stash the global declaration ID, and
> +  //   (2) We maintain pointer alignment.
> +  //
> +  // Note that this wastes 4 bytes on x86-64, which we'll undoubtedly end up
> +  // finding a use for later.
> +  void *Start = Context.Allocate(Size + sizeof(void*));
> +  void *Result = (char*)Start + sizeof(void*);

Looks like we don't get around wasting 4 bytes on i386 too, some code assumes that the pointers are 8-byte aligned. Here's a test log from a x86 box:

FAIL: Clang :: PCH/chain-implicit-definition.cpp (2375 of 9811)
******************** TEST 'Clang :: PCH/chain-implicit-definition.cpp' FAILED ********************
Script:
--
/var/tmp/llvm/Release+Asserts/bin/clang -cc1 -internal-isystem /var/tmp/llvm/Release+Asserts/bin/../lib/clang/3.1/include -emit-llvm-only -include /var/tmp/llvm/tools/clang/test/PCH/chain-implicit-definition.cpp -include /var/tmp/llvm/tools/clang/test/PCH/chain-implicit-definition.cpp /var/tmp/llvm/tools/clang/test/PCH/chain-implicit-definition.cpp
/var/tmp/llvm/Release+Asserts/bin/clang -cc1 -internal-isystem /var/tmp/llvm/Release+Asserts/bin/../lib/clang/3.1/include -emit-llvm-only -chain-include /var/tmp/llvm/tools/clang/test/PCH/chain-implicit-definition.cpp -chain-include /var/tmp/llvm/tools/clang/test/PCH/chain-implicit-definition.cpp /var/tmp/llvm/tools/clang/test/PCH/chain-implicit-definition.cpp
--
Exit Code: 134
Command Output (stderr):
--
clang: /var/tmp/llvm/tools/clang/lib/AST/../../include/clang/AST/VTableBuilder.h:163: clang::VTableComponent::VTableComponent(clang::VTableComponent::Kind, uintptr_t): Assertion `(Ptr & 7) == 0 && "Pointer not sufficiently aligned!"' failed.
0  clang 0x099ce08b
Stack dump:
0.      Program arguments: /var/tmp/llvm/Release+Asserts/bin/clang -cc1 -internal-isystem /var/tmp/llvm/Release+Asserts/bin/../lib/clang/3.1/include -emit-llvm-only -chain-include /var/tmp/llvm/tools/clang/test/PCH/chain-implicit-definition.cpp -chain-include /var/tmp/llvm/tools/clang/test/PCH/chain-implicit-definition.cpp /var/tmp/llvm/tools/clang/test/PCH/chain-implicit-definition.cpp 
1.      <eof> parser at end of file
2.      Per-file LLVM IR generation
3.      /var/tmp/llvm/tools/clang/test/PCH/chain-implicit-definition.cpp:26:3: Generating code for declaration 'C::~C'
/var/tmp/llvm/tools/clang/test/PCH/Output/chain-implicit-definition.cpp.script: line 2:  1718 Aborted                 /var/tmp/llvm/Release+Asserts/bin/clang -cc1 -internal-isystem /var/tmp/llvm/Release+Asserts/bin/../lib/clang/3.1/include -emit-llvm-only -chain-include /var/tmp/llvm/tools/clang/test/PCH/chain-implicit-definition.cpp -chain-include /var/tmp/llvm/tools/clang/test/PCH/chain-implicit-definition.cpp /var/tmp/llvm/tools/clang/test/PCH/chain-implicit-definition.cpp


> +  
> +  // Store the global declaration ID 
> +  unsigned *IDPtr = (unsigned*)Result - 1;
> +  *IDPtr = ID;
> +  
> +  return Result;
> }
> 
> const char *Decl::getDeclKindName() const {
> 
> Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=147617&r1=147616&r2=147617&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)
> +++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Thu Jan  5 16:27:05 2012
> @@ -402,9 +402,6 @@
> }
> 
> void ASTDeclReader::VisitTypedefNameDecl(TypedefNameDecl *TD) {
> -  // Record the declaration -> global ID mapping.
> -  Reader.DeclToID[TD] = ThisDeclID;
> -  
>   RedeclarableResult Redecl = VisitRedeclarable(TD);
>   VisitTypeDecl(TD);
> 
> @@ -421,9 +418,6 @@
> }
> 
> void ASTDeclReader::VisitTagDecl(TagDecl *TD) {
> -  // Record the declaration -> global ID mapping.
> -  Reader.DeclToID[TD] = ThisDeclID;
> -    
>   RedeclarableResult Redecl = VisitRedeclarable(TD);
>   VisitTypeDecl(TD);
> 
> @@ -490,9 +484,6 @@
> }
> 
> void ASTDeclReader::VisitFunctionDecl(FunctionDecl *FD) {
> -  // Record the declaration -> global ID mapping.
> -  Reader.DeclToID[FD] = ThisDeclID;
> -  
>   RedeclarableResult Redecl = VisitRedeclarable(FD);
>   VisitDeclaratorDecl(FD);
> 
> @@ -673,9 +664,6 @@
> }
> 
> void ASTDeclReader::VisitObjCInterfaceDecl(ObjCInterfaceDecl *ID) {
> -  // Record the declaration -> global ID mapping.
> -  Reader.DeclToID[ID] = ThisDeclID;
> -  
>   RedeclarableResult Redecl = VisitRedeclarable(ID);
>   VisitObjCContainerDecl(ID);
>   TypeIDForTypeDecl = Reader.getGlobalTypeID(F, Record[Idx++]);
> @@ -746,9 +734,6 @@
> }
> 
> void ASTDeclReader::VisitObjCProtocolDecl(ObjCProtocolDecl *PD) {
> -  // Record the declaration -> global ID mapping.
> -  Reader.DeclToID[PD] = ThisDeclID;
> -  
>   RedeclarableResult Redecl = VisitRedeclarable(PD);
>   VisitObjCContainerDecl(PD);
>   mergeRedeclarable(PD, Redecl);
> @@ -880,9 +865,6 @@
> }
> 
> void ASTDeclReader::VisitVarDecl(VarDecl *VD) {
> -  // Record the declaration -> global ID mapping.
> -  Reader.DeclToID[VD] = ThisDeclID;
> -  
>   RedeclarableResult Redecl = VisitRedeclarable(VD);
>   VisitDeclaratorDecl(VD);
> 
> @@ -1561,7 +1543,7 @@
>         // Introduce ExistingCanon into the set of pending declaration chains,
>         // if in fact it came from a module file.
>         if (ExistingCanon->isFromASTFile()) {
> -          GlobalDeclID ExistingCanonID = Reader.DeclToID[ExistingCanon];
> +          GlobalDeclID ExistingCanonID = ExistingCanon->getGlobalID();
>           assert(ExistingCanonID && "Unrecorded canonical declaration ID?");
>           if (Reader.PendingDeclChainsKnown.insert(ExistingCanonID))
>             Reader.PendingDeclChains.push_back(ExistingCanonID);
> 
> Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=147617&r1=147616&r2=147617&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)
> +++ cfe/trunk/lib/Serialization/ASTWriter.cpp Thu Jan  5 16:27:05 2012
> @@ -2914,7 +2914,7 @@
>   for (ASTReader::MergedDeclsMap::iterator I = Chain->MergedDecls.begin(),
>                                         IEnd = Chain->MergedDecls.end();
>        I != IEnd; ++I) {
> -    DeclID CanonID = I->first->isFromASTFile()? Chain->DeclToID[I->first]
> +    DeclID CanonID = I->first->isFromASTFile()? I->first->getGlobalID()
>                                               : getDeclID(I->first);
>     assert(CanonID && "Merged declaration not known?");
> 
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits





More information about the cfe-commits mailing list