ziglang / zig

General-purpose programming language and toolchain for maintaining robust, optimal, and reusable software.
https://ziglang.org
MIT License
34.57k stars 2.53k forks source link

Packed struct has unexpected RMW/Multiple writes on arm. #21249

Open taylorh140 opened 1 month ago

taylorh140 commented 1 month ago

Zig Version

0.13.0

Steps to Reproduce and Observed Behavior

I notice when setting registers. Specifically shadowed registers meaning that when you write to them they do not reflect that setting until some event occurs. And in my case it was even worse because they would only take the last value set to any part of the register which made this ... uncomfortable for a minute.

The Type I'm using here is a *volatile to the following item.

            ///  Layerx Window Horizontal Position Configuration Register
            L1WHPCR: packed struct(u32) {
                ///  Window Horizontal Start Position
                WHSTPOS: u12,
                reserved16: u4 = 0,
                ///  Window Horizontal Stop Position
                WHSPPOS: u12,
                padding: u4 = 0,
            }

specifically I used the following assignment expecting it to be a single assignment operator.

self.L1WHPCR = .{ .WHSTPOS = WX0 + self.BPCR.AHBP + 1, .WHSPPOS = WX1 + self.BPCR.AHBP }; //Window horizontal stop start

which seems to have mutltiple rmw cycles: image

where as this code which i would expect would do the same thing only writes to the register once:

const tmp_L1WHCPR: @TypeOf(self.L1WHPCR) = .{ .WHSTPOS = WX0 + self.BPCR.AHBP + 1, .WHSPPOS = WX1 + self.BPCR.AHBP }; //Window horizontal stop start
self.L1WHPCR = tmp_L1WHCPR;

image

however the above is my in code observation. I'm going to see if i can reproduce it on Godbolt to help verify.

https://godbolt.org/z/dhbb5rGWo

However here it looks like the operation is broken into multiple writes. (but I would still expect one write of u32 size).

Expected Behavior

since the packed struct is u32 size i expect the write to be a single str.w (word size store) and all the other operations rather they be rmw or byte size accumlations to happen in register land.

rohlem commented 1 month ago

Related to Result Location Semantics, first reported in https://github.com/ziglang/zig/issues/3696 and specifically re-discussed in https://github.com/ziglang/zig/issues/12064 . (Most recent main RLS discussion is in https://github.com/ziglang/zig/issues/12251.)

In the design of status-quo, the idea was for assignment from a .{ ... } anonymous struct literal not to reserve the stack space for an entire struct instance, so instead each field is modeled as an individual assignment to the corresponding field (via the result location pointer). (Seems like that design mainly considered large structs, say > 1kiB, not something that fits into a u32.) However, this (imo more often than not) results in surprising behavior, because syntactically the programmer only wrote one = sign, and the right hand side is "wrapped" by {}, so splitting up of evaluation and assignment is generally unexpected.

For now if you want a single assignment, the workaround as you've discovered is to declare a separate const or var as stack location and assign from there. To solve the exact use case either packed struct or volatile pointers would have to force different behavior. Imo the general behavior should change for all compound types, however discussion about this is still ongoing (see other linked issues).