Skip to content

Commit

Permalink
Partial revert of d16fba3 to fix ldc-developers#4719 and fix ldc-deve…
Browse files Browse the repository at this point in the history
  • Loading branch information
JohanEngelen committed Aug 20, 2024
1 parent 8d551f7 commit 3591a77
Show file tree
Hide file tree
Showing 5 changed files with 94 additions and 6 deletions.
41 changes: 36 additions & 5 deletions ir/irtypeaggr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,7 @@ AggrTypeBuilder::AggrTypeBuilder(unsigned offset) : m_offset(offset) {
void AggrTypeBuilder::addType(llvm::Type *type, unsigned size) {
const unsigned fieldAlignment = getABITypeAlign(type);
assert(fieldAlignment);
// If the field offset does not have natural alignment, mark the aggregate as
// packed for IR.
if ((m_offset & (fieldAlignment - 1)) != 0) {
m_packed = true;
}
assert((m_offset & (fieldAlignment - 1)) == 0 && "Field is misaligned");
m_defaultTypes.push_back(type);
m_offset += size;
m_fieldIndex++;
Expand Down Expand Up @@ -279,6 +275,41 @@ IrTypeAggr::IrTypeAggr(AggregateDeclaration *ad)
LLStructType::create(gIR->context(), ad->toPrettyChars())),
aggr(ad) {}

bool IrTypeAggr::isPacked(AggregateDeclaration *ad) {
// If the aggregate's size is unknown, any field with type alignment > 1 will
// make it packed.
unsigned aggregateSize = ~0u;
unsigned aggregateAlignment = 1;
if (ad->sizeok == Sizeok::done) {
aggregateSize = ad->structsize;
aggregateAlignment = ad->alignsize;

if (auto sd = ad->isStructDeclaration()) {
if (!sd->alignment.isDefault() && !sd->alignment.isPack())
aggregateAlignment = sd->alignment.get();
}
}

// Classes apparently aren't padded; their size may not match the alignment.
assert((ad->isClassDeclaration() ||
(aggregateSize & (aggregateAlignment - 1)) == 0) &&
"Size not a multiple of alignment?");

// For unions, only a subset of the fields are actually used for the IR type -
// don't care (about a few potentially needlessly packed IR structs).
for (const auto field : ad->fields) {
// The aggregate size, aggregate alignment and the field offset need to be
// multiples of the field type's alignment, otherwise the aggregate type is
// unnaturally aligned, and LLVM would insert padding.
const unsigned fieldTypeAlignment = DtoAlignment(field->type);
const auto mask = fieldTypeAlignment - 1;
if ((aggregateSize | aggregateAlignment | field->offset) & mask)
return true;
}

return false;
}

unsigned IrTypeAggr::getMemberLocation(VarDeclaration *var, bool& isFieldIdx) const {
// Note: The interface is a bit more general than what we actually return.
// Specifically, the frontend offset information we use for overlapping
Expand Down
4 changes: 4 additions & 0 deletions ir/irtypeaggr.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,10 @@ class IrTypeAggr : public IrType {
///
explicit IrTypeAggr(AggregateDeclaration *ad);

/// Returns true, if the LLVM struct type for the aggregate must be declared
/// as packed.
static bool isPacked(AggregateDeclaration *ad);

/// AggregateDeclaration this type represents.
AggregateDeclaration *aggr = nullptr;

Expand Down
3 changes: 2 additions & 1 deletion ir/irtypestruct.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,8 @@ IrTypeStruct *IrTypeStruct::get(StructDeclaration *sd) {
AggrTypeBuilder builder;
builder.addAggregate(sd);
builder.addTailPadding(sd->structsize);
isaStruct(t->type)->setBody(builder.defaultTypes(), builder.isPacked());
bool packed = builder.isPacked() || IrTypeAggr::isPacked(sd);
isaStruct(t->type)->setBody(builder.defaultTypes(), packed);
t->varGEPIndices = builder.varGEPIndices();
}

Expand Down
32 changes: 32 additions & 0 deletions tests/codegen/gh4719.d
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// RUN: %ldc -c -output-ll -of=%t.ll %s

struct TraceBuf {
align(1) uint args;
}

// Test correct compilation (no error) of the context pointer type for the delegate of `foo`.
void foo() {
byte[2] fixDescs;
TraceBuf fixLog;

auto dlg = delegate() {
fixDescs[0] = 1;
fixLog.args = 1;
};
}

class TraceClass {
align(1)
uint args;
}

// Test correct compilation (no error) of the context pointer type for the delegate of `foo2`.
void foo2() {
byte[2] fixDescs;
scope TraceClass fixLog;

auto dlg = delegate() {
fixDescs[0] = 1;
fixLog.args = 1;
};
}
20 changes: 20 additions & 0 deletions tests/compilable/gh4734.d
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// RUN: %ldc -c %s

align(1) struct Item {
KV v;
uint i;
}

struct KV {
align(1) S* s;
uint k;
}

struct S {
Table table;
}

struct Table {
char a;
Item v;
}

0 comments on commit 3591a77

Please sign in to comment.