On May 12, 2011, at 9:07 PM, Chris Lattner wrote:
>
> On May 12, 2011, at 6:05 PM, Douglas Gregor wrote:
>
>> Author: dgregor
>> Date: Thu May 12 20:05:07 2011
>> New Revision: 131276
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=131276&view=rev
>> Log:
>> When determining whether we can make a declaration into a global
>> constant, also consider whether it's a class type that has any mutable
>> fields. If so, it can't be a global constant.
>
> Are bits like this really worth putting into the PCH file? Why not just
> lazily compute them?
Lazily computing them would involve deserializing all of the fields within the
class and all of its subobjects of class type. That may be fine---we could due
so once and compute all of the properties based on those fields at that
time---but it's a shame to require extra deserialization to save a bit.
These bits in CXXRecordDecl::DefinitionData *do* take up way too much space in
the PCH, but the fix should be to make each logical bit take just one bit
rather than the current 6 bits (due to the VBR-6 encoding). A simple change
there could save us a bunch of I/O.
- Doug
> -Chris
>
>>
>> Modified:
>> cfe/trunk/include/clang/AST/DeclCXX.h
>> cfe/trunk/lib/AST/DeclCXX.cpp
>> cfe/trunk/lib/CodeGen/CGBlocks.cpp
>> cfe/trunk/lib/CodeGen/CodeGenModule.cpp
>> cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
>> cfe/trunk/lib/Serialization/ASTWriter.cpp
>> cfe/trunk/test/CodeGenCXX/global-llvm-constant.cpp
>>
>> Modified: cfe/trunk/include/clang/AST/DeclCXX.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=131276&r1=131275&r2=131276&view=diff
>> ==============================================================================
>> --- cfe/trunk/include/clang/AST/DeclCXX.h (original)
>> +++ cfe/trunk/include/clang/AST/DeclCXX.h Thu May 12 20:05:07 2011
>> @@ -337,6 +337,9 @@
>> /// HasPublicFields - True when there are private non-static data members.
>> bool HasPublicFields : 1;
>>
>> + /// \brief True if this class (or any subobject) has mutable fields.
>> + bool HasMutableFields : 1;
>> +
>> /// HasTrivialDefaultConstructor - True when, if this class has a default
>> /// constructor, this default constructor is trivial.
>> ///
>> @@ -822,6 +825,10 @@
>> /// (C++ [class]p7)
>> bool isStandardLayout() const { return data().IsStandardLayout; }
>>
>> + /// \brief Whether this class, or any of its class subobjects, contains a
>> + /// mutable field.
>> + bool hasMutableFields() const { return data().HasMutableFields; }
>> +
>> // hasTrivialDefaultConstructor - Whether this class has a trivial default
>> // constructor
>> // (C++0x [class.ctor]p5)
>>
>> Modified: cfe/trunk/lib/AST/DeclCXX.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclCXX.cpp?rev=131276&r1=131275&r2=131276&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/AST/DeclCXX.cpp (original)
>> +++ cfe/trunk/lib/AST/DeclCXX.cpp Thu May 12 20:05:07 2011
>> @@ -33,7 +33,7 @@
>> Aggregate(true), PlainOldData(true), Empty(true), Polymorphic(false),
>> Abstract(false), IsStandardLayout(true), HasNoNonEmptyBases(true),
>> HasPrivateFields(false), HasProtectedFields(false),
>> HasPublicFields(false),
>> - HasTrivialDefaultConstructor(true),
>> + HasMutableFields(false), HasTrivialDefaultConstructor(true),
>> HasConstExprNonCopyMoveConstructor(false),
>> HasTrivialCopyConstructor(true),
>> HasTrivialMoveConstructor(true), HasTrivialCopyAssignment(true),
>> HasTrivialMoveAssignment(true), HasTrivialDestructor(true),
>> @@ -225,6 +225,10 @@
>> // have trivial destructors.
>> if (!BaseClassDecl->hasTrivialDestructor())
>> data().HasTrivialDestructor = false;
>> +
>> + // Keep track of the presence of mutable fields.
>> + if (BaseClassDecl->hasMutableFields())
>> + data().HasMutableFields = true;
>> }
>>
>> if (VBases.empty())
>> @@ -688,6 +692,10 @@
>> data().HasPublicFields) > 1)
>> data().IsStandardLayout = false;
>>
>> + // Keep track of the presence of mutable fields.
>> + if (Field->isMutable())
>> + data().HasMutableFields = true;
>> +
>> // C++0x [class]p9:
>> // A POD struct is a class that is both a trivial class and a
>> // standard-layout class, and has no non-static data members of type
>> @@ -779,6 +787,10 @@
>> }
>> }
>> }
>> +
>> + // Keep track of the presence of mutable fields.
>> + if (FieldRec->hasMutableFields())
>> + data().HasMutableFields = true;
>> }
>> }
>>
>>
>> Modified: cfe/trunk/lib/CodeGen/CGBlocks.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBlocks.cpp?rev=131276&r1=131275&r2=131276&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/CodeGen/CGBlocks.cpp (original)
>> +++ cfe/trunk/lib/CodeGen/CGBlocks.cpp Thu May 12 20:05:07 2011
>> @@ -189,23 +189,6 @@
>> }
>> }
>>
>> -/// Determines if the given record type has a mutable field.
>> -static bool hasMutableField(const CXXRecordDecl *record) {
>> - for (CXXRecordDecl::field_iterator
>> - i = record->field_begin(), e = record->field_end(); i != e; ++i)
>> - if ((*i)->isMutable())
>> - return true;
>> -
>> - for (CXXRecordDecl::base_class_const_iterator
>> - i = record->bases_begin(), e = record->bases_end(); i != e; ++i) {
>> - const RecordType *record = i->getType()->castAs<RecordType>();
>> - if (hasMutableField(cast<CXXRecordDecl>(record->getDecl())))
>> - return true;
>> - }
>> -
>> - return false;
>> -}
>> -
>> /// Determines if the given type is safe for constant capture in C++.
>> static bool isSafeForCXXConstantCapture(QualType type) {
>> const RecordType *recordType =
>> @@ -222,7 +205,7 @@
>>
>> // Otherwise, we just have to make sure there aren't any mutable
>> // fields that might have changed since initialization.
>> - return !hasMutableField(record);
>> + return !record->hasMutableFields();
>> }
>>
>> /// It is illegal to modify a const object after initialization.
>>
>> Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=131276&r1=131275&r2=131276&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
>> +++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Thu May 12 20:05:07 2011
>> @@ -946,7 +946,9 @@
>> if (Context.getLangOptions().CPlusPlus) {
>> if (const RecordType *Record
>> = Context.getBaseElementType(D->getType())->getAs<RecordType>())
>> - return ConstantInit &&
>> cast<CXXRecordDecl>(Record->getDecl())->isPOD();
>> + return ConstantInit &&
>> + cast<CXXRecordDecl>(Record->getDecl())->isPOD() &&
>> + !cast<CXXRecordDecl>(Record->getDecl())->hasMutableFields();
>> }
>>
>> return true;
>>
>> Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=131276&r1=131275&r2=131276&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)
>> +++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Thu May 12 20:05:07 2011
>> @@ -860,6 +860,7 @@
>> Data.HasPrivateFields = Record[Idx++];
>> Data.HasProtectedFields = Record[Idx++];
>> Data.HasPublicFields = Record[Idx++];
>> + Data.HasMutableFields = Record[Idx++];
>> Data.HasTrivialDefaultConstructor = Record[Idx++];
>> Data.HasConstExprNonCopyMoveConstructor = Record[Idx++];
>> Data.HasTrivialCopyConstructor = Record[Idx++];
>>
>> Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=131276&r1=131275&r2=131276&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)
>> +++ cfe/trunk/lib/Serialization/ASTWriter.cpp Thu May 12 20:05:07 2011
>> @@ -3818,6 +3818,7 @@
>> Record.push_back(Data.HasPrivateFields);
>> Record.push_back(Data.HasProtectedFields);
>> Record.push_back(Data.HasPublicFields);
>> + Record.push_back(Data.HasMutableFields);
>> Record.push_back(Data.HasTrivialDefaultConstructor);
>> Record.push_back(Data.HasConstExprNonCopyMoveConstructor);
>> Record.push_back(Data.HasTrivialCopyConstructor);
>>
>> Modified: cfe/trunk/test/CodeGenCXX/global-llvm-constant.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/global-llvm-constant.cpp?rev=131276&r1=131275&r2=131276&view=diff
>> ==============================================================================
>> --- cfe/trunk/test/CodeGenCXX/global-llvm-constant.cpp (original)
>> +++ cfe/trunk/test/CodeGenCXX/global-llvm-constant.cpp Thu May 12 20:05:07
>> 2011
>> @@ -18,3 +18,15 @@
>> // CHECK: @x2 = constant
>> extern const X x2;
>> const X x2 = { &add };
>> +
>> +struct X1 {
>> + mutable int i;
>> +};
>> +
>> +struct X2 {
>> + X1 array[3];
>> +};
>> +
>> +// CHECK: @x2b = global
>> +extern const X2 x2b;
>> +const X2 x2b = { { { 1 }, { 2 }, { 3 } } };
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> [email protected]
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
