diff --git a/gen/dvalue.cpp b/gen/dvalue.cpp index 2673f891d86..2debc5c4054 100644 --- a/gen/dvalue.cpp +++ b/gen/dvalue.cpp @@ -183,6 +183,7 @@ DRValue *DBitFieldLValue::getRVal() { const auto sizeInBits = intType->getBitWidth(); const auto ptr = val; LLValue *v = gIR->ir->CreateAlignedLoad(intType, ptr, llvm::MaybeAlign(1)); + // TODO: byte-swap v for big-endian targets? if (bf->type->isunsigned()) { if (auto n = bf->bitOffset) @@ -212,6 +213,7 @@ void DBitFieldLValue::store(LLValue *value) { llvm::APInt::getLowBitsSet(intType->getBitWidth(), bf->fieldWidth); const auto oldVal = gIR->ir->CreateAlignedLoad(intType, ptr, llvm::MaybeAlign(1)); + // TODO: byte-swap oldVal for big-endian targets? const auto maskedOldVal = gIR->ir->CreateAnd(oldVal, ~(mask << bf->bitOffset)); @@ -221,6 +223,7 @@ void DBitFieldLValue::store(LLValue *value) { bfVal = gIR->ir->CreateShl(bfVal, n); const auto newVal = gIR->ir->CreateOr(maskedOldVal, bfVal); + // TODO: byte-swap newVal for big-endian targets? gIR->ir->CreateAlignedStore(newVal, ptr, llvm::MaybeAlign(1)); } diff --git a/gen/toir.cpp b/gen/toir.cpp index 003143f142e..b53c097fca0 100644 --- a/gen/toir.cpp +++ b/gen/toir.cpp @@ -139,7 +139,7 @@ static void write_struct_literal(Loc loc, LLValue *mem, StructDeclaration *sd, // get a pointer to this group's IR field const auto ptr = DtoLVal(DtoIndexAggregate(mem, sd, vd)); - // merge all initializers to a single value + // merge all initializers to a single integer value const auto intType = LLIntegerType::get(gIR->context(), group.sizeInBytes * 8); LLValue *val = LLConstant::getNullValue(intType); @@ -165,6 +165,7 @@ static void write_struct_literal(Loc loc, LLValue *mem, StructDeclaration *sd, } IF_LOG Logger::cout() << "merged IR value: " << *val << '\n'; + // TODO: byte-swap val for big-endian targets? gIR->ir->CreateAlignedStore(val, ptr, llvm::MaybeAlign(1)); offset += group.sizeInBytes; diff --git a/ir/iraggr.cpp b/ir/iraggr.cpp index e99e3944a3f..2a9694f4d45 100644 --- a/ir/iraggr.cpp +++ b/ir/iraggr.cpp @@ -257,6 +257,8 @@ void IrAggr::addFieldInitializers( const VarInitMap &explicitInitializers, AggregateDeclaration *decl, unsigned &offset, unsigned &interfaceVtblIndex, bool &isPacked) { + using llvm::APInt; + if (ClassDeclaration *cd = decl->isClassDeclaration()) { if (cd->baseClass) { addFieldInitializers(constants, explicitInitializers, cd->baseClass, @@ -302,6 +304,9 @@ void IrAggr::addFieldInitializers( : getDefaultInitializer(field); }; + // IR field index => APInt for bitfield group + std::unordered_map bitFieldGroupConstants; + const auto addToBitFieldGroup = [&](BitFieldDeclaration *bf, unsigned fieldIndex, unsigned bitOffset) { LLConstant *init = getFieldInit(bf); @@ -310,20 +315,24 @@ void IrAggr::addFieldInitializers( return; } - LLConstant *&constant = constants[baseLLFieldIndex + fieldIndex]; + if (bitFieldGroupConstants.find(fieldIndex) == + bitFieldGroupConstants.end()) { + const auto fieldType = + llvm::cast(b.defaultTypes()[fieldIndex]); // an i8 array + const auto bitFieldSize = fieldType->getNumElements(); + bitFieldGroupConstants.emplace(fieldIndex, APInt(bitFieldSize * 8, 0)); + } + + APInt &constant = bitFieldGroupConstants[fieldIndex]; + const auto intSizeInBits = constant.getBitWidth(); - using llvm::APInt; - const auto fieldType = b.defaultTypes()[fieldIndex]; - const auto intSizeInBits = fieldType->getIntegerBitWidth(); - const APInt oldVal = - constant ? constant->getUniqueInteger() : APInt(intSizeInBits, 0); + const APInt oldVal = constant; const APInt bfVal = init->getUniqueInteger().zextOrTrunc(intSizeInBits); const APInt mask = APInt::getLowBitsSet(intSizeInBits, bf->fieldWidth) << bitOffset; assert(!oldVal.intersects(mask) && "has masked bits set already"); - const APInt newVal = oldVal | ((bfVal << bitOffset) & mask); - constant = LLConstant::getIntegerValue(fieldType, newVal); + constant = oldVal | ((bfVal << bitOffset) & mask); }; // add explicit and non-overlapping implicit initializers @@ -333,7 +342,7 @@ void IrAggr::addFieldInitializers( const auto fieldIndex = pair.second; if (auto bf = field->isBitFieldDeclaration()) { - // multiple bit fields can map to a single IR field (of integer type) + // multiple bit fields can map to a single IR field for the whole group addToBitFieldGroup(bf, fieldIndex, bf->bitOffset); } else { LLConstant *&constant = constants[baseLLFieldIndex + fieldIndex]; @@ -356,6 +365,25 @@ void IrAggr::addFieldInitializers( (bf->offset - primary->offset) * 8 + bf->bitOffset); } + for (const auto &pair : bitFieldGroupConstants) { + const unsigned fieldIndex = pair.first; + const APInt intValue = pair.second; + + LLConstant *&constant = constants[baseLLFieldIndex + fieldIndex]; + assert(!constant && "already have a constant for a bitfield group?"); + + // convert APInt to i8 array + const auto i8Type = getI8Type(); + const auto numBytes = intValue.getBitWidth() / 8; + std::vector bytes; + for (unsigned i = 0; i < numBytes; ++i) { + APInt byteVal = intValue.extractBits(8, i * 8); + bytes.push_back(LLConstant::getIntegerValue(i8Type, byteVal)); + } + + constant = llvm::ConstantArray::get(LLArrayType::get(i8Type, numBytes), bytes); + } + // TODO: sanity check that all explicit initializers have been dealt with? // (potential issue for bitfields in unions...) diff --git a/ir/irtypeaggr.cpp b/ir/irtypeaggr.cpp index 21c1e9a10b3..3f49cc7c65e 100644 --- a/ir/irtypeaggr.cpp +++ b/ir/irtypeaggr.cpp @@ -146,8 +146,9 @@ void AggrTypeBuilder::addAggregate( const uint64_t f_begin = field->offset; const uint64_t f_end = f_begin + fieldSize; + // use an i8 array for bit field groups const auto llType = - isBitField ? LLIntegerType::get(gIR->context(), fieldSize * 8) + isBitField ? llvm::ArrayType::get(getI8Type(), fieldSize) : DtoMemType(field->type); // check for overlap with existing fields (on a byte level, not bits) @@ -212,12 +213,8 @@ void AggrTypeBuilder::addAggregate( fieldSize = af.size; } else { fieldAlignment = getABITypeAlign(llType); - if (vd->isBitFieldDeclaration()) { - fieldSize = af.size; // an IR integer of possibly non-power-of-2 size - } else { - fieldSize = getTypeAllocSize(llType); - assert(fieldSize <= af.size); - } + fieldSize = getTypeAllocSize(llType); + assert(fieldSize <= af.size); } // advance offset to right past this field diff --git a/tests/compilable/gh4646.d b/tests/compilable/gh4646.d new file mode 100644 index 00000000000..686f285061b --- /dev/null +++ b/tests/compilable/gh4646.d @@ -0,0 +1,31 @@ +// RUN: %ldc -c -preview=bitfields %s + +struct BitField { + uint _0 : 1; + uint _1 : 1; + uint _2 : 1; + uint _3 : 1; + uint _4 : 1; + uint _5 : 1; + uint _6 : 1; + uint _7 : 1; + uint _8 : 1; + uint _9 : 1; + uint _10 : 1; + uint _11 : 1; + uint _12 : 1; + uint _13 : 1; + uint _14 : 1; + uint _15 : 1; + uint _16 : 1; +} + +static assert(BitField.sizeof == 4); +static assert(BitField.alignof == 4); + +struct Foo { + BitField bf; +} + +static assert(Foo.sizeof == 4); +static assert(Foo.alignof == 4);