Respect alignment of nested bitfields

[uweigand]

Summary

tools/clang/test/CodeGen/packed-nest-unpacked.c contains this test:

struct XBitfield {

unsigned b1 : 10;
unsigned b2 : 12;
unsigned b3 : 10;

};
struct YBitfield {

char x;
struct XBitfield y;

} __attribute((packed));
struct YBitfield gbitfield;

unsigned test7() {

// CHECK: @test7
// CHECK: load i32, i32* getelementptr inbounds (%struct.YBitfield, %struct.YBitfield* @gbitfield, i32 0, i32 1, i32 0), align 4
return gbitfield.y.b2;

}

The "align 4" is actually wrong. Accessing all of "gbitfield.y" as a single
i32 is of course possible, but that still doesn't make it 4-byte aligned as
it remains packed at offset 1 in the surrounding gbitfield object.

This alignment was changed by commit r169489 by chandlerc, which also
introduced changes to bitfield access code in CGExpr.cpp. Code before
that change used to take into account *both* the alignment of the field to
be accessed within the current struct, *and* the alignment of that outer
struct itself; this logic was removed by the above commit.

However, this is incorrect; we do need to consider both these alignment values:

LV.getAlignment() is the alignment of the surrounding struct
                  (see e.g. EmitLValueForField code computing this value)
Info.StorageAlignment is the alignment of the storage backing
                      the bitfield relative to the struct

Neglecting to consider both values can cause incorrect code to be generated
(I've seen an unaligned access crash on SystemZ due to this bug).

This patch adds back logic to also consider alignment of the outer struct
in EmitLoadOfBitfieldLValue and EmitStoreThroughBitfieldLValue. It also
extends the test case to cover the case of storing into the nested bitfield.

(Patch originally posted to cfe-commits; now moved to Phabricator
to simplify review.)

[rjmccall]

Good catch!

The better fix would be to ignore the bitfield's abstract storage alignment and instead use the base l-value's alignment, adjusted with alignmentAtOffset to the offset of the bitfield's storage. This ensures we get the right alignment when the base l-value is *over*-aligned: in a packed struct, bitfield storage at offset 0 will have alignment 1, but if the packed struct itself is known to have alignment 4 (e.g., if it is embedded in an unpacked struct), then we still know that the storage has alignment 4.alignmentAtOffset(0) == 4.

Also, please use CreateAlignedStore and CreateAlignedLoad.

[uweigand]

Addressed review comments:

• Use CreateAlignedLoad / CreateAlignedStore

[uweigand]

In D11034#201452, @rjmccall wrote:

Good catch!

The better fix would be to ignore the bitfield's abstract storage alignment and instead use the base l-value's alignment, adjusted with alignmentAtOffset to the offset of the bitfield's storage. This ensures we get the right alignment when the base l-value is *over*-aligned: in a packed struct, bitfield storage at offset 0 will have alignment 1, but if the packed struct itself is known to have alignment 4 (e.g., if it is embedded in an unpacked struct), then we still know that the storage has alignment 4.alignmentAtOffset(0) == 4.

Good point. Unfortunately, at the point the access is generated, there doesn't appear to be an easy way to retrieve the offset of the bitfield's storage relative to the surrounding struct. This value is present as "StartOffset" in CGRecordLowering::setBitFieldInfo, but is not actually stored in the CGBitFieldInfo struct. Maybe instead of the StorageAlignment field, the CGBitFieldInfo struct should instead have a StorageOffset field?

Also, please use CreateAlignedStore and CreateAlignedLoad.

OK, done.

[rjmccall]

In D11034#201981, @uweigand wrote:

In D11034#201452, @rjmccall wrote:

Good catch!

The better fix would be to ignore the bitfield's abstract storage alignment and instead use the base l-value's alignment, adjusted with alignmentAtOffset to the offset of the bitfield's storage. This ensures we get the right alignment when the base l-value is *over*-aligned: in a packed struct, bitfield storage at offset 0 will have alignment 1, but if the packed struct itself is known to have alignment 4 (e.g., if it is embedded in an unpacked struct), then we still know that the storage has alignment 4.alignmentAtOffset(0) == 4.

Good point. Unfortunately, at the point the access is generated, there doesn't appear to be an easy way to retrieve the offset of the bitfield's storage relative to the surrounding struct. This value is present as "StartOffset" in CGRecordLowering::setBitFieldInfo, but is not actually stored in the CGBitFieldInfo struct. Maybe instead of the StorageAlignment field, the CGBitFieldInfo struct should instead have a StorageOffset field?

Storing the offset makes sense. If this means that we don't otherwise need StorageAlignment — probably true — then we can drop that at the same time.

[uweigand]

Replaced CGBitFieldInfo::StorageAlignment with a StorageOffset field.

[rjmccall]

LGTM, thanks!

cfe/trunk/lib/CodeGen/CGAtomic.cpp

AtomicInfo(CodeGenFunction &CGF, LValue &lvalue)
             VoidPtrAddr, OffsetInChars.getQuantity());
         auto Addr = CGF.Builder.CreatePointerBitCastOrAddrSpaceCast(
             VoidPtrAddr,
             CGF.Builder.getIntNTy(AtomicSizeInBits)->getPointerTo(),
             "atomic_bitfield_base");
         BFI = OrigBFI;
         BFI.Offset = Offset;
         BFI.StorageSize = AtomicSizeInBits;
+        BFI.StorageOffset += OffsetInChars;
         LVal = LValue::MakeBitfield(Addr, BFI, lvalue.getType(),
                                     lvalue.getAlignment());
         LVal.setTBAAInfo(lvalue.getTBAAInfo());
         AtomicTy = C.getIntTypeForBitwidth(AtomicSizeInBits, OrigBFI.IsSigned);
         if (AtomicTy.isNull()) {
           llvm::APInt Size(
               /*numBits=*/32,
               C.toCharUnitsFromBits(AtomicSizeInBits).getQuantity());

cfe/trunk/lib/CodeGen/CGClass.cpp

public:
 
     void emitMemcpy() {
       // Give the subclass a chance to bail out if it feels the memcpy isn't
       // worth it (e.g. Hasn't aggregated enough data).
       if (!FirstField) {
         return;
       }
 
-      CharUnits Alignment;
-
       uint64_t FirstByteOffset;
       if (FirstField->isBitField()) {
         const CGRecordLayout &RL =
           CGF.getTypes().getCGRecordLayout(FirstField->getParent());
         const CGBitFieldInfo &BFInfo = RL.getBitFieldInfo(FirstField);
-        Alignment = CharUnits::fromQuantity(BFInfo.StorageAlignment);
         // FirstFieldOffset is not appropriate for bitfields,
         // it won't tell us what the storage offset should be and thus might not
         // be properly aligned.
         //
         // Instead calculate the storage offset using the offset of the field in
         // the struct type.
-        const llvm::DataLayout &DL = CGF.CGM.getDataLayout();
-        FirstByteOffset =
-            DL.getStructLayout(RL.getLLVMType())
-                ->getElementOffsetInBits(RL.getLLVMFieldNo(FirstField));
+        FirstByteOffset = CGF.getContext().toBits(BFInfo.StorageOffset);
       } else {
-        Alignment = CGF.getContext().getDeclAlign(FirstField);
         FirstByteOffset = FirstFieldOffset;
       }
 
-      assert((CGF.getContext().toCharUnitsFromBits(FirstByteOffset) %
-              Alignment) == 0 && "Bad field alignment.");
-
       CharUnits MemcpySize = getMemcpySize(FirstByteOffset);
       QualType RecordTy = CGF.getContext().getTypeDeclType(ClassDecl);
       llvm::Value *ThisPtr = CGF.LoadCXXThis();
       LValue DestLV = CGF.MakeNaturalAlignAddrLValue(ThisPtr, RecordTy);
       LValue Dest = CGF.EmitLValueForFieldInitialization(DestLV, FirstField);
       llvm::Value *SrcPtr = CGF.Builder.CreateLoad(CGF.GetAddrOfLocalVar(SrcRec));
       LValue SrcLV = CGF.MakeNaturalAlignAddrLValue(SrcPtr, RecordTy);
       LValue Src = CGF.EmitLValueForFieldInitialization(SrcLV, FirstField);
 
+      CharUnits Offset = CGF.getContext().toCharUnitsFromBits(FirstByteOffset);
+      CharUnits Alignment = DestLV.getAlignment().alignmentAtOffset(Offset);
+
       emitMemcpyIR(Dest.isBitField() ? Dest.getBitFieldAddr() : Dest.getAddress(),
                    Src.isBitField() ? Src.getBitFieldAddr() : Src.getAddress(),
                    MemcpySize, Alignment);
       reset();
     }
 
     void reset() {
       FirstField = nullptr;

cfe/trunk/lib/CodeGen/CGExpr.cpp

if (LV.isGlobalReg())
     return EmitLoadOfGlobalRegLValue(LV);
 
   assert(LV.isBitField() && "Unknown LValue type!");
   return EmitLoadOfBitfieldLValue(LV);
 }
 
 RValue CodeGenFunction::EmitLoadOfBitfieldLValue(LValue LV) {
   const CGBitFieldInfo &Info = LV.getBitFieldInfo();
+  CharUnits Align = LV.getAlignment().alignmentAtOffset(Info.StorageOffset);
 
   // Get the output type.
   llvm::Type *ResLTy = ConvertType(LV.getType());
 
   llvm::Value *Ptr = LV.getBitFieldAddr();
-  llvm::Value *Val = Builder.CreateLoad(Ptr, LV.isVolatileQualified(),
+  llvm::Value *Val = Builder.CreateAlignedLoad(Ptr, Align.getQuantity(),
+                                               LV.isVolatileQualified(),
                                                "bf.load");
-  cast<llvm::LoadInst>(Val)->setAlignment(Info.StorageAlignment);
 
   if (Info.IsSigned) {
     assert(static_cast<unsigned>(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");
void CodeGenFunction::EmitStoreThroughLValue(RValue Src, LValue Dst,
 
   assert(Src.isScalar() && "Can't emit an agg store with this method");
   EmitStoreOfScalar(Src.getScalarVal(), Dst, isInit);
 }
 
 void CodeGenFunction::EmitStoreThroughBitfieldLValue(RValue Src, LValue Dst,
                                                      llvm::Value **Result) {
   const CGBitFieldInfo &Info = Dst.getBitFieldInfo();
+  CharUnits Align = Dst.getAlignment().alignmentAtOffset(Info.StorageOffset);
   llvm::Type *ResLTy = ConvertTypeForMem(Dst.getType());
   llvm::Value *Ptr = Dst.getBitFieldAddr();
 
   // Get the source value, truncated to the width of the bit-field.
   llvm::Value *SrcVal = Src.getScalarVal();
 
   // 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(),
+    llvm::Value *Val = Builder.CreateAlignedLoad(Ptr, Align.getQuantity(),
+                                                 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.StorageSize != Info.Size) {
 
     // Or together the unchanged values and the source value.
     SrcVal = Builder.CreateOr(Val, SrcVal, "bf.set");
   } else {
     assert(Info.Offset == 0);
   }
 
   // Write the new value back out.
-  llvm::StoreInst *Store = Builder.CreateStore(SrcVal, Ptr,
-                                               Dst.isVolatileQualified());
-  Store->setAlignment(Info.StorageAlignment);
+  Builder.CreateAlignedStore(SrcVal, Ptr, Align.getQuantity(),
 
   // 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);

cfe/trunk/lib/CodeGen/CGObjCRuntime.cpp

LValue CGObjCRuntime::EmitValueForIvarAtOffset(CodeGen::CodeGenFunction &CGF,
   //
   // FIXME: This is incredibly wasteful, these should be uniqued or part of some
   // layout object. However, this is blocked on other cleanups to the
   // Objective-C code, so for now we just live with allocating a bunch of these
   // objects.
   CGBitFieldInfo *Info = new (CGF.CGM.getContext()) CGBitFieldInfo(
     CGBitFieldInfo::MakeInfo(CGF.CGM.getTypes(), Ivar, BitOffset, BitFieldSize,
                              CGF.CGM.getContext().toBits(StorageSize),
-                             Alignment.getQuantity()));
+                             CharUnits::fromQuantity(0)));
 
   V = CGF.Builder.CreateBitCast(V,
                                 llvm::Type::getIntNPtrTy(CGF.getLLVMContext(),
                                                          Info->StorageSize));
   return LValue::MakeBitfield(V, *Info,
                               IvarTy.withCVRQualifiers(CVRQualifiers),
                               Alignment);
 }

cfe/trunk/lib/CodeGen/CGRecordLayout.h

struct CGBitFieldInfo {
 
   /// Whether the bit-field is signed.
   unsigned IsSigned : 1;
 
   /// The storage size in bits which should be used when accessing this
   /// bitfield.
   unsigned StorageSize;
 
-  /// The alignment which should be used when accessing the bitfield.
-  unsigned StorageAlignment;
+  /// The offset of the bitfield storage from the start of the struct.
+  CharUnits StorageOffset;
 
   CGBitFieldInfo()
-      : Offset(), Size(), IsSigned(), StorageSize(), StorageAlignment() {}
+      : Offset(), Size(), IsSigned(), StorageSize(), StorageOffset() {}
 
   CGBitFieldInfo(unsigned Offset, unsigned Size, bool IsSigned,
-                 unsigned StorageSize, unsigned StorageAlignment)
+                 unsigned StorageSize, CharUnits StorageOffset)
       : Offset(Offset), Size(Size), IsSigned(IsSigned),
-        StorageSize(StorageSize), StorageAlignment(StorageAlignment) {}
+        StorageSize(StorageSize), StorageOffset(StorageOffset) {}
 
   void print(raw_ostream &OS) const;
   void dump() const;
 
   /// \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 Offset, uint64_t Size,
                                  uint64_t StorageSize,
-                                 uint64_t StorageAlignment);
+                                 CharUnits StorageOffset);
 };
 
 /// CGRecordLayout - This class handles struct and union layout info while
 /// lowering AST types to LLVM types.
 ///
 /// These layout objects are only created on demand as IR generation requires.
 class CGRecordLayout {
   friend class CodeGenTypes;

cfe/trunk/lib/CodeGen/CGRecordLayoutBuilder.cpp

 
 void CGRecordLowering::setBitFieldInfo(
     const FieldDecl *FD, CharUnits StartOffset, llvm::Type *StorageType) {
   CGBitFieldInfo &Info = BitFields[FD->getCanonicalDecl()];
   Info.IsSigned = FD->getType()->isSignedIntegerOrEnumerationType();
   Info.Offset = (unsigned)(getFieldBitOffset(FD) - Context.toBits(StartOffset));
   Info.Size = FD->getBitWidthValue(Context);
   Info.StorageSize = (unsigned)DataLayout.getTypeAllocSizeInBits(StorageType);
-  // Here we calculate the actual storage alignment of the bits.  E.g if we've
-  // got an alignment >= 2 and the bitfield starts at offset 6 we've got an
-  // alignment of 2.
-  Info.StorageAlignment =
-      Layout.getAlignment().alignmentAtOffset(StartOffset).getQuantity();
+  Info.StorageOffset = StartOffset;
   if (Info.Size > Info.StorageSize)
     Info.Size = Info.StorageSize;
   // Reverse the bit offsets for big endian machines. Because we represent
   // a bitfield as a single large integer load, we can imagine the bits
   // counting from the most-significant-bit instead of the
   // least-significant-bit.
   if (DataLayout.isBigEndian())
     Info.Offset = Info.StorageSize - (Info.Offset + Info.Size);
else if (Member->Kind == MemberInfo::VBase)
       VirtualBases[Member->RD] = FieldTypes.size() - 1;
   }
 }
 
 CGBitFieldInfo CGBitFieldInfo::MakeInfo(CodeGenTypes &Types,
                                         const FieldDecl *FD,
                                         uint64_t Offset, uint64_t Size,
                                         uint64_t StorageSize,
-                                        uint64_t StorageAlignment) {
+                                        CharUnits StorageOffset) {
   // This function is vestigial from CGRecordLayoutBuilder days but is still 
   // used in GCObjCRuntime.cpp.  That usage has a "fixme" attached to it that
   // when addressed will allow for the removal of this function.
   llvm::Type *Ty = Types.ConvertTypeForMem(FD->getType());
   CharUnits TypeSizeInBytes =
     CharUnits::fromQuantity(Types.getDataLayout().getTypeAllocSize(Ty));
   uint64_t TypeSizeInBits = Types.getContext().toBits(TypeSizeInBytes);
 
CGBitFieldInfo CGBitFieldInfo::MakeInfo(CodeGenTypes &Types,
   // Reverse the bit offsets for big endian machines. Because we represent
   // a bitfield as a single large integer load, we can imagine the bits
   // counting from the most-significant-bit instead of the
   // least-significant-bit.
   if (Types.getDataLayout().isBigEndian()) {
     Offset = StorageSize - (Offset + Size);
   }
 
-  return CGBitFieldInfo(Offset, Size, IsSigned, StorageSize, StorageAlignment);
+  return CGBitFieldInfo(Offset, Size, IsSigned, StorageSize, StorageOffset);
 }
 
 CGRecordLayout *CodeGenTypes::ComputeRecordLayout(const RecordDecl *D,
                                                   llvm::StructType *Ty) {
   CGRecordLowering Builder(*this, D, /*Packed=*/false);
 
   Builder.lower(/*NonVirtualBaseType=*/false);
 

 }
 
 void CGBitFieldInfo::print(raw_ostream &OS) const {
   OS << "<CGBitFieldInfo"
      << " Offset:" << Offset
      << " Size:" << Size
      << " IsSigned:" << IsSigned
      << " StorageSize:" << StorageSize
-     << " StorageAlignment:" << StorageAlignment << ">";
+     << " StorageOffset:" << StorageOffset.getQuantity() << ">";
 }
 
 void CGBitFieldInfo::dump() const {
   print(llvm::errs());
 }

cfe/trunk/test/CodeGen/bitfield-2.c

 // RUN: %clang_cc1 -emit-llvm -triple x86_64 -O3 -o %t.opt.ll %s \
 // RUN:   -fdump-record-layouts > %t.dump.txt
 // RUN: FileCheck -check-prefix=CHECK-RECORD < %t.dump.txt %s
 // RUN: FileCheck -check-prefix=CHECK-OPT < %t.opt.ll %s
 
 /****/
 
 // Check that we don't read off the end a packed 24-bit structure.
 // PR6176
 
 // CHECK-RECORD: *** Dumping IRgen Record Layout
 // CHECK-RECORD: Record: RecordDecl{{.*}}s0
 // CHECK-RECORD: Layout: <CGRecordLayout
 // CHECK-RECORD:   LLVMType:%struct.s0 = type { [3 x i8] }
 // CHECK-RECORD:   IsZeroInitializable:1
 // CHECK-RECORD:   BitFields:[
-// CHECK-RECORD:     <CGBitFieldInfo Offset:0 Size:24 IsSigned:1 StorageSize:24 StorageAlignment:1>
+// CHECK-RECORD:     <CGBitFieldInfo Offset:0 Size:24 IsSigned:1 StorageSize:24 StorageOffset:0>
 struct __attribute((packed)) s0 {
   int f0 : 24;
 };
 
 struct s0 g0 = { 0xdeadbeef };
 
 int f0_load(struct s0 *a0) {
   int size_check[sizeof(struct s0) == 3 ? 1 : -1];

 // PR5591
 
 // CHECK-RECORD: *** Dumping IRgen Record Layout
 // CHECK-RECORD: Record: RecordDecl{{.*}}s1
 // CHECK-RECORD: Layout: <CGRecordLayout
 // CHECK-RECORD:   LLVMType:%struct.s1 = type { [3 x i8] }
 // CHECK-RECORD:   IsZeroInitializable:1
 // CHECK-RECORD:   BitFields:[
-// CHECK-RECORD:     <CGBitFieldInfo Offset:0 Size:10 IsSigned:1 StorageSize:24 StorageAlignment:1>
-// CHECK-RECORD:     <CGBitFieldInfo Offset:10 Size:10 IsSigned:1 StorageSize:24 StorageAlignment:1>
+// CHECK-RECORD:     <CGBitFieldInfo Offset:0 Size:10 IsSigned:1 StorageSize:24 StorageOffset:0>
+// CHECK-RECORD:     <CGBitFieldInfo Offset:10 Size:10 IsSigned:1 StorageSize:24 StorageOffset:0>
 
 #pragma pack(push)
 #pragma pack(1)
 struct __attribute((packed)) s1 {
   signed f0 : 10;
   signed f1 : 10;
 };
 #pragma pack(pop)

 // PR5567
 
 // CHECK-RECORD: *** Dumping IRgen Record Layout
 // CHECK-RECORD: Record: RecordDecl{{.*}}u2
 // CHECK-RECORD: Layout: <CGRecordLayout
 // CHECK-RECORD:   LLVMType:%union.u2 = type { i8 }
 // CHECK-RECORD:   IsZeroInitializable:1
 // CHECK-RECORD:   BitFields:[
-// CHECK-RECORD:     <CGBitFieldInfo Offset:0 Size:3 IsSigned:0 StorageSize:8 StorageAlignment:1>
+// CHECK-RECORD:     <CGBitFieldInfo Offset:0 Size:3 IsSigned:0 StorageSize:8 StorageOffset:0>
 
 union __attribute__((packed)) u2 {
   unsigned long long f0 : 3;
 };
 
 union u2 g2 = { 0xdeadbeef };
 
 int f2_load(union u2 *a0) {

 // Check that we compute the best alignment possible for each access.
 //
 // CHECK-RECORD: *** Dumping IRgen Record Layout
 // CHECK-RECORD: Record: RecordDecl{{.*}}s7
 // CHECK-RECORD: Layout: <CGRecordLayout
 // CHECK-RECORD:   LLVMType:%struct.s7 = type { i32, i32, i32, i8, i32, [12 x i8] }
 // CHECK-RECORD:   IsZeroInitializable:1
 // CHECK-RECORD:   BitFields:[
-// CHECK-RECORD:     <CGBitFieldInfo Offset:0 Size:5 IsSigned:1 StorageSize:8 StorageAlignment:4>
-// CHECK-RECORD:     <CGBitFieldInfo Offset:0 Size:29 IsSigned:1 StorageSize:32 StorageAlignment:16>
+// CHECK-RECORD:     <CGBitFieldInfo Offset:0 Size:5 IsSigned:1 StorageSize:8 StorageOffset:12>
+// CHECK-RECORD:     <CGBitFieldInfo Offset:0 Size:29 IsSigned:1 StorageSize:32 StorageOffset:16>
 
 struct __attribute__((aligned(16))) s7 {
   int a, b, c;
   int f0 : 5;
   int f1 : 29;
 };
 
 int f7_load(struct s7 *a0) {

cfe/trunk/test/CodeGen/packed-nest-unpacked.c

 struct YBitfield {
   char x;
   struct XBitfield y;
 } __attribute((packed));
 struct YBitfield gbitfield;
 
 unsigned test7() {
   // CHECK: @test7
-  // CHECK: load i32, i32* getelementptr inbounds (%struct.YBitfield, %struct.YBitfield* @gbitfield, i32 0, i32 1, i32 0), align 4
+  // CHECK: load i32, i32* getelementptr inbounds (%struct.YBitfield, %struct.YBitfield* @gbitfield, i32 0, i32 1, i32 0), align 1
   return gbitfield.y.b2;
 }
+
+void test8(unsigned x) {
+  // CHECK: @test8
+  // CHECK: load i32, i32* getelementptr inbounds (%struct.YBitfield, %struct.YBitfield* @gbitfield, i32 0, i32 1, i32 0), align 1
+  // CHECK: store i32 {{.*}}, i32* getelementptr inbounds (%struct.YBitfield, %struct.YBitfield* @gbitfield, i32 0, i32 1, i32 0), align 1
+  gbitfield.y.b2 = x;
+}
+
+struct TBitfield
+{
+  long a;
+  char b;
+  unsigned c:15;
+};
+struct TBitfield tbitfield;
+
+unsigned test9() {
+  // CHECK: @test9
+  // CHECK: load i16, i16* getelementptr inbounds (%struct.TBitfield, %struct.TBitfield* @tbitfield, i32 0, i32 2), align 1
+  return tbitfield.c;
+}
+
+void test10(unsigned x) {
+  // CHECK: @test10
+  // CHECK: load i16, i16* getelementptr inbounds (%struct.TBitfield, %struct.TBitfield* @tbitfield, i32 0, i32 2), align 1
+  // CHECK: store i16 {{.*}}, i16* getelementptr inbounds (%struct.TBitfield, %struct.TBitfield* @tbitfield, i32 0, i32 2), align 1
+  tbitfield.c = x;
+}
+