[cfe-commits] r101222 - in /cfe/trunk: lib/CodeGen/CGExpr.cpp test/CodeGen/bitfield-2.c

Daniel Dunbar daniel at zuster.org
Tue Apr 13 21:08:03 PDT 2010


Author: ddunbar
Date: Tue Apr 13 23:08:03 2010
New Revision: 101222

URL: http://llvm.org/viewvc/llvm-project?rev=101222&view=rev
Log:
IRgen: Move EmitStoreThroughBitfieldLValue to use new CGBitfieldInfo::AccessInfo decomposition, instead of computing the access policy itself.
 - Sadly, this doesn't seem to give any .ll size win so far. It is possible to make this routine significantly smarter & avoid various shifting, masking, and zext/sext, but I'm not really convinced it is worth it. It is tricky, and this is really instcombine's job.

 - No intended functionality change; the test case is just to increase coverage & serves as a demo file, it worked before this commit.

Added:
    cfe/trunk/test/CodeGen/bitfield-2.c
Modified:
    cfe/trunk/lib/CodeGen/CGExpr.cpp

Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=101222&r1=101221&r2=101222&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGExpr.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp Tue Apr 13 23:08:03 2010
@@ -600,21 +600,6 @@
   return EmitLoadOfKVCRefLValue(LV, ExprType);
 }
 
-static llvm::Value *getBitFieldAddr(LValue LV, CGBuilderTy &Builder) {
-  const CGBitFieldInfo &Info = LV.getBitFieldInfo();
-
-  llvm::Value *BaseValue = LV.getBitFieldBaseAddr();
-  const llvm::PointerType *BaseTy =
-    cast<llvm::PointerType>(BaseValue->getType());
-
-  // Cast to the type of the access we will perform.
-  llvm::Value *V = Builder.CreateBitCast(
-    BaseValue, llvm::PointerType::get(Info.FieldTy, BaseTy->getAddressSpace()));
-
-  // Offset by the access index.
-  return Builder.CreateConstGEP1_32(V, Info.FieldNo);
-}
-
 RValue CodeGenFunction::EmitLoadOfBitfieldLValue(LValue LV,
                                                  QualType ExprType) {
   const CGBitFieldInfo &Info = LV.getBitFieldInfo();
@@ -656,7 +641,7 @@
     // Shift out unused low bits and mask out unused high bits.
     llvm::Value *Val = Load;
     if (AI.FieldBitStart)
-      Val = Builder.CreateAShr(Load, AI.FieldBitStart);
+      Val = Builder.CreateLShr(Load, AI.FieldBitStart);
     Val = Builder.CreateAnd(Val, llvm::APInt::getLowBitsSet(AI.AccessWidth,
                                                             AI.TargetBitWidth),
                             "bf.clear");
@@ -678,7 +663,7 @@
   // 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.Size;
+    unsigned ExtraBits = ResSizeInBits - Info.getSize();
     if (ExtraBits)
       Res = Builder.CreateAShr(Builder.CreateShl(Res, ExtraBits),
                                ExtraBits, "bf.val.sext");
@@ -805,88 +790,97 @@
                                                      QualType Ty,
                                                      llvm::Value **Result) {
   const CGBitFieldInfo &Info = Dst.getBitFieldInfo();
-  unsigned StartBit = Info.Start;
-  unsigned BitfieldSize = Info.Size;
-  llvm::Value *Ptr = getBitFieldAddr(Dst, Builder);
-
-  const llvm::Type *EltTy =
-    cast<llvm::PointerType>(Ptr->getType())->getElementType();
-  unsigned EltTySize = CGM.getTargetData().getTypeSizeInBits(EltTy);
 
-  // Get the new value, cast to the appropriate type and masked to exactly the
-  // size of the bit-field.
+  // Get the output type.
+  const llvm::Type *ResLTy = ConvertType(Ty);
+  unsigned ResSizeInBits = CGM.getTargetData().getTypeSizeInBits(ResLTy);
+
+  // Get the source value, truncated to the width of the bit-field.
   llvm::Value *SrcVal = Src.getScalarVal();
-  llvm::Value *NewVal = Builder.CreateIntCast(SrcVal, EltTy, false, "tmp");
-  llvm::Constant *Mask = llvm::ConstantInt::get(VMContext,
-                           llvm::APInt::getLowBitsSet(EltTySize, BitfieldSize));
-  NewVal = Builder.CreateAnd(NewVal, Mask, "bf.value");
+  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.
-    const llvm::Type *SrcTy = SrcVal->getType();
-    llvm::Value *SrcTrunc = Builder.CreateIntCast(NewVal, SrcTy, false,
-                                                  "bf.reload.val");
+    const 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 SrcTySize = CGM.getTargetData().getTypeSizeInBits(SrcTy);
-      llvm::Value *ExtraBits = llvm::ConstantInt::get(SrcTy,
-                                                      SrcTySize - BitfieldSize);
-      SrcTrunc = Builder.CreateAShr(Builder.CreateShl(SrcTrunc, ExtraBits),
-                                    ExtraBits, "bf.reload.sext");
+    if (Info.isSigned()) {
+      unsigned ExtraBits = ResSizeInBits - Info.getSize();
+      if (ExtraBits)
+        ReloadVal = Builder.CreateAShr(Builder.CreateShl(ReloadVal, ExtraBits),
+                                       ExtraBits, "bf.reload.sext");
     }
 
-    *Result = SrcTrunc;
+    *Result = ReloadVal;
   }
 
-  // In some cases the bitfield may straddle two memory locations.  Emit the low
-  // part first and check to see if the high needs to be done.
-  unsigned LowBits = std::min(BitfieldSize, EltTySize - StartBit);
-  llvm::Value *LowVal = Builder.CreateLoad(Ptr, Dst.isVolatileQualified(),
-                                           "bf.prev.low");
-
-  // Compute the mask for zero-ing the low part of this bitfield.
-  llvm::Constant *InvMask =
-    llvm::ConstantInt::get(VMContext,
-             ~llvm::APInt::getBitsSet(EltTySize, StartBit, StartBit + LowBits));
-
-  // Compute the new low part as
-  //   LowVal = (LowVal & InvMask) | (NewVal << StartBit),
-  // with the shift of NewVal implicitly stripping the high bits.
-  llvm::Value *NewLowVal =
-    Builder.CreateShl(NewVal, StartBit, "bf.value.lo");
-  LowVal = Builder.CreateAnd(LowVal, InvMask, "bf.prev.lo.cleared");
-  LowVal = Builder.CreateOr(LowVal, NewLowVal, "bf.new.lo");
-
-  // Write back.
-  Builder.CreateStore(LowVal, Ptr, Dst.isVolatileQualified());
-
-  // If the low part doesn't cover the bitfield emit a high part.
-  if (LowBits < BitfieldSize) {
-    unsigned HighBits = BitfieldSize - LowBits;
-    llvm::Value *HighPtr =  Builder.CreateGEP(Ptr, llvm::ConstantInt::get(
-                            llvm::Type::getInt32Ty(VMContext), 1), "bf.ptr.hi");
-    llvm::Value *HighVal = Builder.CreateLoad(HighPtr,
-                                              Dst.isVolatileQualified(),
-                                              "bf.prev.hi");
-
-    // Compute the mask for zero-ing the high part of this bitfield.
-    llvm::Constant *InvMask =
-      llvm::ConstantInt::get(VMContext, ~llvm::APInt::getLowBitsSet(EltTySize,
-                               HighBits));
-
-    // Compute the new high part as
-    //   HighVal = (HighVal & InvMask) | (NewVal lshr LowBits),
-    // where the high bits of NewVal have already been cleared and the
-    // shift stripping the low bits.
-    llvm::Value *NewHighVal =
-      Builder.CreateLShr(NewVal, LowBits, "bf.value.high");
-    HighVal = Builder.CreateAnd(HighVal, InvMask, "bf.prev.hi.cleared");
-    HighVal = Builder.CreateOr(HighVal, NewHighVal, "bf.new.hi");
+  // 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);
+
+    // Get the field pointer.
+    llvm::Value *Ptr = Dst.getBitFieldBaseAddr();
 
-    // Write back.
-    Builder.CreateStore(HighVal, HighPtr, Dst.isVolatileQualified());
+    // 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) {
+      const llvm::Type *i8PTy = llvm::Type::getInt8PtrTy(VMContext);
+      Ptr = Builder.CreateBitCast(Ptr, i8PTy);
+      Ptr = Builder.CreateConstGEP1_32(Ptr, AI.FieldByteOffset,"bf.field.offs");
+    }
+
+    // Cast to the access type.
+    const llvm::Type *PTy = llvm::Type::getIntNPtrTy(VMContext, AI.AccessWidth,
+                                                     Ty.getAddressSpace());
+    Ptr = Builder.CreateBitCast(Ptr, PTy);
+
+    // Extract the piece of the bit-field value to write in this access, shifted
+    // and masked for placement into memory.
+    llvm::Value *Val = SrcVal;
+    if (AI.TargetBitOffset)
+      Val = Builder.CreateLShr(Val, AI.TargetBitOffset);
+    Val = Builder.CreateAnd(Val, llvm::APInt::getLowBitsSet(ResSizeInBits,
+                                                            AI.TargetBitWidth));
+    if (AI.FieldBitStart)
+      Val = Builder.CreateShl(Val, AI.FieldBitStart);
+
+    // Extend or truncate to the access size.
+    const llvm::Type *AccessLTy =
+      llvm::Type::getIntNTy(VMContext, AI.AccessWidth);
+    if (ResSizeInBits < AI.AccessWidth)
+      Val = Builder.CreateZExt(Val, AccessLTy);
+    else if (ResSizeInBits > AI.AccessWidth)
+      Val = Builder.CreateTrunc(Val, AccessLTy);
+
+    // 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());
+      if (AI.AccessAlignment)
+        Load->setAlignment(AI.AccessAlignment);
+
+      // 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(Val, Builder.CreateAnd(Load, InvMask));
+    }
+
+    // Write the value.
+    llvm::StoreInst *Store = Builder.CreateStore(Val, Ptr,
+                                                 Dst.isVolatileQualified());
+    if (AI.AccessAlignment)
+      Store->setAlignment(AI.AccessAlignment);
   }
 }
 

Added: cfe/trunk/test/CodeGen/bitfield-2.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/bitfield-2.c?rev=101222&view=auto
==============================================================================
--- cfe/trunk/test/CodeGen/bitfield-2.c (added)
+++ cfe/trunk/test/CodeGen/bitfield-2.c Tue Apr 13 23:08:03 2010
@@ -0,0 +1,176 @@
+// RUN: %clang_cc1 -emit-llvm -triple x86_64 -O3 -o - %s | \
+// RUN:   FileCheck -check-prefix=CHECK-OPT %s
+
+/****/
+
+// PR6176
+
+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];
+  return a0->f0;
+}
+int f0_store(struct s0 *a0) {
+  return (a0->f0 = 1);
+}
+int f0_reload(struct s0 *a0) {
+  return (a0->f0 += 1);
+}
+
+// CHECK-OPT: define i64 @test_0()
+// CHECK-OPT:  ret i64 1
+// CHECK-OPT: }
+unsigned long long test_0() {
+  struct s0 g0 = { 0xdeadbeef };
+  unsigned long long res = 0;
+  res ^= g0.f0;
+  res ^= f0_load(&g0) ^ f0_store(&g0) ^ f0_reload(&g0);
+  res ^= g0.f0;
+  return res;
+}
+
+/****/
+
+// PR5591
+
+#pragma pack(push)
+#pragma pack(1)
+struct __attribute((packed)) s1 {
+  signed f0 : 10;
+  signed f1 : 10;
+};
+#pragma pack(pop)
+
+struct s1 g1 = { 0xdeadbeef, 0xdeadbeef };
+
+int f1_load(struct s1 *a0) {
+  int size_check[sizeof(struct s1) == 3 ? 1 : -1];
+  return a0->f1;
+}
+int f1_store(struct s1 *a0) {
+  return (a0->f1 = 1234);
+}
+int f1_reload(struct s1 *a0) {
+  return (a0->f1 += 1234);
+}
+
+// CHECK-OPT: define i64 @test_1()
+// CHECK-OPT:  ret i64 210
+// CHECK-OPT: }
+unsigned long long test_1() {
+  struct s1 g1 = { 0xdeadbeef, 0xdeadbeef };
+  unsigned long long res = 0;
+  res ^= g1.f0 ^ g1.f1;
+  res ^= f1_load(&g1) ^ f1_store(&g1) ^ f1_reload(&g1);
+  res ^= g1.f0 ^ g1.f1;
+  return res;
+}
+
+/****/
+
+// PR5567
+
+union u2 {
+  unsigned long long f0 : 3;
+};
+
+union u2 g2 = { 0xdeadbeef };
+
+int f2_load(union u2 *a0) {
+  return a0->f0;
+}
+int f2_store(union u2 *a0) {
+  return (a0->f0 = 1234);
+}
+int f2_reload(union u2 *a0) {
+  return (a0->f0 += 1234);
+}
+
+// CHECK-OPT: define i64 @test_2()
+// CHECK-OPT:  ret i64 2
+// CHECK-OPT: }
+unsigned long long test_2() {
+  union u2 g2 = { 0xdeadbeef };
+  unsigned long long res = 0;
+  res ^= g2.f0;
+  res ^= f2_load(&g2) ^ f2_store(&g2) ^ f2_reload(&g2);
+  res ^= g2.f0;
+  return res;
+}
+
+/***/
+
+// PR5039
+
+struct s3 {
+  long long f0 : 32;
+  long long f1 : 32;
+};
+
+struct s3 g3 = { 0xdeadbeef, 0xdeadbeef };
+
+int f3_load(struct s3 *a0) {
+  a0->f0 = 1;
+  return a0->f0;
+}
+int f3_store(struct s3 *a0) {
+  a0->f0 = 1;
+  return (a0->f0 = 1234);
+}
+int f3_reload(struct s3 *a0) {
+  a0->f0 = 1;
+  return (a0->f0 += 1234);
+}
+
+// CHECK-OPT: define i64 @test_3()
+// CHECK-OPT:  ret i64 -559039940
+// CHECK-OPT: }
+unsigned long long test_3() {
+  struct s3 g3 = { 0xdeadbeef, 0xdeadbeef };
+  unsigned long long res = 0;
+  res ^= g3.f0 ^ g3.f1;
+  res ^= f3_load(&g3) ^ f3_store(&g3) ^ f3_reload(&g3);
+  res ^= g3.f0 ^ g3.f1;
+  return res;
+}
+
+/***/
+
+// This is a case where the bitfield access will straddle an alignment boundary
+// of its underlying type.
+
+struct s4 {
+  unsigned f0 : 16;
+  unsigned f1 : 28 __attribute__ ((packed));
+};
+
+struct s4 g4 = { 0xdeadbeef, 0xdeadbeef };
+
+int f4_load(struct s4 *a0) {
+  return a0->f0 ^ a0->f1;
+}
+int f4_store(struct s4 *a0) {
+  return (a0->f0 = 1234) ^ (a0->f1 = 5678);
+}
+int f4_reload(struct s4 *a0) {
+  return (a0->f0 += 1234) ^ (a0->f1 += 5678);
+}
+
+// CHECK-OPT: define i64 @test_4()
+// CHECK-OPT:  ret i64 4860
+// CHECK-OPT: }
+unsigned long long test_4() {
+  struct s4 g4 = { 0xdeadbeef, 0xdeadbeef };
+  unsigned long long res = 0;
+  res ^= g4.f0 ^ g4.f1;
+  res ^= f4_load(&g4) ^ f4_store(&g4) ^ f4_reload(&g4);
+  res ^= g4.f0 ^ g4.f1;
+  return res;
+}
+
+/***/





More information about the cfe-commits mailing list