[cfe-commits] r131276 - in /cfe/trunk: include/clang/AST/DeclCXX.h lib/AST/DeclCXX.cpp lib/CodeGen/CGBlocks.cpp lib/CodeGen/CodeGenModule.cpp lib/Serialization/ASTReaderDecl.cpp lib/Serialization/ASTWriter.cpp test/CodeGenCXX/global-llvm-constant.cpp

Douglas Gregor dgregor at apple.com
Fri May 13 08:08:23 PDT 2011


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
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> 




More information about the cfe-commits mailing list