Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make packed structs versatile #3133

Closed
andrewrk opened this issue Aug 28, 2019 · 7 comments
Closed

make packed structs versatile #3133

andrewrk opened this issue Aug 28, 2019 · 7 comments
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Milestone

Comments

@andrewrk
Copy link
Member

Currently, packed structs are always align(1) and each field of a packed struct is either align(1) or a special kind of alignment that indicates bit offsets. I propose the following changes:

  • implement ability to specify explicit field alignment of packed structs (related to ability to set alignment on struct fields #1512), including even bit offsets. This can cause padding to be inserted (possibly even just bits to the next byte).
  • packed structs, like normal structs, have the alignment of the most-aligned field. Minimum 1 byte.
  • without explicit alignment specified, fields get bit packed. However fields can be specified to be byte aligned and pointers to these fields can even be naturally aligned. A packed struct with every field like this: field: T align(T), would be equivalent to an extern struct.
  • ability for packed structs to have fields that are normal structs. In this case whether a packed struct has well-defined memory layout is determined by whether or not all the fields it has have well-defined memory layout. Normal structs and types can still be bit-packed though!
  • when a packed struct has no well-defined memory layout, it can participate in debug safety features (see add safety checks for pointer casting #2414). Perhaps there could even be a special type that could be used as a field, that allows Zig to put the debug safety in that field's location, and packed structs would have well-defined memory layout until the first field that does not have well defined memory layout. This would allow putting this debug safety field at the end, and still doing @bitCast, @ptrCast, or other useful things with packed structs.
  • fix the issues regarding packed structs not ending with byte-alignment. This should be handled semantically equivalent to, for example, u7.

Related bug: #2627

@andrewrk andrewrk added the proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. label Aug 28, 2019
@andrewrk andrewrk added this to the 0.6.0 milestone Aug 28, 2019
@andrewrk andrewrk added the accepted This proposal is planned. label Nov 29, 2019
@andrewrk
Copy link
Member Author

andrewrk commented Jan 2, 2020

Here are some nice tests provided by @MasterQ32:

const std = @import("std");

test "sizeOf, variant 1" { // breaks in master
    const T = packed struct {
        one: u8,
        three: [3]u8,
    };
    std.testing.expectEqual(@as(usize, 4), @sizeOf(T));
}

test "sizeOf, variant 2" { // doesn't break in master
    const T = packed struct {
        three: [3]u8,
        one: u8,
    };
    std.testing.expectEqual(@as(usize, 4), @sizeOf(T));
}

test "daurnimator (original)" { // breaks in master
    const T = packed struct {
        _1: u1,
        x: u7,
        _: u24,
    };
    std.testing.expectEqual(@as(usize, 4), @sizeOf(T));
}

test "daurnimator (variant 1)" { // doesn't break in master
    const T = packed struct {
        _1: u1,
        x: u7,
        _2: u8,
        _3: u16,
    };
    std.testing.expectEqual(@as(usize, 4), @sizeOf(T));
}

test "daurnimator (variant 2)" { // doesn't break in master
    const T = packed struct {
        _1: u1,
        x: u7,
        _2: u16,
        _3: u8,
    };
    std.testing.expectEqual(@as(usize, 4), @sizeOf(T));
}

test "MasterQ32'1" {
    const Flags1 = packed struct {
        // byte 0
        b0_0: u1,
        b0_1: u1,
        b0_2: u1,
        b0_3: u1,
        b0_4: u1,
        b0_5: u1,
        b0_6: u1,
        b0_7: u1,

        // partial byte 1 (but not 8 bits)
        b1_0: u1,
        b1_1: u1,
        b1_2: u1,
        b1_3: u1,
        b1_4: u1,
        b1_5: u1,
        b1_6: u1,

        // some padding to fill to size 3
        _: u9,
    };
    // TODO: This still breaks
    // std.testing.expectEqual(@as(usize, 4), @sizeOf(Flags1));
}


test "MasterQ32'2" {
    const Flags2 = packed struct {
        // byte 0
        b0_0: u1,
        b0_1: u1,
        b0_2: u1,
        b0_3: u1,
        b0_4: u1,
        b0_5: u1,
        b0_6: u1,
        b0_7: u1,

        // partial byte 1 (but not 8 bits)
        b1_0: u1,
        b1_1: u1,
        b1_2: u1,
        b1_3: u1,
        b1_4: u1,
        b1_5: u1,
        b1_6: u1,

        // some padding that should yield @sizeOf(Flags2) == 4
        _: u10, // this *was* originally 17, but the error happens with 10 as well
    };
    std.testing.expectEqual(@as(usize, 4), @sizeOf(Flags2));
}


test "MasterQ32'3" {
    const Flags3 = packed struct {
        // byte 0
        b0_0: u1,
        b0_1: u1,
        b0_2: u1,
        b0_3: u1,
        b0_4: u1,
        b0_5: u1,
        b0_6: u1,
        b0_7: u1,

        // byte 1
        b1_0: u1,
        b1_1: u1,
        b1_2: u1,
        b1_3: u1,
        b1_4: u1,
        b1_5: u1,
        b1_6: u1,
        b1_7: u1,

        // some padding that should yield @sizeOf(Flags2) == 4
        _: u16, // it works, if the padding is 8-based
    };
    std.testing.expectEqual(@as(usize, 4), @sizeOf(Flags3));
}

test "fix for #3651" {
    const T1 = packed struct {
        array: [3][3]u8, // also with align(1)
    };

    const T2 = packed struct {
        array: [9]u8,
    };
    std.testing.expectEqual(@as(usize, 9), @sizeOf(T1));
    std.testing.expectEqual(@as(usize, 9), @sizeOf(T2));
}

@markfirmware
Copy link
Contributor

markfirmware commented Jan 4, 2020

Another case:

const S = packed struct {
    a: u32,
    pad: [3]u32,
    b: u32,
};

test "packed struct @byteOffsetOf" {
    const offset = @byteOffsetOf(S, "b");
    warn("\n\noffset is {}\n\n", .{offset});
    assert(offset == 16);
}

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

@FireFox317 @fengb @andrewrk

@andrewrk
Copy link
Member Author

@mikdusan
Copy link
Member

I have code producing these numbers; need some confirmation I'm on the correct path:

same struct with a single field align(N) changing { 0,1,2,4,8 }
// abi_size=7
// size_in_bits=54
// abi_align=1
const Foo = packed struct {
    a: u12,         // @bitSizeOf=12  @align= 0  @byteOffsetOf= 0  @bitOffsetOf=  0  → 1:0:2
    b: u1,          // @bitSizeOf= 1  @align= 0  @byteOffsetOf= 1  @bitOffsetOf= 12  → 1:4:1
    c: u32,         // @bitSizeOf=32  @align= 0  @byteOffsetOf= 1  @bitOffsetOf= 13  → 1:5:5
    d: u4,          // @bitSizeOf= 4  @align= 0  @byteOffsetOf= 5  @bitOffsetOf= 45  → 1:5:2
    e: bool,        // @bitSizeOf= 1  @align= 0  @byteOffsetOf= 6  @bitOffsetOf= 49  → 1:1:1
    f: u4,          // @bitSizeOf= 4  @align= 0  @byteOffsetOf= 6  @bitOffsetOf= 50  → 1:2:1
};


// abi_size=8
// size_in_bits=58
// abi_align=1
const Foo = packed struct {
    a: u12,         // @bitSizeOf=12  @align= 0  @byteOffsetOf= 0  @bitOffsetOf=  0  → 1:0:2
    b: u1 align(1), // @bitSizeOf= 1  @align= 1  @byteOffsetOf= 2  @bitOffsetOf= 16  → 1:0:1
    c: u32,         // @bitSizeOf=32  @align= 0  @byteOffsetOf= 2  @bitOffsetOf= 17  → 1:1:5
    d: u4,          // @bitSizeOf= 4  @align= 0  @byteOffsetOf= 6  @bitOffsetOf= 49  → 1:1:1
    e: bool,        // @bitSizeOf= 1  @align= 0  @byteOffsetOf= 6  @bitOffsetOf= 53  → 1:5:1
    f: u4,          // @bitSizeOf= 4  @align= 0  @byteOffsetOf= 6  @bitOffsetOf= 54  → 1:6:2
};


// abi_size=8
// size_in_bits=58
// abi_align=2
const Foo = packed struct {
    a: u12,         // @bitSizeOf=12  @align= 0  @byteOffsetOf= 0  @bitOffsetOf=  0  → 2:0:2
    b: u1 align(2), // @bitSizeOf= 1  @align= 2  @byteOffsetOf= 2  @bitOffsetOf= 16  → 1:0:1
    c: u32,         // @bitSizeOf=32  @align= 0  @byteOffsetOf= 2  @bitOffsetOf= 17  → 2:1:6
    d: u4,          // @bitSizeOf= 4  @align= 0  @byteOffsetOf= 6  @bitOffsetOf= 49  → 1:1:1
    e: bool,        // @bitSizeOf= 1  @align= 0  @byteOffsetOf= 6  @bitOffsetOf= 53  → 1:5:1
    f: u4,          // @bitSizeOf= 4  @align= 0  @byteOffsetOf= 6  @bitOffsetOf= 54  → 1:6:2
};


// abi_size=12
// size_in_bits=74
// abi_align=4
const Foo = packed struct {
    a: u12,         // @bitSizeOf=12  @align= 0  @byteOffsetOf= 0  @bitOffsetOf=  0  → 2:0:2
    b: u1 align(4), // @bitSizeOf= 1  @align= 4  @byteOffsetOf= 4  @bitOffsetOf= 32  → 1:0:1
    c: u32,         // @bitSizeOf=32  @align= 0  @byteOffsetOf= 4  @bitOffsetOf= 33  → 4:1:8
    d: u4,          // @bitSizeOf= 4  @align= 0  @byteOffsetOf= 8  @bitOffsetOf= 65  → 1:1:1
    e: bool,        // @bitSizeOf= 1  @align= 0  @byteOffsetOf= 8  @bitOffsetOf= 69  → 1:5:1
    f: u4,          // @bitSizeOf= 4  @align= 0  @byteOffsetOf= 8  @bitOffsetOf= 70  → 1:6:2
};  


// abi_size=16
// size_in_bits=106
// abi_align=8
const Foo = packed struct {
    a: u12,         // @bitSizeOf=12  @align= 0  @byteOffsetOf= 0  @bitOffsetOf=  0  → 2:0:2
    b: u1 align(8), // @bitSizeOf= 1  @align= 8  @byteOffsetOf= 8  @bitOffsetOf= 64  → 1:0:1
    c: u32,         // @bitSizeOf=32  @align= 0  @byteOffsetOf= 8  @bitOffsetOf= 65  → 4:1:8
    d: u4,          // @bitSizeOf= 4  @align= 0  @byteOffsetOf=12  @bitOffsetOf= 97  → 1:1:1
    e: bool,        // @bitSizeOf= 1  @align= 0  @byteOffsetOf=12  @bitOffsetOf=101  → 1:5:1
    f: u4,          // @bitSizeOf= 4  @align= 0  @byteOffsetOf=12  @bitOffsetOf=102  → 1:6:2
};  

@mikdusan
Copy link
Member

mikdusan commented Feb 19, 2020

Here's a packed struct with very inefficient field sizes. One thing that I need to point out is this shows how overlapped the host integers are. Is overlapped ok? If it is I think codegen will be using different <{ ... }> for each field access? If overlapping host integers aren't allowed, I'll need some more examples to understand this.

// abi_size=79
// size_in_bits=630
// abi_align=1
const Foo = packed struct {
    a: u63,          // @bitSizeOf=63  @align= 0  @byteOffsetOf= 0  @bitOffsetOf=  0  → 1:0:8
    b: u63,          // @bitSizeOf=63  @align= 0  @byteOffsetOf= 7  @bitOffsetOf= 63  → 1:7:9
    c: u63,          // @bitSizeOf=63  @align= 0  @byteOffsetOf=15  @bitOffsetOf=126  → 1:6:9
    d: u63,          // @bitSizeOf=63  @align= 0  @byteOffsetOf=23  @bitOffsetOf=189  → 1:5:9
    e: u63,          // @bitSizeOf=63  @align= 0  @byteOffsetOf=31  @bitOffsetOf=252  → 1:4:9
    f: u63,          // @bitSizeOf=63  @align= 0  @byteOffsetOf=39  @bitOffsetOf=315  → 1:3:9
    g: u63,          // @bitSizeOf=63  @align= 0  @byteOffsetOf=47  @bitOffsetOf=378  → 1:2:9
    h: u63,          // @bitSizeOf=63  @align= 0  @byteOffsetOf=55  @bitOffsetOf=441  → 1:1:8
    i: u63,          // @bitSizeOf=63  @align= 0  @byteOffsetOf=63  @bitOffsetOf=504  → 1:0:8
    j: u63,          // @bitSizeOf=63  @align= 0  @byteOffsetOf=70  @bitOffsetOf=567  → 1:7:9
};  

// abi_size=80
// size_in_bits=630
// abi_align=8
const Foo = packed struct {
    a: u63 align(8), // @bitSizeOf=63  @align= 8  @byteOffsetOf= 0  @bitOffsetOf=  0  → 8:0:8
    b: u63,          // @bitSizeOf=63  @align= 0  @byteOffsetOf= 0  @bitOffsetOf= 63  → 8:63:16
    c: u63,          // @bitSizeOf=63  @align= 0  @byteOffsetOf= 8  @bitOffsetOf=126  → 8:62:16
    d: u63,          // @bitSizeOf=63  @align= 0  @byteOffsetOf=16  @bitOffsetOf=189  → 8:61:16
    e: u63,          // @bitSizeOf=63  @align= 0  @byteOffsetOf=24  @bitOffsetOf=252  → 8:60:16
    f: u63,          // @bitSizeOf=63  @align= 0  @byteOffsetOf=32  @bitOffsetOf=315  → 8:59:16
    g: u63,          // @bitSizeOf=63  @align= 0  @byteOffsetOf=40  @bitOffsetOf=378  → 8:58:16
    h: u63,          // @bitSizeOf=63  @align= 0  @byteOffsetOf=48  @bitOffsetOf=441  → 8:57:16
    i: u63,          // @bitSizeOf=63  @align= 0  @byteOffsetOf=56  @bitOffsetOf=504  → 8:56:16
    j: u63,          // @bitSizeOf=63  @align= 0  @byteOffsetOf=64  @bitOffsetOf=567  → 8:55:16
};  

@s-ol
Copy link
Contributor

s-ol commented Oct 7, 2020

Not sure if this is 100% related, but I was expecting this to work on 0.6.0:

comptime {
        const type = packed struct {
                asd: [8]u1,
        };
        @compileLog(@sizeOf(type));
}

...but error: array of 'u1' not allowed in packed struct due to padding bits (same for array of booleans).

This would be very nice to have though for memory-mapped IO; right now something like this:

// written from memory, dont use
const IOPort = packed struct {
  ddr0: bool,
  ddr1: bool,
  ...
  ddr7: bool,
  pin0: bool,
  pin1: bool,
  ...
  pin7: bool,
  port0: bool,
  port1: bool,
  ...
  port7: bool,
}
const PORTB = @intToPtr(*volatile IOPort, 0x28);

works great on AVR and compiles bit access to optimized single-bit instructions (set-bit, clear-bit), but it doesn't lend itself to meta-programming because you cannot (easily) lookup a specific pin given (port: *volatile IOPort, pin_index: u3).

@andrewrk andrewrk removed this from the 0.7.0 milestone Oct 9, 2020
@andrewrk
Copy link
Member Author

Rejected in favor of #5049 and #10113.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Projects
Status: To do
Development

No branches or pull requests

4 participants