wmi updated this revision to Diff 112271.
wmi added a comment.

Try another idea suggested by David.

All the bitfields in a single run are still wrapped inside of a large integer 
according to CGBitFieldInfo. For the bitfields with legal integer types and 
aligned, change their access manner when we generate load/store in llvm IR for 
bitfield (In EmitLoadOfBitfieldLValue and EmitStoreThroughBitfieldLValue). All 
the other bitfields will still be accessed using widen load/store plus masking 
operations. Here is an example:

class A {

  unsigned long f1:2;
  unsigned long f2:6;
  unsigned long f3:8;
  unsigned long f4:4;

};
A a;

f1, f2, f3 and f4 will still be wrapped as a large integer. f1, f2, f4 will 
have the same access code as before. f3 will be accessed as if it is a separate 
unsigned char variable.

In this way, we can reduce the chance of blocking bitfield access combining. 
a.f1 access and a.f4 access can be combined if only no a.f3 access stands in 
between a.f1 and a.f4.  We will generate two less instructions for foo, and one 
more instruction for goo. So it is better, but not perfect.

void foo (unsigned long n1, unsigned long n2, unsigned long n3) {

  a.f1 = n1;
  a.f4 = n4;
  a.f3 = n3;

}

void goo (unsigned long n1, unsigned long n2, unsigned long n3) {

  a.f1 = n1;
  a.f3 = n3;    // a.f3 will still block the combining of a.f1 and a.f2 because 
a.f3 is accessed independently.
  a.f4 = n4;

}


Repository:
  rL LLVM

https://reviews.llvm.org/D36562

Files:
  lib/CodeGen/CGExpr.cpp
  test/CodeGen/2009-12-07-BitFieldAlignment.c

Index: test/CodeGen/2009-12-07-BitFieldAlignment.c
===================================================================
--- test/CodeGen/2009-12-07-BitFieldAlignment.c
+++ test/CodeGen/2009-12-07-BitFieldAlignment.c
@@ -4,7 +4,7 @@
 struct S {
   int a, b;
   void *c;
-  unsigned d : 8;
+  unsigned d : 7;
   unsigned e : 8;
 };
 
Index: lib/CodeGen/CGExpr.cpp
===================================================================
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -1653,30 +1653,72 @@
   return EmitLoadOfBitfieldLValue(LV, Loc);
 }
 
+/// Check if current Field is better to be accessed separately. When current
+/// field has legal integer width, and its bitfield offset is naturally
+/// aligned, it is better to access the bitfield like a separate integer var.
+static bool IsSeparatableBitfield(const CGBitFieldInfo &Info,
+                                  const llvm::DataLayout &DL,
+                                  ASTContext &Context) {
+  if (!DL.isLegalInteger(Info.Size))
+    return false;
+  // Make sure Field is natually aligned.
+  if (Info.Offset %
+          (DL.getABIIntegerTypeAlignment(Info.Size) * Context.getCharWidth()) !=
+      0)
+    return false;
+  return true;
+}
+
 RValue CodeGenFunction::EmitLoadOfBitfieldLValue(LValue LV,
                                                  SourceLocation Loc) {
   const CGBitFieldInfo &Info = LV.getBitFieldInfo();
 
   // Get the output type.
   llvm::Type *ResLTy = ConvertType(LV.getType());
 
   Address Ptr = LV.getBitFieldAddress();
-  llvm::Value *Val = Builder.CreateLoad(Ptr, LV.isVolatileQualified(), "bf.load");
-
-  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");
+  llvm::Value *Val = nullptr;
+  if (IsSeparatableBitfield(Info, CGM.getDataLayout(), getContext())) {
+    // Ptr is the pointer to the Bitfield Storage. Add Offset to the Ptr
+    // if the Offset is not zero.
+    if (Info.Offset != 0) {
+      Address BC = Builder.CreateBitCast(Ptr, Int8PtrTy);
+      llvm::Value *GEP = Builder.CreateGEP(
+          BC.getPointer(),
+          llvm::ConstantInt::get(Int32Ty,
+                                 Info.Offset / getContext().getCharWidth()));
+      Ptr = Address(GEP, Ptr.getAlignment());
+    }
+    // Adjust the element type of Ptr if it has different size with Info.Size.
+    if (Ptr.getElementType()->getScalarSizeInBits() != Info.Size) {
+      llvm::Type *BFType = llvm::Type::getIntNTy(getLLVMContext(), Info.Size);
+      llvm::Type *BFTypePtr =
+          llvm::Type::getIntNPtrTy(getLLVMContext(), Info.Size);
+      unsigned Align = CGM.getDataLayout().getABITypeAlignment(BFType) *
+                       getContext().getCharWidth();
+      Ptr = Builder.CreateBitCast(
+          Address(Ptr.getPointer(), getContext().toCharUnitsFromBits(Align)),
+          BFTypePtr);
+    }
+    Val = Builder.CreateLoad(Ptr, LV.isVolatileQualified(), "bf.load");
   } else {
-    if (Info.Offset)
-      Val = Builder.CreateLShr(Val, Info.Offset, "bf.lshr");
-    if (static_cast<unsigned>(Info.Offset) + Info.Size < Info.StorageSize)
-      Val = Builder.CreateAnd(Val, llvm::APInt::getLowBitsSet(Info.StorageSize,
-                                                              Info.Size),
-                              "bf.clear");
+    Val = Builder.CreateLoad(Ptr, LV.isVolatileQualified(), "bf.load");
+    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");
+    } else {
+      if (Info.Offset)
+        Val = Builder.CreateLShr(Val, Info.Offset, "bf.lshr");
+      if (static_cast<unsigned>(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");
   EmitScalarRangeCheck(Val, LV.getType(), Loc);
@@ -1866,15 +1908,43 @@
 
   // Get the source value, truncated to the width of the bit-field.
   llvm::Value *SrcVal = Src.getScalarVal();
+  llvm::Value *MaskedVal;
 
-  // Cast the source to the storage type and shift it into place.
-  SrcVal = Builder.CreateIntCast(SrcVal, Ptr.getElementType(),
-                                 /*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) {
+  bool SeparatableBitfield =
+      IsSeparatableBitfield(Info, CGM.getDataLayout(), getContext());
+  if (SeparatableBitfield) {
+    // Ptr is the pointer to the Bitfield Storage. Add Offset to the Ptr
+    // if the Offset is not zero.
+    if (Info.Offset != 0) {
+      Address BC = Builder.CreateBitCast(Ptr, Int8PtrTy);
+      llvm::Value *GEP = Builder.CreateGEP(
+          BC.getPointer(),
+          llvm::ConstantInt::get(Int32Ty,
+                                 Info.Offset / getContext().getCharWidth()));
+      Ptr = Address(GEP, Ptr.getAlignment());
+    }
+    // Adjust the element type of Ptr if it has different size with Info.Size.
+    if (Ptr.getElementType()->getScalarSizeInBits() != Info.Size) {
+      llvm::Type *BFType = llvm::Type::getIntNTy(getLLVMContext(), Info.Size);
+      llvm::Type *BFTypePtr =
+          llvm::Type::getIntNPtrTy(getLLVMContext(), Info.Size);
+      unsigned Align = CGM.getDataLayout().getABITypeAlignment(BFType) *
+                       getContext().getCharWidth();
+      Ptr = Builder.CreateBitCast(
+          Address(Ptr.getPointer(), getContext().toCharUnitsFromBits(Align)),
+          BFTypePtr);
+    }
+    // Cast the source to the storage type and shift it into place.
+    SrcVal = Builder.CreateIntCast(SrcVal, Ptr.getElementType(),
+                                   /*IsSigned=*/false);
+    MaskedVal = SrcVal;
+  } else if (Info.StorageSize != Info.Size) {
+    // Cast the source to the storage type and shift it into place.
+    SrcVal = Builder.CreateIntCast(SrcVal, Ptr.getElementType(),
+                                   /*IsSigned=*/false);
+
+    // See if there are other bits in the bitfield's storage we'll need to load
+    // and mask together with source before storing.
     assert(Info.StorageSize > Info.Size && "Invalid bitfield size.");
     llvm::Value *Val =
       Builder.CreateLoad(Ptr, Dst.isVolatileQualified(), "bf.load");
@@ -1900,6 +1970,10 @@
     SrcVal = Builder.CreateOr(Val, SrcVal, "bf.set");
   } else {
     assert(Info.Offset == 0);
+    // Cast the source to the storage type and shift it into place.
+    SrcVal = Builder.CreateIntCast(SrcVal, Ptr.getElementType(),
+                                   /*IsSigned=*/false);
+    MaskedVal = SrcVal;
   }
 
   // Write the new value back out.
@@ -1910,7 +1984,7 @@
     llvm::Value *ResultVal = MaskedVal;
 
     // Sign extend the value if needed.
-    if (Info.IsSigned) {
+    if (Info.IsSigned && !SeparatableBitfield) {
       assert(Info.Size <= Info.StorageSize);
       unsigned HighBits = Info.StorageSize - Info.Size;
       if (HighBits) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to