packed struct produces byte stores instead of word stores

[markfirmware]

See https://github.com/markfirmware/zig-bare-metal-microbit/blob/7d7b6d23f49c454a36cf23ff7fa8806fd113ac1b/lib00_basics.zig#L384

Good - not a packed struct, does a 32-bit word store

0000000000009388 mission03_model_railroad_button_controlled_pwm.Throttle.prepare:
; fn prepare(self: *Throttle) void {
    9388:	81 b0 	sub	sp, #4
    938a:	00 90 	str	r0, [sp]
    938c:	02 48 	ldr	r0, [pc, #8]
    938e:	02 21 	movs	r1, #2
; short_cuts.shorts = 0x002;
    9390:	01 60 	str	r1, [r0]
; fn prepare(self: *Throttle) void {
    9392:	01 b0 	add	sp, #4
    9394:	70 47 	bx	lr
    9396:	c0 46 	mov	r8, r8

0000000000009398 $d.425:
    9398:	00 92 00 40 	.word	0x40009200

Bad - changing that struct to packed struct, it does a sequence of (4) byte stores. Does not work because mmio needs a word write.

0000000000009388 mission03_model_railroad_button_controlled_pwm.Throttle.prepare:
; fn prepare(self: *Throttle) void {
    9388:	81 b0 	sub	sp, #4
    938a:	00 90 	str	r0, [sp]
    938c:	05 48 	ldr	r0, [pc, #20]
    938e:	00 21 	movs	r1, #0
; short_cuts.shorts = 0x002;
    9390:	01 70 	strb	r1, [r0]
    9392:	05 48 	ldr	r0, [pc, #20]
    9394:	01 70 	strb	r1, [r0]
    9396:	05 48 	ldr	r0, [pc, #20]
    9398:	01 70 	strb	r1, [r0]
    939a:	05 48 	ldr	r0, [pc, #20]
    939c:	02 21 	movs	r1, #2
    939e:	01 70 	strb	r1, [r0]
; fn prepare(self: *Throttle) void {
    93a0:	01 b0 	add	sp, #4
    93a2:	70 47 	bx	lr

00000000000093a4 $d.425:
    93a4:	03 92 00 40 	.word	0x40009203
    93a8:	02 92 00 40 	.word	0x40009202
    93ac:	01 92 00 40 	.word	0x40009201
    93b0:	00 92 00 40 	.word	0x40009200

[daurnimator]

packed structs have alignment 1. You need to change it to align(4).

[markfirmware]

Thank you - will try.

[andrewrk]

Is this the same as #1761 or #1834 ?

[markfirmware]

@daurnimator I tried

pub fn io(address: u32, comptime StructType: type) *volatile StructType {                                                                   
    return @intToPtr(*align(4) volatile StructType, address);
}

but I get the same results.

@andrewrk this comment #1834 (comment) points to a gist https://gist.github.com/nic-donaldson/21abc0f0b9a83d3f4e96a5bbb55c9290 that exhibits the same store-byte sequence.

My case includes exactly one u32 which I think is different from and simpler than any of the other reports that are trying to use small bit fields that are not multiples of bytes.

[markfirmware]

@daurnimator are you sure that align(4) will make it work? I just tried 0.5.0+83f6f730c and have the same results.

@vegecode @andrewrk

[justinbalexander]

How does this look to you?

https://zig.godbolt.org/z/AsAfSh

This was the example you referenced in the comment but updated. If there is a bug here it is that if you try to declare the const value inline with the write instead of on its own line, it produces byte stores instead of a word store.

Does Zig plan to make guarantees as to how structs are copied though? Even packed structs?

Oh wait, I see that your struct only has one field that is a u32, so why is it producing byte stores of that one field if the align attribute is added. I'll investigate some more.

[justinbalexander]

This example works in every mode except debug:

https://zig.godbolt.org/z/FbUvWt

[markfirmware]

Does Zig plan to make guarantees as to how structs are copied though? Even packed structs?

Good question.

This example works in every mode except debug:
https://zig.godbolt.org/z/FbUvWt

Ok, I'll have to consider the implications. Thanks!

[ifreund]

This should be well supported now that #5049 is implemented and packed structs use the same semantics as their backing integer type.