compiler: move struct types into InternPool proper#17172
Merged
Conversation
andrewrk
force-pushed
the
ip-structs
branch
3 times, most recently
from
dc7d75a to
76f0557
Compare
Structs were previously using `SegmentedList` to be given indexes, but were not actually backed by the InternPool arrays. After this, the only remaining uses of `SegmentedList` in the compiler are `Module.Decl` and `Module.Namespace`. Once those last two are migrated to become backed by InternPool arrays as well, we can introduce state serialization via writing these arrays to disk all at once. Unfortunately there are a lot of source code locations that touch the struct type API, so this commit is still work-in-progress. Once I get it compiling and passing the test suite, I can provide some interesting data points such as how it affected the InternPool memory size and performance comparison against master branch. I also couldn't resist migrating over a bunch of alignment API over to use the log2 Alignment type rather than a mismash of u32 and u64 byte units with 0 meaning something implicitly different and special at every location. Turns out you can do all the math you need directly on the log2 representation of alignments.
This also modifies AstGen so that struct types use 1 bit each from the flags to communicate if there are nonzero inits, alignments, or comptime fields. This allows adding a struct type to the InternPool without looking ahead in memory to find out the answers to these questions, which is easier for CPUs as well as for me, coding this logic right now.
for the new struct and packed struct encodings.
Not sure what that code was supposed to be doing, it doesn't seem to be reachable.
Let's try to reduce the explosive scope of this branch.
All of the logic in `Value.elemValue` is quite questionable, but printing an error is definitely better than crashing. Notably, this should stop us from hitting crashes when dumping AIR.
We're hitting false compile errors, but this is progress!
Carve out a path forward for being a little bit more intentional about handling "default" alignment values.
Previously it would canonicalize or not depending on some volatile internal state of the compiler, now it forces resolution of the element type to determine the alignment if it needs to.
This changeset fixes the handling of alignment in several places. The new rules are: * `@alignOf(T)` where `T` is a runtime zero-bit type is at least 1, maybe greater. * Zero-bit fields in `extern` structs *do* force alignment, potentially offsetting following fields. * Zero-bit fields *do* have addresses within structs which can be observed and are consistent with `@offsetOf`. These are not necessarily all implemented correctly yet (see disabled test), but this commit fixes all regressions compared to master, and makes one new test pass.
This is an alternate version of c675daa
Zero-byte alignment is no longer valid for runtime types. I made most of these changes in an earlier commit, but missed this case.
andrewrk
force-pushed
the
ip-structs
branch
from
f3f00f4 to
2e5d13e
Compare
Member
Author
|
ghostty perf data point: self-hosted compiler perf data point: |
When struct types have no field names, the names are implicitly understood to be strings corresponding to the field indexes in declaration order. It used to be the case that a NullTerminatedString would be stored for each field in this case, however, now, callers must handle the possibility that there are no names stored at all. This commit introduces `legacyStructFieldName`, a function to fake the previous behavior. Probably something better could be done by reworking all the callsites of this function.
Gotta call the get() function inside the loop if the loop adds anything to InternPool.
It seems the webassembly backend does not want the exception that `structFieldAlignmentExtern` makes for 128-bit integers. Perhaps that logic should be modified to check if the target is wasm. Without this, this branch fails the C ABI tests for wasm, causing this: ``` wasm-ld: warning: function signature mismatch: zig_struct_u128 >>> defined as (i64, i64) -> void in cfuncs.o >>> defined as (i32) -> void in test-c-abi-wasm32-wasi-musl-ReleaseFast.wasm.o ```
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Structs were previously using
SegmentedListto be given indexes, but were not actually backed by the InternPool arrays.After this, the only remaining uses of
SegmentedListin the compiler areModule.DeclandModule.Namespace. Once those last two are migrated to become backed by InternPool arrays as well, we can introduce state serialization via writing these arrays to disk all at once.Unfortunately there are a lot of source code locations that touch the struct type API, so this commit is still work-in-progress. Once I get it compiling and passing the test suite, I can provide some interesting data points such as how it affected the InternPool memory size and performance comparison against master branch.
I also couldn't resist migrating over a bunch of alignment API over to use the log2 Alignment type rather than a mismash of u32 and u64 byte units with 0 meaning something implicitly different and special at every location. Turns out you can do all the math you need directly on the log2 representation of alignments.
This is a sub-task of the Performance Project.