-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
langref: improve packed struct memory layout description #21741
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I have a suggestion and a question, but this is an improvement without addressing either of those.
doc/langref.html.in
Outdated
</li> | ||
<li>The backing integer has the same bit count as the fields' total bit width.</li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In "The backing integer has the same bit count as the fields' total bit width", should this say "must have" instead of "has"? (Also maybe add "If not inferred, ..."?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think "must have" is best here because it communicates a constraint that the user must comply with, but the backing integer is constructed by the compiler for the user, so the user doesn't really need to do anything if they don't want to.
"Has" here just communicates a fact about the backing integer that is useful when communicating the memory layout.
Later in the doc, inferring vs. specifying the backing integer is explained as a readability and program correctness assertion feature, which I think is the correct way to explain it:
The backing integer can be inferred or explicitly provided. When inferred it will be unsigned, and when explicitly provided, its bit count will be enforced at compile time:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated inferred vs explicitly provided sentence
to read:
The backing integer can be inferred or explicitly provided. When inferred, it will be unsigned. When explicitly provided, its bit width will be enforced at compile time to exactly match the total bit width of the fields:
doc/langref.html.in
Outdated
<li>{#syntax#}bool{#endsyntax#} fields use exactly 1 bit.</li> | ||
<li>An {#link|enum#} field uses exactly the bit width of its integer tag type.</li> | ||
<li>A {#link|packed union#} field uses exactly the bit width of the union field with | ||
the largest bit width.</li> | ||
<li>Packed structs, when they themselves are within a packed struct, will only use as many bits as their backing integer.</li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add anything to this doc about backing ints that are not a multiple of 8 bits?
(Perhaps this is mostly relevant in the MMIO case, where users wouldn't want undefined bits, and the load/store operations are going to be an 8-bit multiple? Hrm, and would a u24 get rounded up to a u32?)
The PR is still a doc improvement without this detail, so feel free to leave it off.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add anything to this doc about backing ints that are not a multiple of 8 bits?
This is intended to be captured by:
"The backing integer is subject to the same rules as any integer, including alignment,"
(Perhaps this is mostly relevant in the MMIO case, where users wouldn't want undefined bits, and the load/store operations are going to be an 8-bit multiple? Hrm, and would a u24 get rounded up to a u32?)
I suspect you are correct about its relevance to the MMIO case, and just below in the docs it talks about using overaligned pointers to override the alignment of the packed struct, which I guess is what the MMIO use case needs to do to avoid writing/reading to memory areas that are just padding for alignment?
"Packed structs have the same alignment as their backing integer, however, overaligned pointers to packed structs can override this:"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add anything to this doc about backing ints that are not a multiple of 8 bits?
In case you were referring to packed struct fields inside a packed struct,
6c330be
adds an example to to help explain this case.
I found an article about MMIO in zig: https://www.scattered-thoughts.net/writing/mmio-in-zig/ However, this was written in 2021, and integer-backed packed structs landed in 2022: #5049 (comment) I will attempt to tailor the information into some recommendation for MMIO. Right now, I am thinking:
But I won't be able to make a doc-test that actually uses |
MMIO example needs close review from someone familiar with this use-case. It is inspired by examples in the microzig framework: |
Looks even better to me now! I like the MMIO text and example, and while it makes sense to me, I don't have any particular experience with them to validate it. In the MMIO example, I'm torn between wanting to see more documentation about the |
Most CPUs (except rare old old ones) cannot individually change bits in memory. They can usually only read / write from the smallest addressable unit of memory, which is usually 1 byte. In this example, a micro controller would typically have a special memory address that can be written to manipulate GPIO (general purpose input and output). This example assumes the user has read the microcontroller datasheet, found an address of a register ( The thing to be careful about is not letting the reads / writes from memory to be optimized away (hence the In the example, the entire byte at address |
Technically I could remove the |
intended to close
#14384
#20647
Questions:
1. I don't know what to do about this TODO:> TODO update these docs with a recommendation on how to use packed structs with MMIO> (the use case for volatile packed structs) once this issue is resolved.> Don't worry, there will be a good solution for this use case in zig.Looks like this: