std.PackedIntArray fails for some int types at comptime

[sjb3d]

Zig Version

0.11.0

Steps to Reproduce and Observed Behavior

Hi, I asked a question about this at https://ziggit.dev/t/packedintarray-at-comptime/3915, and it was mentioned that this could be related to #19452, so thought I should report this as an issue.

The issue that use of PackedIntArray during comptime seems to fail (with a compile error) for certain Int types. The compile error (in both 0.11.0 and 0.12.0-dev.3631+c4587dc9f) is:

error: comptime dereference requires 'packed_int_array.PackedIntArrayEndian(u10,.little,256)' to have a well-defined layout, but it does not.

The repro case for this is:

const std = @import("std");

const LookupTable = std.PackedIntArray(u10, 256);

fn makeLookupTable() LookupTable {
    @setEvalBranchQuota(10_000);
    var table: LookupTable = undefined;
    for (0..256) |i| {
        table.set(i, @truncate(i));
    }
    return table;
}

const lookupTable = makeLookupTable();

export fn example(index: usize) usize {
    return lookupTable.get(index);
}

Interestingly, I get the same error when using u10-u12 as the lookup table type, but u1-u9 and u13-u16 all compile fine. I am a confused about what could be special about u10-u12 that is causing the compile error.

Expected Behavior

Repro case compiles for all types u1-u16.

[mlugg]

On 0.12.0, this gives way to another error:

.opt/zig/lib/std/packed_int_array.zig:126:36: error: dereference of '*align(1) u24' exceeds bounds of containing decl of type '[320]u8'

I believe this is a subtle bug in std.PackedIntArray, because the load of Container in setBits will affect @sizeOf(MaxIo) bytes of memory, which is potentially more than (@bitSizeOf(MaxIo) + 7) / 8.

[sjb3d]

Ah thanks, if the remaining issue is in the standard lib then I can dig in a bit more, I can probably take a look in the next few days.

[sjb3d]

I dug in a bit more using @compileLog around this comptime load, it seems like a u24 load from byte offset 317 (of an array with 320 bytes) is requested, which seems fine if this is implemented as a 3-byte load, but could this be rounding up to a power of 2 size?

[mlugg]

Pointer loads fetch @sizeOf(T) bytes from memory - for u24, this is 4, because we use power-of-two sizes until we hit the system word size (at which point we instead work in multiples of that). So indeed, the current logic is wrong.

The solution is probably to either pad the buffer out to a multiple of @sizeOf(MinIo), or to load plain byte arrays in boundary cases and bitcast (or something like that).

Note that if you make this change and PR it, please make sure to add a corresponding test!

[sjb3d]

Ah thanks, I just wrote a smaller test case that confirms what you posted.

const std = @import("std");

fn foo(bytes: []const u8) u24 {
    const p: *align(1) const u24 = @ptrCast(&bytes[1]);
    return p.*;
}

export fn example() u32 {
    const bytes = [_]u8{ 0, 1, 2, 3 };
    return comptime foo(&bytes);
}

Produces: error: dereference of '*align(1) const u24' exceeds bounds of containing decl of type '[4]u8'

So it seems like PackedIntArray has not been written with this power-of-2 behaviour in mind, I'll have a think about how best to address it...

[sjb3d]

Note to self that there is also std.mem.readPackedInt and friends, which seem to work fine loading a u24 from the end of a byte array, so it might be worth refactoring PackedIntArray to make use of this (maybe only when reading near the end?).

Test case for readPackedInt that seems to work fine at comptime:

const std = @import("std");

export fn example() u32 {
    const bytes = [_]u8{ 0, 1, 2, 3 };
    return comptime std.mem.readPackedInt(u24, &bytes, 8, .little);
}