Skip to content

Adds alignment attributes to indirect args and sret slots - #7382

Open
corleypc wants to merge 11 commits into
odin-lang:masterfrom
corleypc:indirect-arg-align
Open

Adds alignment attributes to indirect args and sret slots#7382
corleypc wants to merge 11 commits into
odin-lang:masterfrom
corleypc:indirect-arg-align

Conversation

@corleypc

@corleypc corleypc commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

(Update: This PR now fixes more than a dozen alignment bugs (read thread).)

The PR adds the align attribute (where align > 1) to the compiler owned indirect arguments for the Odin and contextless calling conventions and on the sret slot for all calling conventions. It also fixes two alignment related bugs (detailed below).

An immediate followup to this (and the main goal of doing this in the first place) is adding the nonnull and dereferenceable attributes, which I am working on and it is shaping as a way smaller change.

A lot of the necessary plumbing came with #7327 by @kalsprite, which I was looking forward to it getting merged in order to do this properly. It landed attribute propagation at call sites, type_align_of(source type) for byval arguments and I used the ABI harness for validation.

What's implemented in the PR

  • lb_try_get_alignment is now expanded and tries hard to figure "what alignment can be honestly claimed for this LLVM pointer"?
    It starts assuming the default storage alignment of T for ^T, then looks for provable
    knowledge to override this assumption. Allocas, globals, upstream argument align attributes, phi/selects, chained GEP arithmetic (up to a preconfigered chain depth, currently set to 8) can all override the default alignment (up or down). For example, a pointer to a field of a #packed struct can be resolved below the type default alignment, or GEP arithmetic may upgrade the alignment.

  • A pass over the arguments (lb_abi_add_indirect_source_type_alignments) runs after the target ABI argument types are known and stamps align(N) attributes (when N>1) on:

  1. indirect arguments for the odin and contextless calling conventions, where the compiler controls both caller and callee;
  2. sret return slots for every CC, because the slot is always aligned. On the Odin side it comes from lb_add_local allocas or is RVO forwarded (so keeps the alignment), and the platform/C ABI dictates the same;
  3. the context pointer.
  • The alignment of indirect args is then honored at call sites in lb_emit_call. Alignment of constants is bumped up if insufficient, so that they materialize properly aligned. For non-constant lvalues where lb_try_get_alignment can't prove sufficient alignment (e.g. a #packed field), the value is copied to aligned temp first.

  • Memory intrinsics (memcpy/memmove/memset) currently claim pessimistic alignment. With the PR, when doing copies each pointer operand goes through lb_mem_copy_ptr_alignment (default = pointed type alignment, possibly overridden by lb_try_get_alignment) before converting to rawptr, and memset gets its alignment passed directly. The unaligned builtins (mem_copy/mem_copy_non_overlapping on rawptrs, unaligned_load/unaligned_store, mem_zero) strip the type or pass align 1, but lb_try_get_alignment can still upgrade them.

  • A couple of bugs fixed

  1. lb_try_get_alignment currently returns an access alignment (LoadInst) as if it is the loaded pointer value alignment which fed lb_emit_store memmove. The PR treats loads as opaque.

  2. Call emits currently pass &packed.field as an indirect arg while the callee assumes default alignment which could miscompile on alignment strict targets (RiscV64, maybe Arm32) and can segfault on x86 (replication of the segfault in the PR added regression test). The new alignment check with potential aligned temp copy prevents this.

What does this get us?
Alignment knowledge that can't always be re-derived b the optimizer now flows. As mentioned, the real goal is having nonnull and deferenceable in, but the alignment attribute nets positives on its own too. On strict alignment targets (like riscv64), align 1 memove forces a libcall, while align 8 leads to inline load/stores (verified in IR).

A before/after example on x86
package main

V :: struct { a, b, c, d, e: #simd[4]f32 }   // sruct size > 64 bytes

@(export)
sum5 :: proc(v: V) -> #simd[4]f32 {
	local := v      // currently, llvm loses alignment knowledge at the copy site (memove internally)
	return local.a + local.b + local.c + local.d + local.e
}

Built with -o:speed, default arch target (SSE, but no AVX), LLVM 22.

ASM before
Code assumes align 1 and must load every vector into a register using unaligned load before the addition.

sum5:
	movups	(%rdi), %xmm0
	movups	16(%rdi), %xmm1
	addps	%xmm0, %xmm1
	movups	32(%rdi), %xmm0
	addps	%xmm1, %xmm0
	movups	48(%rdi), %xmm1
	addps	%xmm0, %xmm1
	movups	64(%rdi), %xmm0
	addps	%xmm1, %xmm0
	retq

ASM after
Code knows alignment is 16 and can add directly from aligned memory.

sum5:
	movaps	(%rdi), %xmm0
	addps	16(%rdi), %xmm0
	addps	32(%rdi), %xmm0
	addps	48(%rdi), %xmm0
	addps	64(%rdi), %xmm0
	retq

The copy (technically memmove) is elided in both cases, but in the current version LLVM must assume align 1, cause the local copy loses alignment provenance. (Struct size must be > 64 for this to show up, at <= 64 the stores are built directly in the backend.)
Also note that AVX is not sensitive to alignment, so this won't show up in AVX targeting code.

Adding the dereferenceable attribute (coming soon!) would allow the optimizer to do a bunch of neat tricks with load speculation like hoisting loads out of loops, vectorizing conditional loads, flattening if/elses's, and more. And alignment is prerequisite for dereferenceable, cause the optimizer only speculates with known (and sufficient) alignment.

We need to talk about addresses of packed fields

I believe everything knowable by analysis is addressed, but there are language constructs that actively put a stick in the wheel. (These aren't PR introduced but relate to alignment, so a proper time for them to get a mention).

  1. Memory round trips
    p: ^Big = &packed.bigfield; f(p^)
    the address reaches the call site from a load, and a loaded ^T is trusted as align_of(T) by convention. Alignment analysis can't see through a store/load pair.
  2. A proc can legally return &packed.bigfield on a packed struct; the returned pointer comes as an opaque call result (also trusted).
  3. Unsafe code by design. FFI pointers and cast(^T) on arbitrary rawptrs, LLVM inttoptr (transmute, uintptr). All of these are trusted because the programmer asserted so, identical to C, and we are not really concerned with them. Anything goes and the programmer can shoot legs all the time.

1 and 2 appear for #packed structs or when #max_field_align is less than the field T alignment. If we want to address them, it is a language level thing. In C, dereferencing a pointer to a packed field is UB and clang/gcc have warnings on by default for taking the address of a packed field. In Go, there are no packed structs at all and packing goes through encoding/binary. In Rust, taking the address of a packed field is a compilation error.

Possible actions we can take for Odin.

  1. Document explicitly in the spec (and overview) what the backend relies on. That is, a ^T is presumed align_of(T); underaligned access goes through rawptr or unaligned intrinsics. Basically, define p^ on an underaligned storage as a a programmer bug.

  2. Vet warning like gcc/clang. This can be placed on taking the address or at use, or both. Conversions to rawptr (maybe byte multipoinmters?) can be exempt. This should be cheap to implement, cause field offsets and alignments are static, and the checker has everything it needs.

  3. Runtime checks in debug builds. Emits are essentially the assertions to check, so instrumentation is small addition to emission time, not some new analysis. (Will also catch some of the unsafe stuff in 3.)

  4. Just hard error on &packed.field as ^T, like Rust. :) Will break existing code (if any),

  5. Suggestions?

@gingerBill

Copy link
Copy Markdown
Member

Regarding the possible actions:

  • (1) and (2) together seem like the better options to choose.
  • (3) seems way too costly (and I don't like having different behaviour based on optimization level, unless it is explicitly opted into).
  • (4) sounds like a very bad idea and will probably break the type system completely (you'd need to have something like a specifically-aligned pointer typed).

@corleypc

corleypc commented Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

Pushed new commit 2bfcb04 which fixes two more alignment bugs. One is the <=64 bytes store path ignoring the is_packed custom metadata.

package main
Packed :: struct #packed {
      _: u8,
      v: #simd[4]f32,
}

@(export) g: Packed

f :: proc(p: ^Packed, x: #simd[4]f32) {
      y := p.v  // load is fine, GEP has packed metadata -> load emits align 1
      _ = y
      p.v = x   // segfault here, same GEP, store emitter ignores the metadata
}

main :: proc() { f(&g, {1, 2, 3, 4}) }

The other is GEPs on packed fields on globals folding (so nothing to attach the metadata in the first place) on both loads and stores.

package main
Packed :: struct #packed {
      _: u8,
      v: #simd[4]f32,
}

@(export) g: Packed

main :: proc() {
      y := g.v   // segfault, GEP folds, type align 16 assumed by LLVM
      g.v = y    // same
}

All three segfault on x64, default target. Fixed using the new lb_try_get_alignment (global GEPs transparent there and alignment recovered). Added regression tests to internal.

Edit: Oh, well, I guess I'll be looking at darwin IR. :)

@corleypc

Copy link
Copy Markdown
Contributor Author

2bfcb04 missed fixing alignment for vectors (and scalars) zeroing in lb_mem_zero_ptr, triggered by the analog of g.v = {} in the test. Fixed in a5f49df

testing.expect(t, p2.n == -1)

p2.v = {} // 16 bytes is memset on most targets
p2.v = {} // zero vector store -> lb_mem_zero_ptr's direct store path on most targets

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.

memset was a wrong assumption.

@corleypc

Copy link
Copy Markdown
Contributor Author

While playing with vet variants, looking at &packed.field uses in the library, I found another bug. GEPs derived from a #packed field GEP don't have the is_packed metadata and resolve to type alignment.
Here is another segfault :) (x86, default target, so SSE code)

package main
import "core:simd"
import "core:fmt"

Packed :: struct #packed {
      _:   u8,
      arr: [2]#simd[4]f32, // arr at offs 1, elems at 1 and 17
}

@(export) g: Packed

read_elem :: proc(p: ^Packed, i: int) -> #simd[4]f32 {
      return p.arr[i]
}

main :: proc() {
      g.arr[0] = {1, 2, 3, 4}
      y := #force_no_inline read_elem(&g, 0)   // boom
      fmt.println(simd.to_array(y))
}

Fixed in ed08cda by going back the GEP chain until metadata found or non-GEP reached.

(The actual use in the library (nanovg) was fine (well, at least on x86) due to it using f32's throughout, alignment resolving to 4 bytes and the vectorizer merging to unaligned vector stores.)

@corleypc

Copy link
Copy Markdown
Contributor Author

@gingerBill
After a few iterations, here is the version of the -vet-packed-field-addr flag that I arrived at.

It is a bit more relaxed than the gcc/clang warning. In particular, if the taken underaligned address is immediately cast to a pointer type with sufficient alignment, there is no err. This includes casts to rawptr, ^u8, ^runtime.Unaligned (see below), basically anything with data alignment of 1 for #packed fields or with alignment N for #max_field_align(N) fields.

runtime.Unaligned
While looking at library code to see how packed fields are addressed I stole an idea from rexcode and adapted it into runtime.Unaligned which I believe ties a few loose ends.

// A view of `T` at alignment 1. `^Unaligned(T)` may point at unaligned
// memory, e.g. the address of a field of a `#packed` struct, or of a struct
// whose `#max_field_align` is lower than `T`'s alignment.
// Accesses through `.value` are misalignment safe.
Unaligned :: struct($T: typeid) #packed { value: T }

This single field #packed wrapper allows explicit downgrade of the claimed alignment of the address, while not erasing its type. It is zero cost (in the sense that all its work is done at compile time). It composes well. Examples:

  • ^Unaligned(T) is safe as parameter type when working with #packed and max_field_align(N) structs;
// f keeps the type knowledge, accesses are safe
f :: proc(u: ^runtime.Unaligned(i64)) { u.value += 1 }
f((^runtime.Unaligned(i64))(&p.n))
  • [^]Unaligned(T) — a multipointer for iterating unaligned records out of a byte buffer, with a stride size_of(T), and every access safe;
elems := ([^]runtime.Unaligned(f32))(raw_data(buf[1:]))
sum := elems[0].value + elems[1].value
  • []Unaligned(T) — a slice view over such records;
  • Unaligned(T) as a field of an otherwise normal struct; declares one intentionally unaligned member without packing the whole struct.

There is one downside in that you need to use .value. (It can be avoided for concrete struct types by struct #packed { using v: T }, and the using gives you transparent field access (u.x instead of u.value.x), but this can't work for a generic wrapper.)

Some examples of what would be catched by the vet and what is legal

A few examples from the library first:
type := (^[4]byte)(&ch.type)^ from "core:image/png"
ch is a PNG_Chunk_Header (#packed) and type is u32be-backed enum, so a bare &ch.type would vet error (data alignment 4 > #packed alignment 1). But the address goes directly into the (^[4]byte) cast, and [4]byte has alignment 1. No vet error.

op := &inst.ops[slot] from "core:rexcode"
Instruction (type of inst) is #packed, but so is Operand (type of ops[slot]). The resulting ^Operand has data alignment 1. No vet error.

The last one is from "vendor:nanovg"

__xformToMat3x4 :: proc(m3: ^[12]f32, t: [6]f32) {....}
....
__xformToMat3x4(&frag.scissorMat, invxform)

frag is a ^FragUniforms, which is #packed, and scissorMat is a [12]f32 field. So &frag.scissorMat produced a ^[12]f32, which is data alignment 4, while the #packed frag only guarantees align 1. Vet error. This was actually the only ver error in the entire library. I fixed this by returning the result by value instead of in a ptr param. Another possibility would have been to use ^runtime.Unaligned. (But returning by value also happens to compile to smaller and faster code (at -o:speed, x86, default target, LLVM 22.), because in the original by ptr param return, the compiler has to assume the ptr aliases. (#no_alias would have made it equal))

More examples:

import "base:runtime"

P :: struct #packed {
	b:     u8,
	v:     #simd[4]f32,
	n:     i64,
	arr:   [2]f32,
	inner:  struct { f: f32, g: f32 },
	slice: []i64,
	ptr:   ^struct { x: f32 },
	sub:   runtime.Unaligned(f32),
}
p:  P

M :: struct #max_field_align(4) {
	a:     u16,
	b:     u64, // placed at alignment 4
	inner: struct { x: i64 },  // i64 inside, container capped to 4
}
m: M

poly :: proc(x: ^$T) {}

vet_errors :: proc() {
	a := &p.v         // vet err - simd field, claims align 16
	b := &p.n         // vet err - i64 field, claims align 8
	c := &p.arr[1]    // vet err - element of an array field
	pp := &p
	d := &pp.v        // vet err - through ^P (auto deref)
	e := &p.inner.f   // vet err - field of a nested (even though non-packed) struct
	poly(&p.n)        // vet err - poly param, no align-1
	f := &m.b         // vet err - align 8 field of a #max_field_align(4) struct
	g := &m.inner.x   // vet err - i64 in a nested struct, align cap 4
}

take_unal_addr :: proc(u: ^runtime.Unaligned(#simd[4]f32)) {}

// these all pass
passing :: proc() {
	a := &p.b           // u8 field, data align 1
	b := (^u8)(&p.n)    // direct cast to ^u8
	c := rawptr(&p.v)   // conversion to rawptr
	take_unal_addr((^runtime.Unaligned(#simd[4]f32))(&p.v)) // runtime.Unaligned cast
	poly((^runtime.Unaligned(i64))(&p.n))   // same, into a poly param, T infers as runtime.Unaligned(i64)
	not_packed: struct { f: f32 }
	d := &not_packed.f  // not a #packed
	e := &p             // whole packed struct, data align 1
	f := &p.slice[0]    // slice element, not packed storage (only slice header in packed)
	g := &p.ptr.x       // through a pointer field, ptr is in packed but not what it points to
	h := &p.sub         // pointee itself #packed: align 1
	i := &m.a           // align 2 field within align 4 cap
	j := (^u32)(&m.b)  // cast within the align 4 cap
	k := (^runtime.Unaligned(u64))(&m.b) // runtime.Unaligned also covers #max_field_align fields
}

I haven't added the flag to -vet, so currently it needs to be added explicitly on the command line.

Two more segfault bugs fixed along the way, derived GEPs for fields of #max_field_align structs and array element type conversions for array fields in #packed structs. Tests added for these two.

If the vet flag is satisfactory, I believe this PR is now complete.

@corleypc corleypc mentioned this pull request Aug 24, 2026
@corleypc

corleypc commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

Meanwhile, a few more alignment bugs fixed in 429ee55

  1. #align(N) can lower a struct/union/raw_union alignment below a field type alignment, but field access still assumes type alignment. (Issue Append bug with unaligned structures containing u128 #6179 is a manifestation of this bug for structs.)

This accounts for a big chunk of the change, cause union variant accesses needed fixes per site.

For structs, the fix uses existing max-align custom metadata. lb_emit_struct_ep now folds custom_align into the max-align cap (honoring both, if present), so a field GEP carries max-align metadata and lb_adjust_access_alignment_from_addr limits every load/store alignment through it.

Raw union fields also get the metadata, cause lb_emit_deep_field_gep now routes them through lb_emit_struct_ep (a zero offset GEP) instead of a pointer cast, and the metadata attaches to the GEP instruction.

For unions, the variant pointer is a cast, not a GEP, so the metadata can't attach. Instead, the alignment cap is applied at each site. This needed a lb_emit_store version with max alignment limit, so variant stores go through lb_emit_store_with_max_align(..., type_align_of(union)), variant loads in lb_emit_union_cast are capped with lb_cap_access_alignment. The union tag GEP does get max-align metadata. By-ref type switch case variable is bound to a zero offset GEP (gets the metadata), and for zeroing the capped alignment goes into lb_mem_zero_ptr (its scalar path now honors it).

Test cases for all of these added to tests/internal/test_custom_align.odin

A minimal segfault example (Linux, x64, LLVM 22)

package main
import "core:fmt"
import "core:mem"

S :: struct #align(1) { 
    a: #simd[4]f32  // type align 16
}   // size_of(S) == 16, align_of(S) == 1

// export so it isn't folded
@(export) rs :: proc(s: ^S) -> #simd[4]f32 { return s.a }

main :: proc() {
	raw, _ := mem.alloc_bytes(size_of(S), align_of(S)) // align 1 requested, so legal
	s := cast(^S)(&raw[0])
	fmt.println(rs(s)) // segfault
}
  1. #min_field_align incorrectly overrides #packed (e.g. on an outer struct).
package main
import "core:fmt"

Inner :: struct #min_field_align(16) { v: #simd[4]f32 }
Outer :: struct #packed { x: u8, inner: Inner }

Helper :: struct {
      force: #simd[4]f32, // foces align 16
      _:     u8,
      o:     Outer,       // offset 17, inner.v at 18
}
@(export) h: Helper
@(export)
f :: proc(o: ^Outer) -> #simd[4]f32 {
      return o.inner.v
}

main :: proc() {
      h.o.inner.v = {1, 2, 3, 4} // const GEP works
      fmt.println(#force_no_inline f(&h.o))  // segfault
}

Fixed in lb_adjust_access_alignment_from_addr by making #packed have the final say. Test added.

  1. A pointer's load's alignment is incorrectly assumed for the loaded pointer itself. A similar bug was already fixed in lb_try_get_alignment (described in the first post here). Fixed the same way. An instance of the same bug was fixed in lb_try_vector_cast.

This needs to be built with -o:speed for the segfault to manifest. (Observed on Linux, x64, default target (need SSE for the crash), o:speed, LLVM 22)

package main
import "core:fmt"

S :: struct { p: ^[8]u8 }
@(export) backing: [32]u8
@(export) buf: [2]#simd[4]f32

@(export)
f :: proc(s: ^S, t: ^S) -> ([8]u8, #simd[4]f32) {
    // s.p^ + t.p^ makes the compiler assume align 8 on the
    // loads through s.p/t.p (incorrectly, from the loads' own alignment)
    r := s.p^ + t.p^
    low3bits := uintptr(rawptr(s.p)) & 7 // optimizer folds the & 7 to 0, cheerfully assuming s.p is align 8
    q := (^#simd[4]f32)(rawptr(uintptr(&buf) + 15 + low3bits)) // movaps to buf+15 -> segfault
    return r, q^
}

main :: proc() { 
    // explicitly misalign off to (base+off) & 7 == 1
    base := uintptr(&backing[0])
    off := int((1 + 8 - (base & 7)) & 7) 
    for i in 0..<8 {
            backing[off+i]   = u8(i + 1)
            backing[off+8+i] = u8(10 * (i + 1))
    }
    buf[1] = {1, 2, 3, 4}

    s := S{(^[8]u8)(rawptr(&backing[off]))}
    t := S{(^[8]u8)(rawptr(&backing[off+8]))}
    r, v := #force_no_inline f(&s, &t)
    fmt.println(r, v)
}

Internal tests are not run at o:speed, so no test added for this one.

  1. For vectors max_simd_align caps alignment, but load/store codegen still uses LLVM's natural vector alignment. That is, codegen lags the ABI changes in ABI Conformance Harness + Fixes #7327, which set max_simd_align per target. For example, a 32-byte vector is 32-aligned to LLVM but in reality only 16-aligned on darwin (amd64/arm64) and the other arm64 targets.

Since vectors can appear at tons of store places, fixing this needed adding the OdinLLVMBuildStore and OdinLLVMBuildStoreAligned wrappers (similar already existed for loads so trivial fix there), which honor the alignment. And then replacing stores throughout accounts for a lot of the commit.

This was IR and asm verified.

-target:linux_arm64 (max_simd_align == 16)

V :: #simd[32]u8  // align_of(V) == 16
@(export) loc :: proc(v: V) -> V { x := v; return x }

IR:

%x = alloca <32 x i8>, align 16
store <32 x i8> %1, ptr %x, align 32     ; contradicts the x alloca

-target:darwin_amd64 -microarch:x86-64-v3 (AVX) (max_simd_align == 16)

V :: #simd[8]f32
@(export) ld :: proc(p: ^V) -> V { return p^ }
@(export) al :: proc() -> int { return align_of(V) }

ASM:

_ld:  vmovaps (%rdi), %ymm0 ; retq   # requires 32-byte alignment -> would segfault
_al:  movl $16, %eax ; retq             # align_of(V) only grants align 16

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