Packed arrays or arrays in packed structs

Hey, there :smiley:
I’m rewriting the xv6 kernel in Zig just for funs. While implementing the Interrupt Controller (PLIC; see spec) i need to access bit flags and a whole lot of them (or at least if i want to implement the spec correctly, there are only 2 really in use right now).
My first naive approach was to use an array in a packed struct…

pub const PLIC: *struct {
    const NUM_CONTEXTS = 15872;
    const NUM_SRCS = 1024;

    /// offset: 0x0
    /// size: 0x1000
    /// Notice: priorities[0] is reserved
    priorities: [NUM_SRCS]u32 = undefined,

    /// offset: 0x1000
    /// size: 0x80
    /// Notice: pending[0] is hardwired to false
    pending: [NUM_SRCS]bool = undefined,
    ...
} = 0x0c000000; // This is where qemu puts the PLIC

I soon figured out (aka. the compiler screamed at me…) that packed structs cann’t contain arrays (see Working around "packed structs cannot contain arrays", Using arrays in packed structs) and i guess there are reasons for this (see make `packed struct` always use a single backing integer, inferring it if not explicitly provided · Issue #10113 · ziglang/zig · GitHub).
And even if arrays would be allowed the total size of the PLIC is too big (I think this is a limitation of the implementation though).

My next idea was to do something like:

pub const PLIC = struct {
    const NUM_CONTEXTS = 15872;
    const NUM_SRCS = 1024;
    const BASE = memlayout.PLIC;

    /// offset: 0x0
    /// size: 0x1000
    /// Notice: priorities[0] is reserved
    const priorities: *[NUM_SRCS]u32 = @ptrFromInt(@intFromPtr(PLIC.BASE) + 0x0);

    /// offset: 0x1000
    /// size: 0x80
    /// Notice: pending[0] is hardwired to false
    const pending: *[NUM_SRCS]bool = @ptrFromInt(@intFromPtr(PLIC.BASE) + 0x1000);
    ...

Which I like less that the first one but sure it’s not too bad, right? Yes, except there is a Problem.
The following code doesn’t panic (x86_64, 0.15.2):

pub fn main() !void {
    const a: *[2]u8 = @ptrFromInt(@intFromPtr(&[2]bool{ false, true }));
    @import("std").debug.assert(a[0] == 0);
    @import("std").debug.assert(a[1] == 1);
}

What I am trying to say is, that arrays are not bit packed and I don’t know a way to get what I want :confused:. Any ideas?

1 Like

What’s the problem and why should it panic?

I’m not completely clear on the shape of the data you’re trying to access. The spec uses terminology that it doesn’t define, and doesn’t go into sufficient detail.

It seems like the concept of bit pointers could model this data, but alas, Zig doesn’t have bit pointers.

std.bit_set (a 32-bit-sized one, given how the PLIC memory map is described) is probably one component of the solution for accessing this data conveniently.

Zig does have them, but it’s quite obscure. The only things I found are some small section in the language reference in the packed structs part and on zig.guide

I also don’t know what you want to do but usually using some odd-sized integers can be used for padding.

const foo = packed struct {
    _padding: i12,
    useful_array: i100,
    _padding2: i3,
};

You could then maybe create a bitpointer to a specific bit inside the useful_array field or mask the relevant bits.

Hopefully that helps in some way.

1 Like

Vectors are allowed in packed struct. May that’s what you need?

const std = @import("std");

pub fn main() !void {
    const Struct = packed struct { vector: @Vector(16, bool) };
    const bytes: [2]u8 = .{ 0b1001_0001, 0b1000_0010 };
    const s: Struct = @bitCast(bytes);
    std.debug.print("{}\n", .{s.vector});
}
{ true, false, false, false, true, false, false, true, false, true, false, false, false, false, false, true }
1 Like

If you’re trying to control the format of the struct (padding, etc) to conform to a spec, then you probably want an extern struct rather than a packed struct. And an extern struct can contain arrays.

1 Like

Includes a thorough explanation of how current bit pointer syntax works

1 Like

Sorry this isn’t really related to the question itself. It’s was to show arrays aren’t bit packed (which is probably the right choice most of the time in an access time vs space trade off)

Not really what I’m looking for. Padding is not the problem atm. Thx anyways :slight_smile:

IIRC the bit_set types don’t have guaranteed layout. You’d need to copy/paste StaticBitSet into your own codebase and declare it extern.

Looks like you’re really left with using u32 and manual bit-twiddling, which isn’t bad in Zig.

You’d do the same in C, unless you take a dependence on implementation details because C doesn’t guarantee the layout of bitfields.

You can use a type function to generate a set of bitflags with nearly endless length:

/// Compose a packed struct with a given number of pseudo-anonymous fields.
pub fn AnonymousBitFlags(comptime field_num: u16) type {
	var fields: []const std.builtin.Type.StructField = &.{};
	@setEvalBranchQuota(@as(comptime_int, field_num) * 1024);
	for(0..field_num) |i| {
		fields = fields ++ &[1]std.builtin.Type.StructField{.{
			.name = std.fmt.comptimePrint("b{d}", .{i}),
			.type = bool,
			.is_comptime = false,
			.default_value_ptr = &false,
			.alignment = 0,
		}};
	}
	return @Type(.{.@"struct" = .{
		.layout = .@"packed",
		.backing_integer = @Type(.{.int = .{
			.bits = field_num,
			.signedness = .unsigned,
		}}),
		.fields = fields,
		.decls = &.{},
		.is_tuple = false,
	}});
}

pub const PLIC = struct{
	const NUM_CONTEXTS = 15872;
	const NUM_SRCS = 1024;
	
	const PendingBackingInteger = @Type(.{.int = .{
		.bits = NUM_SRCS,
		.signedness = .unsigned,
	}});
	
	priorities: [NUM_SRCS]u32 = undefined,
	
	/// Could leave this as undefined instead of default-initialising it.
	pending: AnonymousBitFlags(NUM_SRCS) = @bitCast(@as(PendingBackingInteger, 0)),
	
	// Comptime-assertions of field size
	comptime{
		if(@bitSizeOf(@FieldType(@This(), "pending")) != NUM_SRCS) unreachable;
		if(@sizeOf(@FieldType(@This(), "pending")) != @divExact(NUM_SRCS, 8)) unreachable;
	}
};

From the comptime-assertions, you can see that the size of the bitflags struct is exactly what we expect it to be.

I’ve omitted it for brevity’s sake, but if you’re intent on the PLIC being a packed struct, you could easily replicate this approach for the priorities as well, generating a packed struct of u32s instead of booleans. The two combined would result in the size of the PLIC’s backing integer being just over half of the maximum bit-width possible for an integer in Zig.

How you work with the bitflags is really up to you. You can wrap them in a struct type that provides methods to access them in an “array-like way”, or you can just @bitCast() them to the backing integer type and do bit operations on them, exactly like you would for C bitflags.

Here’s an example of a method that’d allow you to perform array-like indexing:

pub fn get_pending(self: PLIC, index: std.math.Log2Int(PendingBackingInteger)) bool {
	// Use a nasty trick to unroll all possibilities into separate functions at comptime.
	// This allows us to treat the index as comptime-known, even though it actually isn't.
	@setEvalBranchQuota(NUM_SRCS * 1024);
	switch(index){
		inline else => |i| {
			return @field(self.pending, std.meta.fields(@FieldType(PLIC, "pending"))[i].name);
		}
	}
}

And the same method implemented using bit operations:

pub fn get_pending(self: PLIC, index: std.math.Log2Int(PendingBackingInteger)) bool {
	return
		@as(PendingBackingInteger, @bitCast(self.pending)) &
		@as(PendingBackingInteger, 1) << index
		> 0
	;
}

Both seem to compile to roughly the same assembly instructions on ReleaseFast.

1 Like

First of thanks. This was really help full. I remember considering vectors at some point but don’t know why i decided against them at that time. It works almost as I want it to.
There is a matrix with one enable bit per context and source. This would be ideal for a 2d array (or rather vector in this case). Sadly vectors in vectors are not allowed (which, at least to me, also kind of makes sense). Anyways one long vector + functions to access it by index will work fine as well…

Thought of that, tried that. Two problems:

  1. Since extern structs are C-ABI compatible smallest type size is 1 byte. I need 1 bit.
  2. Arrays are still not packed. Each element has it’s own byte :expressionless:
    Still thanks for your help, though

Packed structs themselves are allowed inside packed struct if memory serves. Maybe that can be used to model a matrix? Just use numeric field names a la tuple.

1 Like

this is subject to change, the backend has complete control over what a vector is, including whether it is packed or not.

It is an implementation detail that shouldn’t be relied on upon, and it will become a compiler error regardless of the backend in the future.

1 Like

Hm, that’s interesting. Before posting here I had thought about the problem quite a bit but since I found a satisfying answer I saw it as a bit of a language quirk and nothing further. If using Vectors in packed structs is not guaranteed to work maybe it’s worth thinking about array in packed structs again. If I understand correctly the problem is the following:

Currently implemented is 1. I might be wrong but if we just always use reverse ordering I think now we have the same problem with little-endian. So 2. is probably also not an option. 4. just sounds like a footgun. 3. on the other hand doesn’t sound to bad in my opinion. Building on this my idea was the following:
Arrays now have an order. Either ascending or descending (this naming is on purpose not something like default and reverse). If an order is specified we have a guarantee by the compiler to honor it. If no order is specified we have no guarantees about the in memory representation.
When using an array in a packed struct you have to specify an order.
Something like:

const assert = @import("std").debug.assert;

const a: [2]i32 = .{ 0, -1 }; // No ordering specified
const b: order(.ascending) [2]i32 = .{0, -1};
const c: order(.descending) [2]i32 = .{0, -1}; 
assert(&b[1] == @ptrFromInt(@intFromPointer(&b) + 4);
assert(&c[1] == @ptrFromInt(@intFromPointer(&c) - 4);

For most use cases this doesn’t change anything except that you have no guaranteed memory layout. This shouldn’t matter though because if you want a guaranteed layout you can just use order().

Any opinions? Is this just some very specific use case and it’s okay that this is awkward or is this actually a problem worth solving?

My thoughts are we should just have (explicitly) packed arrays, yes it will be bitpacked (petition to make that the keyword :p).

const Foo = packed struct {
    x: i32,
    y: packed [2]i32,
};

It’s also convenient to reuse a keyword, instead of adding a new one. packed makes sense semantically here.

I think it makes sense for packed arrays to follow the same semantics of packed struct/union, That being least significant → most significant bits, with the mapping of bits → type being native endian.

When using for protocols, you already have to deal with what endian the protocol uses.

If just internally used within your code, you have the control to do whatever you want.

Indexing in reverse order is trivial maths; if you need/want to be smart with larger memory operations, you are either restricted by a protocol, or you have the control to make it work how you want.
If you have a conflict between multiple fancy operations, you would have that regardless of if arrays had built-in ordering.

So I don’t think there is much benefit in having ordering on arrays, even if it’s limited to packed arrays.

3 Likes

AFAIK that is an approved proposal, and I just saw a pr go by on the bot channel watching issues someone made in that direction.