[cfe-commits] r125605 - /cfe/trunk/lib/CodeGen/CGRecordLayoutBuilder.cpp

[Ken Dyck]

On Tue, Feb 15, 2011 at 5:21 PM, John McCall <rjmccall at apple.com> wrote:
> @@ -310,47 +309,46 @@
>  }
>
>  void CGRecordLayoutBuilder::LayoutBitField(const FieldDecl *D,
> -                                           uint64_t FieldOffset) {
> -  uint64_t FieldSize =
> +                                           uint64_t fieldOffset) {
> +  uint64_t fieldSize =
>     D->getBitWidth()->EvaluateAsInt(Types.getContext()).getZExtValue();
>
> -  if (FieldSize == 0)
> +  if (fieldSize == 0)
>     return;
>
> -  uint64_t NextFieldOffset = NextFieldOffsetInBytes * 8;
> -  unsigned NumBytesToAppend;
> +  uint64_t nextFieldOffsetInBits = NextFieldOffset.getQuantity() * 8;

You could use Types.getContext().toBits(NextFieldOffset) here to
eliminate the literal 8.

> +  unsigned numBytesToAppend;
>
> -  if (FieldOffset < NextFieldOffset) {
> +  if (fieldOffset < nextFieldOffsetInBits) {
>     assert(BitsAvailableInLastField && "Bitfield size mismatch!");
> -    assert(NextFieldOffsetInBytes && "Must have laid out at least one byte!");
> +    assert(!NextFieldOffset.isZero() && "Must have laid out at least one byte");
>
>     // The bitfield begins in the previous bit-field.
> -    NumBytesToAppend =
> -      llvm::RoundUpToAlignment(FieldSize - BitsAvailableInLastField, 8) / 8;
> +    numBytesToAppend =
> +      llvm::RoundUpToAlignment(fieldSize - BitsAvailableInLastField, 8) / 8;
>   } else {
> -    assert(FieldOffset % 8 == 0 && "Field offset not aligned correctly");
> +    assert(fieldOffset % 8 == 0 && "Field offset not aligned correctly");
>
>     // Append padding if necessary.
> -    AppendPadding(FieldOffset / 8, 1);
> +    AppendPadding(CharUnits::fromQuantity(fieldOffset / 8), CharUnits::One());

Replacing with Types.getContext().toCharUnitsFromBits(fieldOffset)
would eliminate the literal 8.
>
> -    NumBytesToAppend =
> -      llvm::RoundUpToAlignment(FieldSize, 8) / 8;
> +    numBytesToAppend = llvm::RoundUpToAlignment(fieldSize, 8) / 8;
>
> -    assert(NumBytesToAppend && "No bytes to append!");
> +    assert(numBytesToAppend && "No bytes to append!");
>   }
>
>   // Add the bit field info.
>   BitFields.insert(std::make_pair(D,
> -                   CGBitFieldInfo::MakeInfo(Types, D, FieldOffset, FieldSize)));
> +                   CGBitFieldInfo::MakeInfo(Types, D, fieldOffset, fieldSize)));
>
> -  AppendBytes(NumBytesToAppend);
> +  AppendBytes(CharUnits::fromQuantity(numBytesToAppend));
>
>   BitsAvailableInLastField =
> -    NextFieldOffsetInBytes * 8 - (FieldOffset + FieldSize);
> +    NextFieldOffset.getQuantity() * 8 - (fieldOffset + fieldSize);

Types.getContext().toBits(NextFieldOffset)?



> @@ -439,78 +439,97 @@
>  void CGRecordLayoutBuilder::LayoutUnion(const RecordDecl *D) {
>   assert(D->isUnion() && "Can't call LayoutUnion on a non-union record!");
>
> -  const ASTRecordLayout &Layout = Types.getContext().getASTRecordLayout(D);
> -
> -  const llvm::Type *Ty = 0;
> -  uint64_t Size = 0;
> -  unsigned Align = 0;
> -
> -  bool HasOnlyZeroSizedBitFields = true;
> +  const ASTRecordLayout &layout = Types.getContext().getASTRecordLayout(D);
>
> -  unsigned FieldNo = 0;
> -  for (RecordDecl::field_iterator Field = D->field_begin(),
> -       FieldEnd = D->field_end(); Field != FieldEnd; ++Field, ++FieldNo) {
> -    assert(Layout.getFieldOffset(FieldNo) == 0 &&
> +  const llvm::Type *unionType = 0;
> +  CharUnits unionSize = CharUnits::Zero();
> +  CharUnits unionAlign = CharUnits::Zero();
> +
> +  bool hasOnlyZeroSizedBitFields = true;
> +
> +  unsigned fieldNo = 0;
> +  for (RecordDecl::field_iterator field = D->field_begin(),
> +       fieldEnd = D->field_end(); field != fieldEnd; ++field, ++fieldNo) {
> +    assert(layout.getFieldOffset(fieldNo) == 0 &&
>           "Union field offset did not start at the beginning of record!");
> -    const llvm::Type *FieldTy = LayoutUnionField(*Field, Layout);
> +    const llvm::Type *fieldType = LayoutUnionField(*field, layout);
>
> -    if (!FieldTy)
> +    if (!fieldType)
>       continue;
>
> -    HasOnlyZeroSizedBitFields = false;
> +    hasOnlyZeroSizedBitFields = false;
>
> -    unsigned FieldAlign = Types.getTargetData().getABITypeAlignment(FieldTy);
> -    uint64_t FieldSize = Types.getTargetData().getTypeAllocSize(FieldTy);
> +    CharUnits fieldAlign = CharUnits::fromQuantity(
> +                          Types.getTargetData().getABITypeAlignment(fieldType));
> +    CharUnits fieldSize = CharUnits::fromQuantity(
> +                             Types.getTargetData().getTypeAllocSize(fieldType));
>
> -    if (FieldAlign < Align)
> +    if (fieldAlign < unionAlign)
>       continue;
>
> -    if (FieldAlign > Align || FieldSize > Size) {
> -      Ty = FieldTy;
> -      Align = FieldAlign;
> -      Size = FieldSize;
> +    if (fieldAlign > unionAlign || fieldSize > unionSize) {
> +      unionType = fieldType;
> +      unionAlign = fieldAlign;
> +      unionSize = fieldSize;
>     }
>   }
>
>   // Now add our field.
> -  if (Ty) {
> -    AppendField(0, Ty);
> +  if (unionType) {
> +    AppendField(CharUnits::Zero(), unionType);
>
> -    if (getTypeAlignment(Ty) > Layout.getAlignment().getQuantity()) {
> +    if (getTypeAlignment(unionType) > layout.getAlignment()) {
>       // We need a packed struct.
>       Packed = true;
> -      Align = 1;
> +      unionAlign = CharUnits::One();
>     }
>   }
> -  if (!Align) {
> -    assert(HasOnlyZeroSizedBitFields &&
> +  if (unionAlign.isZero()) {
> +    assert(hasOnlyZeroSizedBitFields &&
>            "0-align record did not have all zero-sized bit-fields!");
> -    Align = 1;
> +    unionAlign = CharUnits::One();
>   }
>
>   // Append tail padding.
> -  uint64_t RecordSize = Layout.getSize().getQuantity();
> -  if (RecordSize > Size)
> -    AppendPadding(RecordSize, Align);
> +  CharUnits recordSize = layout.getSize();
> +  if (recordSize > unionSize)
> +    AppendPadding(recordSize, unionAlign);
>  }
>
>  void CGRecordLayoutBuilder::LayoutBase(const CXXRecordDecl *base,
> -                                       uint64_t baseOffsetInBits) {
> -  uint64_t baseOffsetInBytes = baseOffsetInBits / 8;
> -  AppendPadding(baseOffsetInBytes, 1);
> +                                       const CGRecordLayout &baseLayout,
> +                                       CharUnits baseOffset) {
> +  AppendPadding(baseOffset, CharUnits::One());
>
>   const ASTRecordLayout &baseASTLayout
>     = Types.getContext().getASTRecordLayout(base);
>
> -  // FIXME: use a better type than [sizeof(base) x i8].
> -  // We could use the base layout's subobject type as the actualy
> -  // subobject type in the layout if its size is the nvsize of the
> -  // base, or if we'd need padding out to the enclosing object anyhow.
> -  AppendBytes(baseASTLayout.getNonVirtualSize());
> +  // Fields and bases can be laid out in the tail padding of previous
> +  // bases.  If this happens, we need to allocate the base as an i8
> +  // array; otherwise, we can use the subobject type.  However,
> +  // actually doing that would require knowledge of what immediately
> +  // follows this base in the layout, so instead we do a conservative
> +  // approximation, which is to use the base subobject type if it
> +  // has the same LLVM storage size as the nvsize.
> +
> +  // The nvsize, i.e. the unpadded size of the base class.
> +  CharUnits nvsize = baseASTLayout.getNonVirtualSize();
> +
> +#if 0
> +  const llvm::StructType *subobjectType = baseLayout.getBaseSubobjectLLVMType();
> +  const llvm::StructLayout *baseLLVMLayout =
> +    Types.getTargetData().getStructLayout(subobjectType);
> +  CharUnits stsize = CharUnits::fromQuantity(baseLLVMLayout->getSizeInBytes());
> +
> +  if (nvsize == stsize)
> +    AppendField(baseOffset, subobjectType);
> +  else
> +#endif

Did you mean to keep this dead block for later?

> @@ -697,84 +715,84 @@
>  void CGRecordLayoutBuilder::AppendTailPadding(uint64_t RecordSize) {
>   assert(RecordSize % 8 == 0 && "Invalid record size!");
>
> -  uint64_t RecordSizeInBytes = RecordSize / 8;
> -  assert(NextFieldOffsetInBytes <= RecordSizeInBytes && "Size mismatch!");
> +  CharUnits RecordSizeInBytes = CharUnits::fromQuantity(RecordSize / 8);

Context.toCharUnitsFromBits(RecordSize)?

-Ken