Rework the bitfield access IR generation to address PR13619 and

generally support the C++11 memory model requirements for bitfield
accesses by relying more heavily on LLVM's memory model.

The primary change this introduces is to move from a manually aligned
and strided access pattern across the bits of the bitfield to a much
simpler lump access of all bits in the bitfield followed by math to
extract the bits relevant for the particular field.

This simplifies the code significantly, but relies on LLVM to
intelligently lowering these integers.

I have tested LLVM's lowering both synthetically and in benchmarks. The
lowering appears to be functional, and there are no really significant
performance regressions. Different code patterns accessing bitfields
will vary in how this impacts them. The only real regressions I'm seeing
are a few patterns where the LLVM code generation for loads that feed
directly into a mask operation don't take advantage of the x86 ability
to do a smaller load and a cheap zero-extension. This doesn't regress
any benchmark in the nightly test suite on my box past the noise
threshold, but my box is quite noisy. I'll be watching the LNT numbers,
and will look into further improvements to the LLVM lowering as needed.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@169489 91177308-0d34-0410-b5e6-96231b3b80d8

5 files changed, 351 insertions, 518 deletions

diff --git a/lib/CodeGen/CGExpr.cpp b/lib/CodeGen/CGExpr.cpp
index f0ee2343d4..94bc81c8f4 100644
--- a/lib/CodeGen/CGExpr.cpp
+++ b/lib/CodeGen/CGExpr.cpp
@@ -1155,72 +1155,30 @@ RValue CodeGenFunction::EmitLoadOfBitfieldLValue(LValue LV) {
 
   // Get the output type.
   llvm::Type *ResLTy = ConvertType(LV.getType());
-  unsigned ResSizeInBits = CGM.getDataLayout().getTypeSizeInBits(ResLTy);
-
-  // Compute the result as an OR of all of the individual component accesses.
-  llvm::Value *Res = 0;
-  for (unsigned i = 0, e = Info.getNumComponents(); i != e; ++i) {
-    const CGBitFieldInfo::AccessInfo &AI = Info.getComponent(i);
-    CharUnits AccessAlignment = AI.AccessAlignment;
-    if (!LV.getAlignment().isZero())
-      AccessAlignment = std::min(AccessAlignment, LV.getAlignment());
-
-    // Get the field pointer.
-    llvm::Value *Ptr = LV.getBitFieldBaseAddr();
-
-    // Only offset by the field index if used, so that incoming values are not
-    // required to be structures.
-    if (AI.FieldIndex)
-      Ptr = Builder.CreateStructGEP(Ptr, AI.FieldIndex, "bf.field");
-
-    // Offset by the byte offset, if used.
-    if (!AI.FieldByteOffset.isZero()) {
-      Ptr = EmitCastToVoidPtr(Ptr);
-      Ptr = Builder.CreateConstGEP1_32(Ptr, AI.FieldByteOffset.getQuantity(),
-                                       "bf.field.offs");
-    }
-
-    // Cast to the access type.
-    llvm::Type *PTy = llvm::Type::getIntNPtrTy(getLLVMContext(), AI.AccessWidth,
-                       CGM.getContext().getTargetAddressSpace(LV.getType()));
-    Ptr = Builder.CreateBitCast(Ptr, PTy);
-
-    // Perform the load.
-    llvm::LoadInst *Load = Builder.CreateLoad(Ptr, LV.isVolatileQualified());
-    Load->setAlignment(AccessAlignment.getQuantity());
-
-    // Shift out unused low bits and mask out unused high bits.
-    llvm::Value *Val = Load;
-    if (AI.FieldBitStart)
-      Val = Builder.CreateLShr(Load, AI.FieldBitStart);
-    Val = Builder.CreateAnd(Val, llvm::APInt::getLowBitsSet(AI.AccessWidth,
-                                                            AI.TargetBitWidth),
-                            "bf.clear");
-
-    // Extend or truncate to the target size.
-    if (AI.AccessWidth < ResSizeInBits)
-      Val = Builder.CreateZExt(Val, ResLTy);
-    else if (AI.AccessWidth > ResSizeInBits)
-      Val = Builder.CreateTrunc(Val, ResLTy);
 
-    // Shift into place, and OR into the result.
-    if (AI.TargetBitOffset)
-      Val = Builder.CreateShl(Val, AI.TargetBitOffset);
-    Res = Res ? Builder.CreateOr(Res, Val) : Val;
-  }
-
-  // If the bit-field is signed, perform the sign-extension.
-  //
-  // FIXME: This can easily be folded into the load of the high bits, which
-  // could also eliminate the mask of high bits in some situations.
-  if (Info.isSigned()) {
-    unsigned ExtraBits = ResSizeInBits - Info.getSize();
-    if (ExtraBits)
-      Res = Builder.CreateAShr(Builder.CreateShl(Res, ExtraBits),
-                               ExtraBits, "bf.val.sext");
+  llvm::Value *Ptr = LV.getBitFieldAddr();
+  llvm::Value *Val = Builder.CreateLoad(Ptr, LV.isVolatileQualified(),
+                                        "bf.load");
+  cast<llvm::LoadInst>(Val)->setAlignment(Info.StorageAlignment);
+
+  if (Info.IsSigned) {
+    assert((Info.Offset + Info.Size) <= Info.StorageSize);
+    unsigned HighBits = Info.StorageSize - Info.Offset - Info.Size;
+    if (HighBits)
+      Val = Builder.CreateShl(Val, HighBits, "bf.shl");
+    if (Info.Offset + HighBits)
+      Val = Builder.CreateAShr(Val, Info.Offset + HighBits, "bf.ashr");
+  } else {
+    if (Info.Offset)
+      Val = Builder.CreateLShr(Val, Info.Offset, "bf.lshr");
+    if (Info.Offset + Info.Size < Info.StorageSize)
+      Val = Builder.CreateAnd(Val, llvm::APInt::getLowBitsSet(Info.StorageSize,
+                                                              Info.Size),
+                              "bf.clear");
   }
+  Val = Builder.CreateIntCast(Val, ResLTy, Info.IsSigned, "bf.cast");
 
-  return RValue::get(Res);
+  return RValue::get(Val);
 }
 
 // If this is a reference to a subset of the elements of a vector, create an
@@ -1350,106 +1308,71 @@ void CodeGenFunction::EmitStoreThroughLValue(RValue Src, LValue Dst, bool isInit
 void CodeGenFunction::EmitStoreThroughBitfieldLValue(RValue Src, LValue Dst,
                                                      llvm::Value **Result) {
   const CGBitFieldInfo &Info = Dst.getBitFieldInfo();
-
-  // Get the output type.
   llvm::Type *ResLTy = ConvertTypeForMem(Dst.getType());
-  unsigned ResSizeInBits = CGM.getDataLayout().getTypeSizeInBits(ResLTy);
+  llvm::Value *Ptr = Dst.getBitFieldAddr();
 
   // Get the source value, truncated to the width of the bit-field.
   llvm::Value *SrcVal = Src.getScalarVal();
 
-  if (hasBooleanRepresentation(Dst.getType()))
-    SrcVal = Builder.CreateIntCast(SrcVal, ResLTy, /*IsSigned=*/false);
-
-  SrcVal = Builder.CreateAnd(SrcVal, llvm::APInt::getLowBitsSet(ResSizeInBits,
-                                                                Info.getSize()),
-                             "bf.value");
-
-  // Return the new value of the bit-field, if requested.
-  if (Result) {
-    // Cast back to the proper type for result.
-    llvm::Type *SrcTy = Src.getScalarVal()->getType();
-    llvm::Value *ReloadVal = Builder.CreateIntCast(SrcVal, SrcTy, false,
-                                                   "bf.reload.val");
-
-    // Sign extend if necessary.
-    if (Info.isSigned()) {
-      unsigned ExtraBits = ResSizeInBits - Info.getSize();
-      if (ExtraBits)
-        ReloadVal = Builder.CreateAShr(Builder.CreateShl(ReloadVal, ExtraBits),
-                                       ExtraBits, "bf.reload.sext");
-    }
+  // Cast the source to the storage type and shift it into place.
+  SrcVal = Builder.CreateIntCast(SrcVal,
+                                 Ptr->getType()->getPointerElementType(),
+                                 /*IsSigned=*/false);
+  llvm::Value *MaskedVal = SrcVal;
+
+  // See if there are other bits in the bitfield's storage we'll need to load
+  // and mask together with source before storing.
+  if (Info.StorageSize != Info.Size) {
+    assert(Info.StorageSize > Info.Size && "Invalid bitfield size.");
+    llvm::Value *Val = Builder.CreateLoad(Ptr, Dst.isVolatileQualified(),
+                                          "bf.load");
+    cast<llvm::LoadInst>(Val)->setAlignment(Info.StorageAlignment);
+
+    // Mask the source value as needed.
+    if (!hasBooleanRepresentation(Dst.getType()))
+      SrcVal = Builder.CreateAnd(SrcVal,
+                                 llvm::APInt::getLowBitsSet(Info.StorageSize,
+                                                            Info.Size),
+                                 "bf.value");
+    MaskedVal = SrcVal;
+    if (Info.Offset)
+      SrcVal = Builder.CreateShl(SrcVal, Info.Offset, "bf.shl");
+
+    // Mask out the original value.
+    Val = Builder.CreateAnd(Val,
+                            ~llvm::APInt::getBitsSet(Info.StorageSize,
+                                                     Info.Offset,
+                                                     Info.Offset + Info.Size),
+                            "bf.clear");
 
-    *Result = ReloadVal;
+    // Or together the unchanged values and the source value.
+    SrcVal = Builder.CreateOr(Val, SrcVal, "bf.set");
+  } else {
+    assert(Info.Offset == 0);
   }
 
-  // Iterate over the components, writing each piece to memory.
-  for (unsigned i = 0, e = Info.getNumComponents(); i != e; ++i) {
-    const CGBitFieldInfo::AccessInfo &AI = Info.getComponent(i);
-    CharUnits AccessAlignment = AI.AccessAlignment;
-    if (!Dst.getAlignment().isZero())
-      AccessAlignment = std::min(AccessAlignment, Dst.getAlignment());
-
-    // Get the field pointer.
-    llvm::Value *Ptr = Dst.getBitFieldBaseAddr();
-    unsigned addressSpace =
-      cast<llvm::PointerType>(Ptr->getType())->getAddressSpace();
-
-    // Only offset by the field index if used, so that incoming values are not
-    // required to be structures.
-    if (AI.FieldIndex)
-      Ptr = Builder.CreateStructGEP(Ptr, AI.FieldIndex, "bf.field");
-
-    // Offset by the byte offset, if used.
-    if (!AI.FieldByteOffset.isZero()) {
-      Ptr = EmitCastToVoidPtr(Ptr);
-      Ptr = Builder.CreateConstGEP1_32(Ptr, AI.FieldByteOffset.getQuantity(),
-                                       "bf.field.offs");
-    }
+  // Write the new value back out.
+  llvm::StoreInst *Store = Builder.CreateStore(SrcVal, Ptr,
+                                               Dst.isVolatileQualified());
+  Store->setAlignment(Info.StorageAlignment);
 
-    // Cast to the access type.
-    llvm::Type *AccessLTy =
-      llvm::Type::getIntNTy(getLLVMContext(), AI.AccessWidth);
-
-    llvm::Type *PTy = AccessLTy->getPointerTo(addressSpace);
-    Ptr = Builder.CreateBitCast(Ptr, PTy);
-
-    // Extract the piece of the bit-field value to write in this access, limited
-    // to the values that are part of this access.
-    llvm::Value *Val = SrcVal;
-    if (AI.TargetBitOffset)
-      Val = Builder.CreateLShr(Val, AI.TargetBitOffset);
-    Val = Builder.CreateAnd(Val, llvm::APInt::getLowBitsSet(ResSizeInBits,
-                                                            AI.TargetBitWidth));
-
-    // Extend or truncate to the access size.
-    if (ResSizeInBits < AI.AccessWidth)
-      Val = Builder.CreateZExt(Val, AccessLTy);
-    else if (ResSizeInBits > AI.AccessWidth)
-      Val = Builder.CreateTrunc(Val, AccessLTy);
-
-    // Shift into the position in memory.
-    if (AI.FieldBitStart)
-      Val = Builder.CreateShl(Val, AI.FieldBitStart);
-
-    // If necessary, load and OR in bits that are outside of the bit-field.
-    if (AI.TargetBitWidth != AI.AccessWidth) {
-      llvm::LoadInst *Load = Builder.CreateLoad(Ptr, Dst.isVolatileQualified());
-      Load->setAlignment(AccessAlignment.getQuantity());
-
-      // Compute the mask for zeroing the bits that are part of the bit-field.
-      llvm::APInt InvMask =
-        ~llvm::APInt::getBitsSet(AI.AccessWidth, AI.FieldBitStart,
-                                 AI.FieldBitStart + AI.TargetBitWidth);
-
-      // Apply the mask and OR in to the value to write.
-      Val = Builder.CreateOr(Builder.CreateAnd(Load, InvMask), Val);
+  // Return the new value of the bit-field, if requested.
+  if (Result) {
+    llvm::Value *ResultVal = MaskedVal;
+
+    // Sign extend the value if needed.
+    if (Info.IsSigned) {
+      assert(Info.Size <= Info.StorageSize);
+      unsigned HighBits = Info.StorageSize - Info.Size;
+      if (HighBits) {
+        ResultVal = Builder.CreateShl(ResultVal, HighBits, "bf.result.shl");
+        ResultVal = Builder.CreateAShr(ResultVal, HighBits, "bf.result.ashr");
+      }
     }
 
-    // Write the value.
-    llvm::StoreInst *Store = Builder.CreateStore(Val, Ptr,
-                                                 Dst.isVolatileQualified());
-    Store->setAlignment(AccessAlignment.getQuantity());
+    ResultVal = Builder.CreateIntCast(ResultVal, ResLTy, Info.IsSigned,
+                                      "bf.result.cast");
+    *Result = ResultVal;
   }
 }
 
@@ -2333,10 +2256,21 @@ LValue CodeGenFunction::EmitLValueForField(LValue base,
     const CGRecordLayout &RL =
       CGM.getTypes().getCGRecordLayout(field->getParent());
     const CGBitFieldInfo &Info = RL.getBitFieldInfo(field);
+    llvm::Value *Addr = base.getAddress();
+    unsigned Idx = RL.getLLVMFieldNo(field);
+    if (Idx != 0)
+      // For structs, we GEP to the field that the record layout suggests.
+      Addr = Builder.CreateStructGEP(Addr, Idx, field->getName());
+    // Get the access type.
+    llvm::Type *PtrTy = llvm::Type::getIntNPtrTy(
+      getLLVMContext(), Info.StorageSize,
+      CGM.getContext().getTargetAddressSpace(base.getType()));
+    if (Addr->getType() != PtrTy)
+      Addr = Builder.CreateBitCast(Addr, PtrTy);
+
     QualType fieldType =
       field->getType().withCVRQualifiers(base.getVRQualifiers());
-    return LValue::MakeBitfield(base.getAddress(), Info, fieldType,
-                                base.getAlignment());
+    return LValue::MakeBitfield(Addr, Info, fieldType, base.getAlignment());
   }
 
   const RecordDecl *rec = field->getParent();
diff --git a/lib/CodeGen/CGObjCRuntime.cpp b/lib/CodeGen/CGObjCRuntime.cpp
index 744bed81f7..85470455af 100644
--- a/lib/CodeGen/CGObjCRuntime.cpp
+++ b/lib/CodeGen/CGObjCRuntime.cpp
@@ -89,14 +89,13 @@ LValue CGObjCRuntime::EmitValueForIvarAtOffset(CodeGen::CodeGenFunction &CGF,
                                                unsigned CVRQualifiers,
                                                llvm::Value *Offset) {
   // Compute (type*) ( (char *) BaseValue + Offset)
-  llvm::Type *I8Ptr = CGF.Int8PtrTy;
   QualType IvarTy = Ivar->getType();
   llvm::Type *LTy = CGF.CGM.getTypes().ConvertTypeForMem(IvarTy);
-  llvm::Value *V = CGF.Builder.CreateBitCast(BaseValue, I8Ptr);
+  llvm::Value *V = CGF.Builder.CreateBitCast(BaseValue, CGF.Int8PtrTy);
   V = CGF.Builder.CreateInBoundsGEP(V, Offset, "add.ptr");
-  V = CGF.Builder.CreateBitCast(V, llvm::PointerType::getUnqual(LTy));
 
   if (!Ivar->isBitField()) {
+    V = CGF.Builder.CreateBitCast(V, llvm::PointerType::getUnqual(LTy));
     LValue LV = CGF.MakeNaturalAlignAddrLValue(V, IvarTy);
     LV.getQuals().addCVRQualifiers(CVRQualifiers);
     return LV;
@@ -116,16 +115,14 @@ LValue CGObjCRuntime::EmitValueForIvarAtOffset(CodeGen::CodeGenFunction &CGF,
   // Note, there is a subtle invariant here: we can only call this routine on
   // non-synthesized ivars but we may be called for synthesized ivars.  However,
   // a synthesized ivar can never be a bit-field, so this is safe.
-  const ASTRecordLayout &RL =
-    CGF.CGM.getContext().getASTObjCInterfaceLayout(OID);
-  uint64_t TypeSizeInBits = CGF.CGM.getContext().toBits(RL.getSize());
   uint64_t FieldBitOffset = LookupFieldBitOffset(CGF.CGM, OID, 0, Ivar);
   uint64_t BitOffset = FieldBitOffset % CGF.CGM.getContext().getCharWidth();
-  uint64_t ContainingTypeAlign = CGF.CGM.getContext().getTargetInfo().getCharAlign();
-  uint64_t ContainingTypeSize = TypeSizeInBits - (FieldBitOffset - BitOffset);
+  uint64_t AlignmentBits = CGF.CGM.getContext().getTargetInfo().getCharAlign();
   uint64_t BitFieldSize = Ivar->getBitWidthValue(CGF.getContext());
-  CharUnits ContainingTypeAlignCharUnits = 
-    CGF.CGM.getContext().toCharUnitsFromBits(ContainingTypeAlign);
+  CharUnits StorageSize =
+    CGF.CGM.getContext().toCharUnitsFromBits(
+      llvm::RoundUpToAlignment(BitOffset + BitFieldSize, AlignmentBits));
+  CharUnits Alignment = CGF.CGM.getContext().toCharUnitsFromBits(AlignmentBits);
 
   // Allocate a new CGBitFieldInfo object to describe this access.
   //
@@ -135,11 +132,15 @@ LValue CGObjCRuntime::EmitValueForIvarAtOffset(CodeGen::CodeGenFunction &CGF,
   // objects.
   CGBitFieldInfo *Info = new (CGF.CGM.getContext()) CGBitFieldInfo(
     CGBitFieldInfo::MakeInfo(CGF.CGM.getTypes(), Ivar, BitOffset, BitFieldSize,
-                             ContainingTypeSize, ContainingTypeAlign));
+                             CGF.CGM.getContext().toBits(StorageSize),
+                             Alignment.getQuantity()));
 
+  V = CGF.Builder.CreateBitCast(V,
+                                llvm::Type::getIntNPtrTy(CGF.getLLVMContext(),
+                                                         Info->StorageSize));
   return LValue::MakeBitfield(V, *Info,
                               IvarTy.withCVRQualifiers(CVRQualifiers),
-                              ContainingTypeAlignCharUnits);
+                              Alignment);
 }
 
 namespace {
diff --git a/lib/CodeGen/CGRecordLayout.h b/lib/CodeGen/CGRecordLayout.h
index 3db5e0483b..f343ef70fa 100644
--- a/lib/CodeGen/CGRecordLayout.h
+++ b/lib/CodeGen/CGRecordLayout.h
@@ -23,122 +23,71 @@ namespace llvm {
 namespace clang {
 namespace CodeGen {
 
-/// \brief Helper object for describing how to generate the code for access to a
-/// bit-field.
+/// \brief Structure with information about how a bitfield should be accessed.
 ///
-/// This structure is intended to describe the "policy" of how the bit-field
-/// should be accessed, which may be target, language, or ABI dependent.
-class CGBitFieldInfo {
-public:
-  /// Descriptor for a single component of a bit-field access. The entire
-  /// bit-field is constituted of a bitwise OR of all of the individual
-  /// components.
-  ///
-  /// Each component describes an accessed value, which is how the component
-  /// should be transferred to/from memory, and a target placement, which is how
-  /// that component fits into the constituted bit-field. The pseudo-IR for a
-  /// load is:
-  ///
-  ///   %0 = gep %base, 0, FieldIndex
-  ///   %1 = gep (i8*) %0, FieldByteOffset
-  ///   %2 = (i(AccessWidth) *) %1
-  ///   %3 = load %2, align AccessAlignment
-  ///   %4 = shr %3, FieldBitStart
-  ///
-  /// and the composed bit-field is formed as the boolean OR of all accesses,
-  /// masked to TargetBitWidth bits and shifted to TargetBitOffset.
-  struct AccessInfo {
-    /// Offset of the field to load in the LLVM structure, if any.
-    unsigned FieldIndex;
-
-    /// Byte offset from the field address, if any. This should generally be
-    /// unused as the cleanest IR comes from having a well-constructed LLVM type
-    /// with proper GEP instructions, but sometimes its use is required, for
-    /// example if an access is intended to straddle an LLVM field boundary.
-    CharUnits FieldByteOffset;
-
-    /// Bit offset in the accessed value to use. The width is implied by \see
-    /// TargetBitWidth.
-    unsigned FieldBitStart;
-
-    /// Bit width of the memory access to perform.
-    unsigned AccessWidth;
-
-    /// The alignment of the memory access, assuming the parent is aligned.
-    CharUnits AccessAlignment;
-
-    /// Offset for the target value.
-    unsigned TargetBitOffset;
-
-    /// Number of bits in the access that are destined for the bit-field.
-    unsigned TargetBitWidth;
-  };
-
-private:
-  /// The components to use to access the bit-field. We may need up to three
-  /// separate components to support up to i64 bit-field access (4 + 2 + 1 byte
-  /// accesses).
-  //
-  // FIXME: De-hardcode this, just allocate following the struct.
-  AccessInfo Components[3];
+/// Often we layout a sequence of bitfields as a contiguous sequence of bits.
+/// When the AST record layout does this, we represent it in the LLVM IR's type
+/// as either a sequence of i8 members or a byte array to reserve the number of
+/// bytes touched without forcing any particular alignment beyond the basic
+/// character alignment.
+///
+/// Then accessing a particular bitfield involves converting this byte array
+/// into a single integer of that size (i24 or i40 -- may not be power-of-two
+/// size), loading it, and shifting and masking to extract the particular
+/// subsequence of bits which make up that particular bitfield. This structure
+/// encodes the information used to construct the extraction code sequences.
+/// The CGRecordLayout also has a field index which encodes which byte-sequence
+/// this bitfield falls within. Let's assume the following C struct:
+///
+///   struct S {
+///     char a, b, c;
+///     unsigned bits : 3;
+///     unsigned more_bits : 4;
+///     unsigned still_more_bits : 7;
+///   };
+///
+/// This will end up as the following LLVM type. The first array is the
+/// bitfield, and the second is the padding out to a 4-byte alignmnet.
+///
+///   %t = type { i8, i8, i8, i8, i8, [3 x i8] }
+///
+/// When generating code to access more_bits, we'll generate something
+/// essentially like this:
+///
+///   define i32 @foo(%t* %base) {
+///     %0 = gep %t* %base, i32 0, i32 3
+///     %2 = load i8* %1
+///     %3 = lshr i8 %2, 3
+///     %4 = and i8 %3, 15
+///     %5 = zext i8 %4 to i32
+///     ret i32 %i
+///   }
+///
+struct CGBitFieldInfo {
+  /// The offset within a contiguous run of bitfields that are represented as
+  /// a single "field" within the LLVM struct type. This offset is in bits.
+  unsigned Offset : 16;
 
   /// The total size of the bit-field, in bits.
-  unsigned Size;
-
-  /// The number of access components to use.
-  unsigned NumComponents;
+  unsigned Size : 15;
 
   /// Whether the bit-field is signed.
-  bool IsSigned : 1;
+  unsigned IsSigned : 1;
 
-public:
-  CGBitFieldInfo(unsigned Size, unsigned NumComponents, AccessInfo *_Components,
-                 bool IsSigned) : Size(Size), NumComponents(NumComponents),
-                                  IsSigned(IsSigned) {
-    assert(NumComponents <= 3 && "invalid number of components!");
-    for (unsigned i = 0; i != NumComponents; ++i)
-      Components[i] = _Components[i];
-
-    // Check some invariants.
-    unsigned AccessedSize = 0;
-    for (unsigned i = 0, e = getNumComponents(); i != e; ++i) {
-      const AccessInfo &AI = getComponent(i);
-      AccessedSize += AI.TargetBitWidth;
-
-      // We shouldn't try to load 0 bits.
-      assert(AI.TargetBitWidth > 0);
-
-      // We can't load more bits than we accessed.
-      assert(AI.FieldBitStart + AI.TargetBitWidth <= AI.AccessWidth);
-
-      // We shouldn't put any bits outside the result size.
-      assert(AI.TargetBitWidth + AI.TargetBitOffset <= Size);
-    }
-
-    // Check that the total number of target bits matches the total bit-field
-    // size.
-    assert(AccessedSize == Size && "Total size does not match accessed size!");
-  }
-
-public:
-  /// \brief Check whether this bit-field access is (i.e., should be sign
-  /// extended on loads).
-  bool isSigned() const { return IsSigned; }
-
-  /// \brief Get the size of the bit-field, in bits.
-  unsigned getSize() const { return Size; }
+  /// The storage size in bits which should be used when accessing this
+  /// bitfield.
+  unsigned StorageSize;
 
-  /// @name Component Access
-  /// @{
+  /// The alignment which should be used when accessing the bitfield.
+  unsigned StorageAlignment;
 
-  unsigned getNumComponents() const { return NumComponents; }
+  CGBitFieldInfo()
+      : Offset(), Size(), IsSigned(), StorageSize(), StorageAlignment() {}
 
-  const AccessInfo &getComponent(unsigned Index) const {
-    assert(Index < getNumComponents() && "Invalid access!");
-    return Components[Index];
-  }
-
-  /// @}
+  CGBitFieldInfo(unsigned Offset, unsigned Size, bool IsSigned,
+                 unsigned StorageSize, unsigned StorageAlignment)
+      : Offset(Offset), Size(Size), IsSigned(IsSigned),
+        StorageSize(StorageSize), StorageAlignment(StorageAlignment) {}
 
   void print(raw_ostream &OS) const;
   void dump() const;
@@ -146,17 +95,11 @@ public:
   /// \brief Given a bit-field decl, build an appropriate helper object for
   /// accessing that field (which is expected to have the given offset and
   /// size).
-  static CGBitFieldInfo MakeInfo(class CodeGenTypes &Types, const FieldDecl *FD,
-                                 uint64_t FieldOffset, uint64_t FieldSize);
-
-  /// \brief Given a bit-field decl, build an appropriate helper object for
-  /// accessing that field (which is expected to have the given offset and
-  /// size). The field decl should be known to be contained within a type of at
-  /// least the given size and with the given alignment.
-  static CGBitFieldInfo MakeInfo(CodeGenTypes &Types, const FieldDecl *FD,
-                                 uint64_t FieldOffset, uint64_t FieldSize,
-                                 uint64_t ContainingTypeSizeInBits,
-                                 unsigned ContainingTypeAlign);
+  static CGBitFieldInfo MakeInfo(class CodeGenTypes &Types,
+                                 const FieldDecl *FD,
+                                 uint64_t Offset, uint64_t Size,
+                                 uint64_t StorageSize,
+                                 uint64_t StorageAlignment);
 };
 
 /// CGRecordLayout - This class handles struct and union layout info while
@@ -240,7 +183,6 @@ public:
   /// \brief Return llvm::StructType element number that corresponds to the
   /// field FD.
   unsigned getLLVMFieldNo(const FieldDecl *FD) const {
-    assert(!FD->isBitField() && "Invalid call for bit-field decl!");
     assert(FieldInfo.count(FD) && "Invalid field for record!");
     return FieldInfo.lookup(FD);
   }
diff --git a/lib/CodeGen/CGRecordLayoutBuilder.cpp b/lib/CodeGen/CGRecordLayoutBuilder.cpp
index ac542abf5d..123848b8fc 100644
--- a/lib/CodeGen/CGRecordLayoutBuilder.cpp
+++ b/lib/CodeGen/CGRecordLayoutBuilder.cpp
@@ -100,10 +100,6 @@ private:
   /// Alignment - Contains the alignment of the RecordDecl.
   CharUnits Alignment;
 
-  /// BitsAvailableInLastField - If a bit field spans only part of a LLVM field,
-  /// this will have the number of bits still available in the field.
-  char BitsAvailableInLastField;
-
   /// NextFieldOffset - Holds the next field offset.
   CharUnits NextFieldOffset;
 
@@ -115,6 +111,12 @@ private:
   /// LayoutUnion - Will layout a union RecordDecl.
   void LayoutUnion(const RecordDecl *D);
 
+  /// Lay out a sequence of contiguous bitfields.
+  bool LayoutBitfields(const ASTRecordLayout &Layout,
+                       unsigned &FirstFieldNo,
+                       RecordDecl::field_iterator &FI,
+                       RecordDecl::field_iterator FE);
+
   /// LayoutField - try to layout all fields in the record decl.
   /// Returns false if the operation failed because the struct is not packed.
   bool LayoutFields(const RecordDecl *D);
@@ -194,7 +196,7 @@ public:
     : BaseSubobjectType(0),
       IsZeroInitializable(true), IsZeroInitializableAsBase(true),
       Packed(false), IsMsStruct(false),
-      Types(Types), BitsAvailableInLastField(0) { }
+      Types(Types) { }
 
   /// Layout - Will layout a RecordDecl.
   void Layout(const RecordDecl *D);
@@ -230,13 +232,10 @@ void CGRecordLayoutBuilder::Layout(const RecordDecl *D) {
 }
 
 CGBitFieldInfo CGBitFieldInfo::MakeInfo(CodeGenTypes &Types,
-                               const FieldDecl *FD,
-                               uint64_t FieldOffset,
-                               uint64_t FieldSize,
-                               uint64_t ContainingTypeSizeInBits,
-                               unsigned ContainingTypeAlign) {
-  assert(ContainingTypeAlign && "Expected alignment to be specified");
-
+                                        const FieldDecl *FD,
+                                        uint64_t Offset, uint64_t Size,
+                                        uint64_t StorageSize,
+                                        uint64_t StorageAlignment) {
   llvm::Type *Ty = Types.ConvertTypeForMem(FD->getType());
   CharUnits TypeSizeInBytes =
     CharUnits::fromQuantity(Types.getDataLayout().getTypeAllocSize(Ty));
@@ -244,7 +243,7 @@ CGBitFieldInfo CGBitFieldInfo::MakeInfo(CodeGenTypes &Types,
 
   bool IsSigned = FD->getType()->isSignedIntegerOrEnumerationType();
 
-  if (FieldSize > TypeSizeInBits) {
+  if (Size > TypeSizeInBits) {
     // We have a wide bit-field. The extra bits are only used for padding, so
     // if we have a bitfield of type T, with size N:
     //
@@ -254,173 +253,127 @@ CGBitFieldInfo CGBitFieldInfo::MakeInfo(CodeGenTypes &Types,
     //
     // T t : sizeof(T);
     //
-    FieldSize = TypeSizeInBits;
-  }
-
-  // in big-endian machines the first fields are in higher bit positions,
-  // so revert the offset. The byte offsets are reversed(back) later.
-  if (Types.getDataLayout().isBigEndian()) {
-    FieldOffset = ((ContainingTypeSizeInBits)-FieldOffset-FieldSize);
-  }
-
-  // Compute the access components. The policy we use is to start by attempting
-  // to access using the width of the bit-field type itself and to always access
-  // at aligned indices of that type. If such an access would fail because it
-  // extends past the bound of the type, then we reduce size to the next smaller
-  // power of two and retry. The current algorithm assumes pow2 sized types,
-  // although this is easy to fix.
-  //
-  assert(llvm::isPowerOf2_32(TypeSizeInBits) && "Unexpected type size!");
-  CGBitFieldInfo::AccessInfo Components[3];
-  unsigned NumComponents = 0;
-  unsigned AccessedTargetBits = 0;       // The number of target bits accessed.
-  unsigned AccessWidth = TypeSizeInBits; // The current access width to attempt.
-
-  // If requested, widen the initial bit-field access to be register sized. The
-  // theory is that this is most likely to allow multiple accesses into the same
-  // structure to be coalesced, and that the backend should be smart enough to
-  // narrow the store if no coalescing is ever done.
-  //
-  // The subsequent code will handle align these access to common boundaries and
-  // guaranteeing that we do not access past the end of the structure.
-  if (Types.getCodeGenOpts().UseRegisterSizedBitfieldAccess) {
-    if (AccessWidth < Types.getTarget().getRegisterWidth())
-      AccessWidth = Types.getTarget().getRegisterWidth();
-  }
-
-  // Round down from the field offset to find the first access position that is
-  // at an aligned offset of the initial access type.
-  uint64_t AccessStart = FieldOffset - (FieldOffset % AccessWidth);
-
-  // Adjust initial access size to fit within record.
-  while (AccessWidth > Types.getTarget().getCharWidth() &&
-         AccessStart + AccessWidth > ContainingTypeSizeInBits) {
-    AccessWidth >>= 1;
-    AccessStart = FieldOffset - (FieldOffset % AccessWidth);
-  }
-
-  while (AccessedTargetBits < FieldSize) {
-    // Check that we can access using a type of this size, without reading off
-    // the end of the structure. This can occur with packed structures and
-    // -fno-bitfield-type-align, for example.
-    if (AccessStart + AccessWidth > ContainingTypeSizeInBits) {
-      // If so, reduce access size to the next smaller power-of-two and retry.
-      AccessWidth >>= 1;
-      assert(AccessWidth >= Types.getTarget().getCharWidth()
-             && "Cannot access under byte size!");
-      continue;
-    }
-
-    // Otherwise, add an access component.
-
-    // First, compute the bits inside this access which are part of the
-    // target. We are reading bits [AccessStart, AccessStart + AccessWidth); the
-    // intersection with [FieldOffset, FieldOffset + FieldSize) gives the bits
-    // in the target that we are reading.
-    assert(FieldOffset < AccessStart + AccessWidth && "Invalid access start!");
-    assert(AccessStart < FieldOffset + FieldSize && "Invalid access start!");
-    uint64_t AccessBitsInFieldStart = std::max(AccessStart, FieldOffset);
-    uint64_t AccessBitsInFieldSize =
-      std::min(AccessWidth + AccessStart,
-               FieldOffset + FieldSize) - AccessBitsInFieldStart;
-
-    assert(NumComponents < 3 && "Unexpected number of components!");
-    CGBitFieldInfo::AccessInfo &AI = Components[NumComponents++];
-    AI.FieldIndex = 0;
-    // FIXME: We still follow the old access pattern of only using the field
-    // byte offset. We should switch this once we fix the struct layout to be
-    // pretty.
-
-    // on big-endian machines we reverted the bit offset because first fields are
-    // in higher bits. But this also reverts the bytes, so fix this here by reverting
-    // the byte offset on big-endian machines.
-    if (Types.getDataLayout().isBigEndian()) {
-      AI.FieldByteOffset = Types.getContext().toCharUnitsFromBits(
-          ContainingTypeSizeInBits - AccessStart - AccessWidth);
-    } else {
-      AI.FieldByteOffset = Types.getContext().toCharUnitsFromBits(AccessStart);
-    }
-    AI.FieldBitStart = AccessBitsInFieldStart - AccessStart;
-    AI.AccessWidth = AccessWidth;
-    AI.AccessAlignment = Types.getContext().toCharUnitsFromBits(
-        llvm::MinAlign(ContainingTypeAlign, AccessStart));
-    AI.TargetBitOffset = AccessedTargetBits;
-    AI.TargetBitWidth = AccessBitsInFieldSize;
-
-    AccessStart += AccessWidth;
-    AccessedTargetBits += AI.TargetBitWidth;
+    Size = TypeSizeInBits;
   }
 
-  assert(AccessedTargetBits == FieldSize && "Invalid bit-field access!");
-  return CGBitFieldInfo(FieldSize, NumComponents, Components, IsSigned);
-}
+  // Reverse the bit offsets for big endian machines.
+  if (Types.getDataLayout().isBigEndian())
+    Offset = Size - Offset - 1;
 
-CGBitFieldInfo CGBitFieldInfo::MakeInfo(CodeGenTypes &Types,
-                                        const FieldDecl *FD,
-                                        uint64_t FieldOffset,
-                                        uint64_t FieldSize) {
-  const RecordDecl *RD = FD->getParent();
-  const ASTRecordLayout &RL = Types.getContext().getASTRecordLayout(RD);
-  uint64_t ContainingTypeSizeInBits = Types.getContext().toBits(RL.getSize());
-  unsigned ContainingTypeAlign = Types.getContext().toBits(RL.getAlignment());
-
-  return MakeInfo(Types, FD, FieldOffset, FieldSize, ContainingTypeSizeInBits,
-                  ContainingTypeAlign);
+  return CGBitFieldInfo(Offset, Size, IsSigned, StorageSize, StorageAlignment);
 }
 
-void CGRecordLayoutBuilder::LayoutBitField(const FieldDecl *D,
-                                           uint64_t fieldOffset) {
-  uint64_t fieldSize = D->getBitWidthValue(Types.getContext());
-
-  if (fieldSize == 0)
-    return;
-
-  uint64_t nextFieldOffsetInBits = Types.getContext().toBits(NextFieldOffset);
-  CharUnits numBytesToAppend;
-  unsigned charAlign = Types.getContext().getTargetInfo().getCharAlign();
-
-  if (fieldOffset < nextFieldOffsetInBits && !BitsAvailableInLastField) {
-    assert(fieldOffset % charAlign == 0 && 
-           "Field offset not aligned correctly");
-
-    CharUnits fieldOffsetInCharUnits = 
-      Types.getContext().toCharUnitsFromBits(fieldOffset);
+/// \brief Layout the range of bitfields from BFI to BFE as contiguous storage.
+bool CGRecordLayoutBuilder::LayoutBitfields(const ASTRecordLayout &Layout,
+                                            unsigned &FirstFieldNo,
+                                            RecordDecl::field_iterator &FI,
+                                            RecordDecl::field_iterator FE) {
+  assert(FI != FE);
+  uint64_t FirstFieldOffset = Layout.getFieldOffset(FirstFieldNo);
+  uint64_t NextFieldOffsetInBits = Types.getContext().toBits(NextFieldOffset);
+
+  unsigned CharAlign = Types.getContext().getTargetInfo().getCharAlign();
+  assert(FirstFieldOffset % CharAlign == 0 &&
+         "First field offset is misaligned");
+  CharUnits FirstFieldOffsetInBytes
+    = Types.getContext().toCharUnitsFromBits(FirstFieldOffset);
+
+  unsigned StorageAlignment
+    = llvm::MinAlign(Alignment.getQuantity(),
+                     FirstFieldOffsetInBytes.getQuantity());
+
+  if (FirstFieldOffset < NextFieldOffsetInBits) {
+    CharUnits FieldOffsetInCharUnits =
+      Types.getContext().toCharUnitsFromBits(FirstFieldOffset);
 
     // Try to resize the last base field.
-    if (ResizeLastBaseFieldIfNecessary(fieldOffsetInCharUnits))
-      nextFieldOffsetInBits = Types.getContext().toBits(NextFieldOffset);
-  }
-
-  if (fieldOffset < nextFieldOffsetInBits) {
-    assert(BitsAvailableInLastField && "Bitfield size mismatch!");
-    assert(!NextFieldOffset.isZero() && "Must have laid out at least one byte");
+    if (!ResizeLastBaseFieldIfNecessary(FieldOffsetInCharUnits))
+      llvm_unreachable("We must be able to resize the last base if we need to "
+                       "pack bits into it.");
 
-    // The bitfield begins in the previous bit-field.
-    numBytesToAppend = Types.getContext().toCharUnitsFromBits(
-      llvm::RoundUpToAlignment(fieldSize - BitsAvailableInLastField, 
-                               charAlign));
-  } else {
-    assert(fieldOffset % charAlign == 0 && 
-           "Field offset not aligned correctly");
-
-    // Append padding if necessary.
-    AppendPadding(Types.getContext().toCharUnitsFromBits(fieldOffset), 
-                  CharUnits::One());
-
-    numBytesToAppend = Types.getContext().toCharUnitsFromBits(
-        llvm::RoundUpToAlignment(fieldSize, charAlign));
-
-    assert(!numBytesToAppend.isZero() && "No bytes to append!");
+    NextFieldOffsetInBits = Types.getContext().toBits(NextFieldOffset);
+    assert(FirstFieldOffset >= NextFieldOffsetInBits);
   }
 
-  // Add the bit field info.
-  BitFields.ins