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

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

Open
taylorh140 opened this issue Aug 30, 2024 · 1 comment
Open

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

taylorh140 opened this issue Aug 30, 2024 · 1 comment
Labels
bug Observed behavior contradicts documented or intended behavior

Comments

@taylorh140
Copy link

taylorh140 commented Aug 30, 2024

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.

@taylorh140 taylorh140 added the bug Observed behavior contradicts documented or intended behavior label Aug 30, 2024
@rohlem
Copy link
Contributor

rohlem commented Aug 30, 2024

Related to Result Location Semantics, first reported in #3696 and specifically re-discussed in #12064 . (Most recent main RLS discussion is in #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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Observed behavior contradicts documented or intended behavior
Projects
None yet
Development

No branches or pull requests

2 participants