[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

[Douglas Gregor]

On Jan 5, 2012, at 3:47 PM, Benjamin Kramer wrote:

> 
> 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.
>> [snip]
>> 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

I was convinced that we didn't use that third bit. Fixed in r147626, thanks!

	- Doug