compiler: split Decl into Nav and Cau
#20964
Merged
Conversation
This file contains 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
|
Holy line count, Batman! |
kubkon
force-pushed
the
the-great-decl-split-mk2
branch
from
e96b967
to
30cea80
Compare
jacobly0
force-pushed
the
the-great-decl-split-mk2
branch
from
f33ee13
to
1631b46
Compare
The type `Zcu.Decl` in the compiler is problematic: over time it has gained many responsibilities. Every source declaration, container type, generic instantiation, and `@extern` has a `Decl`. The functions of these `Decl`s are in some cases entirely disjoint. After careful analysis, I determined that the two main responsibilities of `Decl` are as follows: * A `Decl` acts as the "subject" of semantic analysis at comptime. A single unit of analysis is either a runtime function body, or a `Decl`. It registers incremental dependencies, tracks analysis errors, etc. * A `Decl` acts as a "global variable": a pointer to it is consistent, and it may be lowered to a specific symbol by the codegen backend. This commit eliminates `Decl` and introduces new types to model these responsibilities: `Cau` (Comptime Analysis Unit) and `Nav` (Named Addressable Value). Every source declaration, and every container type requiring resolution (so *not* including `opaque`), has a `Cau`. For a source declaration, this `Cau` performs the resolution of its value. (When ziglang#131 is implemented, it is unsolved whether type and value resolution will share a `Cau` or have two distinct `Cau`s.) For a type, this `Cau` is the context in which type resolution occurs. Every non-`comptime` source declaration, every generic instantiation, and every distinct `extern` has a `Nav`. These are sent to codegen/link: the backends by definition do not care about `Cau`s. This commit has some minor technically-breaking changes surrounding `usingnamespace`. I don't think they'll impact anyone, since the changes are fixes around semantics which were previously inconsistent (the behavior changed depending on hashmap iteration order!). Aside from that, this changeset has no significant user-facing changes. Instead, it is an internal refactor which makes it easier to correctly model the responsibilities of different objects, particularly regarding incremental compilation. The performance impact should be negligible, but I will take measurements before merging this work into `master`. Co-authored-by: Jacob Young <jacobly0@users.noreply.github.com> Co-authored-by: Jakub Konka <kubkon@jakubkonka.com>
Eliding the namespace when a container type has no decls was an experiment in saving memory, but it ended up causing more trouble than it was worth in various places. So, take the small memory hit for reified types, and just give every container type a namespace.
mlugg
force-pushed
the
the-great-decl-split-mk2
branch
from
1631b46
to
153e7d6
Compare
|
Performance tests are indicating that analysis performance suffers considerably, but codegen is improved enough to make up for it. I'm not surprised that codegen improved, but I am surprised at how significant these changes are (both for analysis and codegen). I'd be inclined to go ahead with merge, since for actual compilations this PR doesn't seem to represent a perf regression, but if @andrewrk is uncomfortable with the Analyze BehaviorBuild Behavior (x86_64-linux selfhosted)Analyze Hello WorldBuild Hello World (x86_64-linux selfhosted) |
|
can you take a data point for building the self-hosted compiler? behavior tests and hello world are a bit of outliers. |
|
These data points are for building the self-hosted compiler with
AnalyzeBuild (x86_64-linux selfhosted) |
mlugg
added a commit
to mlugg/zig
that referenced
this pull request
Aug 12, 2024
This fixes the regressions introduced by ziglang#20964. It also cleans up the "outdated file root" mechanism by virtue of deleting it. We now detect outdated file roots just after updating ZIR refs, and re-scan their namespaces. @jacobly0, this deletes some asserts in `InternPool` which didn't look appropriate to me, in order to deal with ziglang#20697.
mlugg
added a commit
to mlugg/zig
that referenced
this pull request
Aug 13, 2024
This commit makes more progress towards incremental compilation, fixing some crashes in the frontend. Notably, it fixes the regressions introduced by ziglang#20964. It also cleans up the "outdated file root" mechanism, by virtue of deleting it: we now detect outdated file roots just after updating ZIR refs, and re-scan their namespaces.
mlugg
added a commit
to mlugg/zig
that referenced
this pull request
Aug 17, 2024
This commit makes more progress towards incremental compilation, fixing some crashes in the frontend. Notably, it fixes the regressions introduced by ziglang#20964. It also cleans up the "outdated file root" mechanism, by virtue of deleting it: we now detect outdated file roots just after updating ZIR refs, and re-scan their namespaces.
jacobly0
pushed a commit
to mlugg/zig
that referenced
this pull request
Aug 17, 2024
This commit makes more progress towards incremental compilation, fixing some crashes in the frontend. Notably, it fixes the regressions introduced by ziglang#20964. It also cleans up the "outdated file root" mechanism, by virtue of deleting it: we now detect outdated file roots just after updating ZIR refs, and re-scan their namespaces.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The type
Zcu.Declin the compiler is problematic: over time it has gained many responsibilities. Every source declaration, container type, generic instantiation, and@externhas aDecl. The functions of theseDecls are in some cases entirely disjoint.After careful analysis, I determined that the two main responsibilities of
Declare as follows:Declacts as the "subject" of semantic analysis at comptime. A single unit of analysis is either a runtime function body, or aDecl. It registers incremental dependencies, tracks analysis errors, etc.Declacts as a "global variable": a pointer to it is consistent, and it may be lowered to a specific symbol by the codegen backend.This commit eliminates
Decland introduces new types to model these responsibilities:Cau(Comptime Analysis Unit) andNav(Named Addressable Value).Every source declaration, and every container type requiring resolution (so not including
opaque), has aCau. For a source declaration, thisCauperforms the resolution of its value. (When #131 is implemented, it is unsolved whether type and value resolution will share aCauor have two distinctCaus.) For a type, thisCauis the context in which type resolution occurs.Every non-
comptimesource declaration, every generic instantiation, and every distinctexternhas aNav. These are sent to codegen/link: the backends by definition do not care aboutCaus.This commit has some minor technically-breaking changes surrounding
usingnamespace. I don't think they'll impact anyone, since the changes are fixes around semantics which were previously inconsistent (the behavior changed depending on hashmap iteration order!).Aside from that, this changeset has no user-facing changes. Instead, it is an internal refactor which makes it easier to correctly model the responsibilities of different objects, particularly regarding incremental compilation. The performance impact should be negligible, but I will take measurements before merging this work into
master.