]> granicus.if.org Git - clang/commitdiff
IRgen: (Reapply 101222, with fixes) Move EmitStoreThroughBitfieldLValue to use new...
authorDaniel Dunbar <daniel@zuster.org>
Thu, 15 Apr 2010 03:47:33 +0000 (03:47 +0000)
committerDaniel Dunbar <daniel@zuster.org>
Thu, 15 Apr 2010 03:47:33 +0000 (03:47 +0000)
 - 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.

The new fixes from r101222 are:

 1. The shift to the target position needs to occur after the value is extended to the correct size. This broke Clang bootstrap, among other things no doubt.

 2. Swap the order of arguments to OR, to get a tad more constant folding.

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

lib/CodeGen/CGExpr.cpp
test/CodeGen/bitfield-2.c [new file with mode: 0644]

index 6a35335c8f15d9132d4820ba1fb0b51a20ef287d..d338a49d2c4276a42887d335f3bd73ef2ac52d5b 100644 (file)
@@ -600,21 +600,6 @@ RValue CodeGenFunction::EmitLoadOfLValue(LValue LV, QualType ExprType) {
   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 @@ RValue CodeGenFunction::EmitLoadOfBitfieldLValue(LValue LV,
     // 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 @@ RValue CodeGenFunction::EmitLoadOfBitfieldLValue(LValue LV,
   // 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,99 @@ void CodeGenFunction::EmitStoreThroughBitfieldLValue(RValue Src, LValue Dst,
                                                      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 output type.
+  const llvm::Type *ResLTy = ConvertType(Ty);
+  unsigned ResSizeInBits = CGM.getTargetData().getTypeSizeInBits(ResLTy);
 
-  // Get the new value, cast to the appropriate type and masked to exactly the
-  // size of the bit-field.
+  // 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;
-  }
-
-  // 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");
-
-    // Write back.
-    Builder.CreateStore(HighVal, HighPtr, Dst.isVolatileQualified());
+    *Result = ReloadVal;
+  }
+
+  // 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();
+
+    // 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, 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.
+    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);
+
+    // 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());
+      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(Builder.CreateAnd(Load, InvMask), Val);
+    }
+
+    // Write the value.
+    llvm::StoreInst *Store = Builder.CreateStore(Val, Ptr,
+                                                 Dst.isVolatileQualified());
+    if (AI.AccessAlignment)
+      Store->setAlignment(AI.AccessAlignment);
   }
 }
 
diff --git a/test/CodeGen/bitfield-2.c b/test/CodeGen/bitfield-2.c
new file mode 100644 (file)
index 0000000..79033d7
--- /dev/null
@@ -0,0 +1,207 @@
+// 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;
+}
+
+/***/
+
+
+struct s5 {
+  unsigned f0 : 2;
+  _Bool f1 : 1;
+  _Bool f2 : 1;
+};
+
+struct s5 g5 = { 0xdeadbeef, 0xdeadbeef };
+
+int f5_load(struct s5 *a0) {
+  return a0->f0 ^ a0->f1;
+}
+int f5_store(struct s5 *a0) {
+  return (a0->f0 = 0xF) ^ (a0->f1 = 0xF) ^ (a0->f2 = 0xF);
+}
+int f5_reload(struct s5 *a0) {
+  return (a0->f0 += 0xF) ^ (a0->f1 += 0xF) ^ (a0->f2 += 0xF);
+}
+
+// CHECK-OPT: define i64 @test_5()
+// CHECK-OPT:  ret i64 2
+// CHECK-OPT: }
+unsigned long long test_5() {
+  struct s5 g5 = { 0xdeadbeef, 0xdeadbeef, 0xdeadbeef };
+  unsigned long long res = 0;
+  res ^= g5.f0 ^ g5.f1 ^ g5.f2;
+  res ^= f5_load(&g5) ^ f5_store(&g5) ^ f5_reload(&g5);
+  res ^= g5.f0 ^ g5.f1 ^ g5.f2;
+  return res;
+}