Skip to content

Track lvalue alignment through fields and array elements - #5301

Open
TurkeyMan wants to merge 2 commits into
ldc-developers:masterfrom
TurkeyMan:lvalue-alignment-fields
Open

TurkeyMan wants to merge 2 commits into
ldc-developers:masterfrom
TurkeyMan:lvalue-alignment-fields

Conversation

@TurkeyMan

@TurkeyMan TurkeyMan commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Follow-up to #5297, which tracked alignment only for lvalues formed through a pointer, slice or ref. This extends it to every lvalue, so struct copies and zero-inits through fields, class members and static array elements carry their real alignment too.

Every DLValue now states its alignment; the constructor no longer has a default, so each site that creates one makes the call:

  • a field gets MinAlign(base, offset), where the base is the containing lvalue's alignment, the pointee type's for a struct pointer, or ClassDeclaration::alignsize for a class reference;
  • static array elements, slices of static arrays and complex parts follow the same rule; casts and repaints keep their source's alignment;
  • storage LDC allocates for a type takes that type's alignment, as discussed in Carry pointee alignment on struct copies through pointers and refs #5297. A few LDC-created globals and the tuple temporary, whose layout isn't governed by a D type, take the alignment LLVM gives them.

Locals bound to memory LDC didn't allocate for them record that memory's alignment where they're bound, rather than assuming their declared one: the NRVO variable (the sret buffer, aligned only for the return type), a ref local (its referent), a foreach value variable placed on an array element, and an in-place-constructed temporary (its destination).

In-place construction no longer hands an under-aligned destination to a callee as its sret pointer or this, nor to a vector store or a temporary's storage, since those all assume the type's alignment; such a destination is constructed in a temporary and copied. Today LDC passes e.g. a field at offset 1 of an align(1) struct straight to the callee as sret ... align 8. With that guard in place, a call's lvalue result takes its type's alignment, as you suggested in #5297.

Frontend: class fields now rely on class instances sitting at alignsize, which UnionExp violated on x86_64 posix (it emplaces RealExp/ComplexExp in 8-aligned storage). This cherry-picks 6709111 (dlang/dmd#23966).

Not in this PR:

  • Loads and stores still use the type's ABI alignment, so a field of an align(1) struct is still loaded as if naturally aligned. Honouring the tracked alignment there is a separate, smaller change that I'll open next.
  • Alignment can't be recovered where the frontend's own lowering forms a slice or pointer to under-aligned data: foreach (ref e; packed.arr) (lowered through __r = arr[]) and with (packed.field) (lowered through &field). That needs a frontend/spec change.
  • NRVO of an over-aligned local (align(64) S s; return s;) silently drops the extra alignment, since the local aliases the sret buffer. That's pre-existing; the frontend should decline NRVO there.

Testing: new tests/codegen/field_alignment.d covers packed and class fields, static array elements, ref locals, the sret guard, NRVO, array-to-struct casts and globals. The lit suite and druntime unittests pass with an assertion-enabled build.

UnionExp emplaces RealExp and ComplexExp into a union declared align(8),
but on x86_64 posix targets real_t is the x87 80-bit type with 16-byte
alignment, so those instances (and the complex_t field of ComplexExp)
were placed at addresses only 8-aligned. A compiler that copies the
complex_t field with its natural alignment emits movaps and faults.

(cherry picked from commit 6709111)
@TurkeyMan
TurkeyMan force-pushed the lvalue-alignment-fields branch from 2d84dd0 to 9be3444 Compare October 11, 2026 09:21
Every DLValue now states the alignment its address is guaranteed to
have. A field takes MinAlign(base, offset), where the base is the
containing lvalue's alignment, the pointee type's for a struct pointer,
or the class instance alignment for a class reference. Static array
elements, slices of static arrays and complex parts follow the same
rule, casts keep their source's alignment, and storage LDC allocates for
a type takes that type's alignment.

A local whose memory LDC didn't allocate for it records that memory's
alignment where it is bound: the NRVO variable (the sret buffer, aligned
for the return type), a ref variable (its referent), a foreach value
variable placed on an array element, and an in-place constructed
temporary (its destination).

Struct copies and zero-initialisations through fields, class members and
static array elements therefore carry their real alignment instead of 1.

In-place construction no longer hands an under-aligned destination to a
callee as its sret pointer or `this`, nor to a vector store or a
temporary's storage, since those all assume the type's alignment; such a
destination is constructed in a temporary and copied. This also lets a
call's sret result take its type's alignment.
@TurkeyMan
TurkeyMan force-pushed the lvalue-alignment-fields branch from 9be3444 to d4348ac Compare October 11, 2026 10:27
{
// CHECK: load i32, ptr @{{.*}}globalInt{{.*}}, align 4
return globalInt;
}

@kinke kinke Oct 12, 2026 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Thx; I haven't looked in detail yet.]

I'm missing a test for variables with a non-default alignment; I only see the overaligned NRVO-var case, where it's ignored.

Ideally a more complex expression, involving indexing and dotvar field access. Something like:

align(2) Big[5] bigs;
bigs[0].a[] = 0; // should be a memset-to-0 with destination alignment 2

For both locals and globals.

@TurkeyMan TurkeyMan Oct 13, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-default alignment is handled in the patch that follows this one. This one was more than big enough already though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants