mirrors/Odin
1
0
Fork 0
mirror of https://github.com/odin-lang/Odin.git synced 2026-08-14 01:34:35 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #7277 from corleypc/range-soa-array-elem-fix

Browse Source 
Fixes range loop over array elements in soa containers
This commit is contained in:
Jeroen van Rijn
2026-08-11 03:57:56 +02:00
committed by GitHub
parent 6d5dbdce13 87912a8b88
commit 9932b4d2d4
6 changed files with 341 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
src/check_expr.cpp
View File

@@ -12124,6 +12124,16 @@ gb_internal ExprKind check_slice_expr(CheckerContext *c, Operand *o, Ast *node,
case Type_Array:
valid = true;
max_count = t->Array.count;
if (is_type_soa_pointer(o->type)) {
// #soa element pointer; the pointed element is scattered like soa[i] itself,
// so it can't be sliced through the ptr (nor directly -> soa[i][:] is also rejected below)
gbString str = expr_to_string(node);
error(node, "Cannot slice '%s' through an #soa pointer, element is not contiguous in memory", str);
gb_string_free(str);
o->mode = Addressing_Invalid;
o->expr = node;
return kind;
}
if (o->mode != Addressing_Variable && !is_type_pointer(o->type)) {
gbString str = expr_to_string(node);
error(node, "Cannot slice array '%s', value is not addressable", str);

7
src/check_stmt.cpp
View File

@@ -1883,7 +1883,12 @@ gb_internal void check_range_stmt(CheckerContext *ctx, Ast *node, u32 mod_flags)
break;
case Type_Array:
is_possibly_addressable = operand.mode == Addressing_Variable || is_ptr;
// for #soa container with array element type, the element carries Addressing_SoaVariable,
// rather than Addressing_Variable; the element itself has no address,
// but each component does, so for &v in soa[i] is addressable
is_possibly_addressable = operand.mode == Addressing_Variable ||
operand.mode == Addressing_SoaVariable ||
is_ptr;
array_add(&vals, t->Array.elem);
array_add(&vals, t_int);
break;

34
src/llvm_backend_expr.cpp
View File

@@ -5087,26 +5087,11 @@ gb_internal lbAddr lb_build_addr_soa_elem_index(lbProcedure *p, Ast *expr, lbAdd
return lb_addr_soa_field_elem(lb_soa_field_elem_ptr(p, soa_addr.addr, cast(i32)component, soa_addr.soa.index));
}
// for non-constant index do chain select between the component pointers;
// an element array holds at most 4 components, so this is at most 3 compares and 3 selects (branchless),
// and should be folded if j turns out constant after inlining;
// TODO: more efficient codegen can be done, most definitely for fixed kind, possibly for slice/dynamic,
// (but this works for both kinds)
//
// Note: a temp holding the whole element would be simpler but would not be an lvalue,
// and writes go through here
lbValue index = lb_emit_conv(p, lb_build_expr(p, ie->index), t_int);
lb_emit_bounds_check(p, ast_token(ie->index), index, lb_const_int(p->module, t_int, component_count));
lbValue ptr = lb_soa_field_elem_ptr(p, soa_addr.addr, 0, soa_addr.soa.index);
// lb_emit_select evaluates both arms, so every candidate address is formed;
// only the selected one is loaded or stored through
for (i64 component = 1; component < component_count; component++) {
lbValue candidate = lb_soa_field_elem_ptr(p, soa_addr.addr, cast(i32)component, soa_addr.soa.index);
lbValue is_component = lb_emit_comp(p, Token_CmpEq, index, lb_const_int(p->module, t_int, component));
ptr = lb_emit_select(p, is_component, candidate, ptr);
}
return lb_addr_soa_field_elem(ptr);
return lb_addr_soa_field_elem(lb_soa_array_component_elem_ptr(p, soa_addr.addr, index, soa_addr.soa.index, component_count));
}
gb_internal lbAddr lb_build_addr_index_expr(lbProcedure *p, Ast *expr) {
@@ -5194,6 +5179,13 @@ gb_internal lbAddr lb_build_addr_index_expr(lbProcedure *p, Ast *expr) {
if (array_addr.kind == lbAddr_SoaVariable) {
return lb_build_addr_soa_elem_index(p, expr, array_addr, t);
}
// p[j], where p is an #soa element pointer (e.g. p := &soa[i])
if (is_type_soa_pointer(type_of_expr(ie->expr))) {
// build a soa variable from the soa ptr to get the address of the component
lbValue soa_ptr = lb_addr_load(p, array_addr);
lbAddr soa_addr = lb_addr_soa_variable_from_soa_ptr(p, soa_ptr);
return lb_build_addr_soa_elem_index(p, expr, soa_addr, t);
}
lbValue array = lb_addr_get_ptr(p, array_addr);
if (deref) {
array = lb_emit_load(p, array);
@@ -6578,10 +6570,7 @@ gb_internal lbAddr lb_build_addr_internal(lbProcedure *p, Ast *expr) {
// base p doesn't lower to an lbAddr_SoaVariable on its own
// (it is a local holding the soa pointer), so build the element
// addr here the same way an explicit p^ does
lbValue value = lb_build_expr(p, se->expr);
lbValue ptr = lb_emit_struct_ev(p, value, 0);
lbValue idx = lb_emit_struct_ev(p, value, 1);
addr = lb_addr_soa_variable(ptr, idx, nullptr);
addr = lb_addr_soa_variable_from_soa_ptr(p, lb_build_expr(p, se->expr));
} else {
addr = lb_build_addr(p, se->expr);
}
@@ -6803,10 +6792,7 @@ gb_internal lbAddr lb_build_addr_internal(lbProcedure *p, Ast *expr) {
case_ast_node(de, DerefExpr, expr);
Type *t = type_of_expr(de->expr);
if (is_type_soa_pointer(t)) {
lbValue value = lb_build_expr(p, de->expr);
lbValue ptr = lb_emit_struct_ev(p, value, 0);
lbValue idx = lb_emit_struct_ev(p, value, 1);
return lb_addr_soa_variable(ptr, idx, nullptr);
return lb_addr_soa_variable_from_soa_ptr(p, lb_build_expr(p, de->expr));
}
lbValue addr = lb_build_expr(p, de->expr);
return lb_addr(addr);

50
src/llvm_backend_general.cpp
View File

@@ -616,6 +616,16 @@ gb_internal lbAddr lb_addr_soa_variable(lbValue addr, lbValue index, Ast *index_
return v;
}
// lbAddr_SoaVariable for the element pointed by an #soa pointer,
// unpacked from the ptr's {^container, index} pair
//
// the nullptr index_expr is deliberate, there is no source index expression,
// the index was already bounds checked when the pointer was formed (e.g. p := &soa[i])
gb_internal lbAddr lb_addr_soa_variable_from_soa_ptr(lbProcedure *p, lbValue soa_ptr) {
GB_ASSERT_MSG(is_type_soa_pointer(soa_ptr.type), "%s", type_to_string(soa_ptr.type));
return lb_addr_soa_variable(lb_emit_struct_ev(p, soa_ptr, 0), lb_emit_struct_ev(p, soa_ptr, 1), nullptr);
}
// pointer to the index element of the field_index component
//
// the returned pointer type depends on the soa kind (because the field types do):
@@ -634,6 +644,41 @@ gb_internal lbValue lb_soa_field_elem_ptr(lbProcedure *p, lbValue soa_ptr, i32 f
return lb_emit_ptr_offset(p, lb_emit_load(p, field), index);
}
// the same address as lb_soa_field_elem_ptr, for a component index only known at runtime,
// this is array-element #soa only, unlike lb_soa_field_elem_ptr which serves any soa kind
//
// Note: the caller bounds checks component_index if needed
gb_internal lbValue lb_soa_array_component_elem_ptr(lbProcedure *p, lbValue soa_ptr, lbValue component_index, lbValue elem_index, i64 component_count) {
Type *t = base_type(type_deref(soa_ptr.type));
GB_ASSERT_MSG(t->kind == Type_Struct && t->Struct.soa_kind != StructSoa_None, "%s", type_to_string(t));
GB_ASSERT_MSG(base_type(t->Struct.soa_elem)->kind == Type_Array,
"indexing a component at runtime needs uniformly typed fields, got element %s",
type_to_string(t->Struct.soa_elem));
if (component_count == 0) {
// a [0]T element has no components, nor does its soa struct have a field to get a ptr to;
// emit a typed nil so that code is well formed;
// callers either check bounds (driect indexing) or skip the code (e.g. range loop over array element),
// so this is never dereferenced when bounds checks are on;
// with bounds checks off the access is out of bounds by definition
return lb_const_nil(p->module, alloc_type_pointer(base_type(t->Struct.soa_elem)->Array.elem));
}
// do chain select between the component pointers;
// an element array holds at most 4 components, so this is at most 3 compares and 3 selects (branchless),
// and it is folded if j is resolved to constant after inlining/unrolling;
// TODO: more efficient codegen can be done, most definitely for fixed kind, possibly for slice/dynamic,
// (but this works for both kinds)
//
// lb_emit_select evaluates both arms, so every candidate address is
// formed; only the selected one is dereferenced
lbValue ptr = lb_soa_field_elem_ptr(p, soa_ptr, 0, elem_index);
for (i64 component = 1; component < component_count; component++) {
lbValue candidate = lb_soa_field_elem_ptr(p, soa_ptr, cast(i32)component, elem_index);
lbValue is_component = lb_emit_comp(p, Token_CmpEq, component_index, lb_const_int(p->module, t_int, component));
ptr = lb_emit_select(p, is_component, candidate, ptr);
}
return ptr;
}
// lbAddr over an lb_soa_field_elem_ptr pointer, retyped ^T when it came as [^]T
gb_internal lbAddr lb_addr_soa_field_elem(lbValue ptr) {
if (is_type_multi_pointer(ptr.type)) {
@@ -1502,10 +1547,7 @@ gb_internal lbValue lb_emit_load(lbProcedure *p, lbValue value) {
LLVMValueRef v = OdinLLVMBuildLoad(p, lb_type(p->module, t), value.value);
return lbValue{v, t};
} else if (is_type_soa_pointer(value.type)) {
lbValue ptr = lb_emit_struct_ev(p, value, 0);
lbValue idx = lb_emit_struct_ev(p, value, 1);
lbAddr addr = lb_addr_soa_variable(ptr, idx, nullptr);
return lb_addr_load(p, addr);
return lb_addr_load(p, lb_addr_soa_variable_from_soa_ptr(p, value));
}
GB_ASSERT_MSG(is_type_pointer(value.type), "%s", type_to_string(value.type));

49
src/llvm_backend_stmt.cpp
View File

@@ -377,11 +377,22 @@ gb_internal void lb_build_when_stmt(lbProcedure *p, AstWhenStmt *ws) {
gb_internal void lb_build_range_indexed(lbProcedure *p, lbValue expr, Type *val_type, lbValue count_ptr,
lbValue *val_, lbValue *idx_, lbBlock **loop_, lbBlock **done_,
bool is_reverse, i64 unroll_count=0) {
bool is_reverse, i64 unroll_count=0, lbAddr const *soa_elem=nullptr) {
lbModule *m = p->module;
// when ranging over an #soa element rather than an array, expr is unused, there being no array
// to point at, so expr_type (the [N]T being ranged over) comes from the container's soa_elem
bool const is_soa_elem = soa_elem != nullptr && soa_elem->kind == lbAddr_SoaVariable;
lbValue count = {};
Type *expr_type = base_type(type_deref(expr.type));
Type *expr_type = nullptr;
if (is_soa_elem) {
Type *soa = base_type(type_deref(soa_elem->addr.type));
GB_ASSERT(soa->kind == Type_Struct && soa->Struct.soa_kind != StructSoa_None);
expr_type = base_type(soa->Struct.soa_elem);
} else {
expr_type = base_type(type_deref(expr.type));
}
switch (expr_type->kind) {
case Type_Array:
count = lb_const_int(m, t_int, expr_type->Array.count);
@@ -472,7 +483,13 @@ gb_internal void lb_build_range_indexed(lbProcedure *p, lbValue expr, Type *val_
switch (expr_type->kind) {
case Type_Array: {
if (val_type != nullptr) {
val = lb_emit_load(p, lb_emit_array_ep(p, expr, idx));
if (is_soa_elem) {
lbValue ptr = lb_soa_array_component_elem_ptr(p, soa_elem->addr, idx, soa_elem->soa.index, expr_type->Array.count);
lbAddr component = lb_addr_soa_field_elem(ptr);
val = lb_emit_load(p, component.addr);
} else {
val = lb_emit_load(p, lb_emit_array_ep(p, expr, idx));
}
}
break;
}
@@ -1343,7 +1360,8 @@ gb_internal void lb_build_range_stmt(lbProcedure *p, AstRangeStmt *rs, Scope *sc
break;
}
case Type_Array: {
lbValue array;
lbValue array = {};
lbAddr const *soa_elem = nullptr;
lbAddr addr = lb_build_addr(p, expr);
switch (addr.kind) {
case lbAddr_Swizzle:
@@ -1352,9 +1370,28 @@ gb_internal void lb_build_range_stmt(lbProcedure *p, AstRangeStmt *rs, Scope *sc
// NOTE(laytan): apply the swizzle.
array = lb_address_from_load(p, lb_addr_load(p, addr));
break;
case lbAddr_SoaVariable:
// for v in soa[i] (and other direct forms);
// there is no array here to range over, the element is scattered across the field
// arrays and has no address of its own, only a container pointer and the element
// index, which is what this lbAddr carries. lb_build_range_indexed uses those
// with the loop counter as the component index to address each component directly
//
// the element index bounds check is hoisted here,
// the component index is in range by construction and needs none
lb_emit_soa_index_bounds_check(p, addr.addr, addr.soa.index, addr.soa.index_expr);
soa_elem = &addr;
break;
default:
array = lb_addr_get_ptr(p, addr);
if (is_type_pointer(type_deref(array.type))) {
if (is_type_soa_pointer(type_deref(array.type))) {
// for v in p, where p is an #soa element ptr (e.g. p := &soa[i]);
// we need to build the soa variable from the soa ptr
// and then produce the soa_elem as in the lbAddr_SoaVariable case above;
lbValue soa_ptr = lb_emit_load(p, array);
addr = lb_addr_soa_variable_from_soa_ptr(p, soa_ptr);
soa_elem = &addr;
} else if (is_type_pointer(type_deref(array.type))) {
array = lb_emit_load(p, array);
}
break;
@@ -1362,7 +1399,7 @@ gb_internal void lb_build_range_stmt(lbProcedure *p, AstRangeStmt *rs, Scope *sc
lbAddr count_ptr = lb_add_local_generated(p, t_int, false);
lb_addr_store(p, count_ptr, lb_const_int(p->module, t_int, et->Array.count));
lb_build_range_indexed(p, array, val0_type, count_ptr.addr, &val, &key, &loop, &done, rs->reverse);
lb_build_range_indexed(p, array, val0_type, count_ptr.addr, &val, &key, &loop, &done, rs->reverse, 0, soa_elem);
break;
}
case Type_EnumeratedArray: {

226
tests/core/runtime/test_core_runtime.odin
View File

@@ -568,6 +568,27 @@ test_soa_array_elem_chained_indexing :: proc(t: ^testing.T) {
testing.expect_value(t, dyn_write.x[1], 90)
testing.expect_value(t, dyn_write.w[1], 93)
// through a pointer to the element
ep := &fixed_write[1]
k = 2
testing.expect_value(t, ep[k], 72)
testing.expect_value(t, ep[1], ep^[1])
ep[0] = 100
testing.expect_value(t, fixed_write.x[1], 100)
ep[k] = 102
testing.expect_value(t, fixed_write.z[1], 102)
pe := &ep[3]
pe^ = 103
testing.expect_value(t, fixed_write.w[1], 103)
eps := &slice_write[0]
eps[1] = 143
testing.expect_value(t, dyn_write.y[0], 143)
epd := &dyn_write[1]
testing.expect_value(t, epd[k], 92)
epd[0] = 190
testing.expect_value(t, dyn_write.x[1], 190)
// multi dim array element type, only the outer index is scattered
nested: #soa[2][3][2]u16
nested[1].x = {1, 3}
@@ -590,6 +611,211 @@ test_soa_array_elem_chained_indexing :: proc(t: ^testing.T) {
testing.expect_value(t, nested.y[1][0], 51)
testing.expect_value(t, nested.z[1][0], 52)
testing.expect_value(t, nested[0][0], [2]u16{0, 0})
pn := &nested[1]
testing.expect_value(t, pn[0][0], 50)
pn[0][1] = 60
testing.expect_value(t, nested.x[1][1], 60)
}
// ranging over an element of array-element typed #soa,
// for v in soa[i], or for &v in soa[i]
@(test)
test_soa_array_elem_range :: proc(t: ^testing.T) {
fixed: #soa[3][4]u16
fixed[0] = [4]u16{90, 91, 92, 93}
fixed[1] = [4]u16{10, 11, 12, 13}
for v, j in fixed[1] {
testing.expect_value(t, v, fixed[1][j])
}
#reverse for v, j in fixed[0] {
testing.expect_value(t, v, fixed[0][j])
}
got: [4]u16
i := 0
for v in fixed[1] {
got[i] = v
i += 1
}
testing.expect_value(t, got, [4]u16{10, 11, 12, 13})
// double loop
sum : u16 = 0
for e in fixed {
for v in e {
sum += v
}
}
testing.expect_value(t, sum, 90 + 91 + 92 + 93 + 10 + 11 + 12 + 13)
// through pointer to the container
got = {}
i = 0
p := &fixed
k := 1
for v in p[k] {
got[i] = v
i += 1
}
testing.expect_value(t, got, [4]u16{10, 11, 12, 13})
// slice
slice := fixed[:]
got = {}
i = 0
for v in slice[1] {
got[i] = v
i += 1
}
testing.expect_value(t, got, [4]u16{10, 11, 12, 13})
// dynamic
dyn: #soa[dynamic][4]u16
defer delete(dyn)
append_soa(&dyn, [4]u16{20, 21, 22, 23})
append_soa(&dyn, [4]u16{30, 31, 32, 33})
got = {}
i = 0
for v in dyn[1] {
got[i] = v
i += 1
}
testing.expect_value(t, got, [4]u16{30, 31, 32, 33})
// multi dim array type
nested: #soa[2][3][2]u16
nested[1] = [3][2]u16{{1, 2}, {3, 4}, {5, 6}}
flat: [6]u16
n := 0
for row in nested[1] {
for v in row {
flat[n] = v
n += 1
}
}
testing.expect_value(t, flat, [6]u16{1, 2, 3, 4, 5, 6})
// a write during the loop is visible to later iterations
// (same as normal arrays)
live: #soa[2][4]u16
live[0] = [4]u16{1, 2, 3, 4}
seen: [4]u16
for v, j in live[0] {
if j == 0 {
live.z[0] = 99
}
seen[j] = v
}
testing.expect_value(t, seen, [4]u16{1, 2, 99, 4})
//////////////////////////
// for &v in soa[i]
//////////////////////////
// fixed: #soa[3][4]u16
fixed[1] = [4]u16{1, 2, 3, 4}
for &v in fixed[1] {
v *= 10
}
testing.expect_value(t, fixed[1], [4]u16{10, 20, 30, 40})
testing.expect_value(t, fixed.x[1], 10)
testing.expect_value(t, fixed.w[1], 40)
fixed[1] = [4]u16{1, 2, 3, 4}
for &v, j in fixed[1] {
v += u16(j)
}
testing.expect_value(t, fixed[1], [4]u16{1, 3, 5, 7})
fixed[1] = [4]u16{1, 2, 3, 4}
#reverse for &v in fixed[1] {
v *= 2
}
testing.expect_value(t, fixed[1], [4]u16{2, 4, 6, 8})
// through pointer to the container
fixed[1] = [4]u16{1, 2, 3, 4}
k = 1
p = &fixed
for &v in p[k] {
v += 100
}
testing.expect_value(t, fixed[1], [4]u16{101, 102, 103, 104})
// slice
fixed[1] = [4]u16{1, 2, 3, 4}
slice = fixed[:]
for &v in slice[1] {
v *= 3
}
testing.expect_value(t, fixed[1], [4]u16{3, 6, 9, 12})
// dynamic
dyn[0] = [4]u16{1, 2, 3, 4}
for &v in dyn[0] {
v += 5
}
testing.expect_value(t, dyn[0], [4]u16{6, 7, 8, 9})
// multi dim array type
nested[0] = [2]u16{1, 2}
nested[1] = [2]u16{5, 6}
for &e in nested {
for &v in e {
v += 1
}
}
testing.expect_value(t, nested[0], [2]u16{2, 3})
testing.expect_value(t, nested[1], [2]u16{6, 7})
//////////////////////////
// through a pointer to the element
// for v in ep^, or for v in ep
//////////////////////////
fixed[1] = [4]u16{1, 2, 3, 4}
ep := &fixed[1]
got = {}
i = 0
for v in ep^ {
got[i] = v
i += 1
}
testing.expect_value(t, got, [4]u16{1, 2, 3, 4})
for &v, j in ep^ {
v += u16(10 * (j + 1))
}
testing.expect_value(t, fixed[1], [4]u16{11, 22, 33, 44})
// auto-deref
fixed[1] = [4]u16{1, 2, 3, 4}
got = {}
i = 0
for v in ep {
got[i] = v
i += 1
}
testing.expect_value(t, got, [4]u16{1, 2, 3, 4})
for &v in ep {
v *= 2
}
testing.expect_value(t, fixed[1], [4]u16{2, 4, 6, 8})
// a [0]T element has no components; the loop must not run
// (and the compiler must not crash :)
c0: #soa[2][0]u16
n = 0
for v in c0[0] {
_ = v
n += 1
}
testing.expect_value(t, n, 0)
}
// "using" on an #soa for-in looping variable