From: Daniel Dunbar Date: Wed, 14 Apr 2010 04:08:03 +0000 (+0000) Subject: IRgen: Move EmitStoreThroughBitfieldLValue to use new CGBitfieldInfo::AccessInfo... X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=89cdaa94ecac5e83197c5889830ee37ff1faea58;p=clang 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. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@101222 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/CodeGen/CGExpr.cpp b/lib/CodeGen/CGExpr.cpp index 6a35335c8f..3a224ce1b4 100644 --- a/lib/CodeGen/CGExpr.cpp +++ b/lib/CodeGen/CGExpr.cpp @@ -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(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,97 @@ 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(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 = 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"); } - *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()); + // 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); } } diff --git a/test/CodeGen/bitfield-2.c b/test/CodeGen/bitfield-2.c new file mode 100644 index 0000000000..258f4177f8 --- /dev/null +++ b/test/CodeGen/bitfield-2.c @@ -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; +} + +/***/