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

Simplify and compact switch ZIR, and resolve union payload captures with PTR #15880

Merged
merged 8 commits into from
Jun 13, 2023

Conversation

mlugg
Copy link
Member

@mlugg mlugg commented May 28, 2023

While looking to implement #2473 (Coming Soon ™️), I found that the existing representation of switch expressions in ZIR was incompatible with that proposal. Further inspection led me to conclude that the existing representation in ZIR is unnecessarily large, and also uses far more tags than it needs to.

All but the last commit of this PR are dedicated to simplifying this ZIR. The 8 tags used in master for switch expressions have been reduced down to 2, and the size in memory of the ZIR for switch expressions is decreased across the board (stats below).

The new encoding has two instructions: switch_block and switch_block_ref, where the latter is for passing the operand by reference. Some bits in the trailing data are repurposed to provide information about captures. The main capture is referenced through the switch block instruction itself (since this otherwise can't be referenced inside a prong), and inline tag captures, if present, are referenced through a dummy instruction, like how errdefer captures work.

The final commit of this PR implements #2812. See its commit message for a bit more information on the strategy there.

ZIR stats (prior to proposal implementation, which will not affect the ZIR):

File Old New Change
test/behavior/inline_switch.zig 20.26K 19.39K -4.3%
test/behavior/switch.zig 80.58K 77.20K -4.2%
src/Sema.zig 4.57M 4.50M -1.4%
src/value.zig 795.33K 768.85K -3.3%

@mlugg mlugg requested a review from kristoff-it as a code owner May 28, 2023 01:07
@mlugg mlugg force-pushed the feat/better-switch-zir-2 branch 3 times, most recently from ad0c2ef to b6f22fc Compare May 30, 2023 06:14
mlugg added 8 commits June 13, 2023 12:42
By indexing from the very first switch case rather than into scalar and
multi cases separately, the instructions for capturing in multi cases
become unnecessary, freeing up 2 ZIR tags.
This is in preparation for ziglang#2473. Also fixes a bug where switching on
bools allows invalid case combinations.
These tags are unnecessary, as this information can be more efficiently
encoded within the switch_block instruction itself. We also use a neat
little trick to avoid needing a dummy instruction (like is used for
errdefer captures): since the switch_block itself cannot otherwise be
referenced within a prong, we can repurpose its index within prongs to
refer to the captured value.
This is a follow-up to a previous commit which eliminated switch_capture
and switch_capture_ref. All captures are now handled directly by
`switch_block`, which has also eliminated some unnecessary Block data in
Sema.
This finishes the process of consolidating switch expressions in ZIR
into as simple and compact a representation as is possible. There are
now just two ZIR tags dedicated to switch expressions: switch_block and
switch_block_ref, with the latter being for an operand passed by
reference.
This is a bit harder than it seems at first glance. Actually resolving
the type is the easy part: the interesting thing is actually getting the
capture value. We split this into three cases:

* If all payload types are the same (as is required in status quo), we
  can just do what we already do: get the first field value.
* If all payloads are in-memory coercible to the resolved type, we still
  fetch the first field, but we also emit a `bitcast` to convert to the
  resolved type.
* Otherwise, we need to handle each case separately. We emit a nested
  `switch_br` which, for each possible case, gets the corresponding
  union field, and coerces it to the resolved type. As an optimization,
  the inner switch's 'else' prong is used for any peer which is
  in-memory coercible to the target type, and the bitcast approach
  described above is used.

Pointer captures have the additional constraint that all payload types
must be in-memory coercible to the resolved type.

Resolves: ziglang#2812
To do this, I expanded SwitchProngSrc a bit. Several of the tags there
aren't actually used by any current errors, but they're there for
consistency and if we ever need them.

Also delete a now-redundant test and fix another.
@mlugg mlugg force-pushed the feat/better-switch-zir-2 branch from b6f22fc to 42dc753 Compare June 13, 2023 12:19
@andrewrk andrewrk enabled auto-merge June 13, 2023 15:28
@andrewrk andrewrk merged commit df63194 into ziglang:master Jun 13, 2023
@andrewrk
Copy link
Member

perf data point for self-hosted compiler (2% slower):

Benchmark 1: before/bin/zig build-exe --stack 33554432 /home/andy/dev/zig/src/main.zig --cache-dir /home/andy/dev/zig/zig-cache --global-cache-dir /home/andy/.cache/zig --name zig --mod build_options::/home/andy/dev/zig/zig-cache/c/58a1564dccbec7b9340b137525e685a4/options.zig --deps build_options --zig-lib-dir /home/andy/dev/zig/lib -fno-emit-bin
  Time (mean ± σ):      7.018 s ±  0.083 s    [User: 6.911 s, System: 0.185 s]
  Range (min … max):    6.953 s …  7.112 s    3 runs
 
Benchmark 2: after/bin/zig build-exe --stack 33554432 /home/andy/dev/zig/src/main.zig --cache-dir /home/andy/dev/zig/zig-cache --global-cache-dir /home/andy/.cache/zig --name zig --mod build_options::/home/andy/dev/zig/zig-cache/c/58a1564dccbec7b9340b137525e685a4/options.zig --deps build_options --zig-lib-dir /home/andy/dev/zig/lib -fno-emit-bin
  Time (mean ± σ):      7.136 s ±  0.030 s    [User: 7.043 s, System: 0.168 s]
  Range (min … max):    7.105 s …  7.164 s    3 runs
 
Summary
  'before/bin/zig build-exe --stack 33554432 /home/andy/dev/zig/src/main.zig --cache-dir /home/andy/dev/zig/zig-cache --global-cache-dir /home/andy/.cache/zig --name zig --mod build_options::/home/andy/dev/zig/zig-cache/c/58a1564dccbec7b9340b137525e685a4/options.zig --deps build_options --zig-lib-dir /home/andy/dev/zig/lib -fno-emit-bin' ran
    1.02 ± 0.01 times faster than 'after/bin/zig build-exe --stack 33554432 /home/andy/dev/zig/src/main.zig --cache-dir /home/andy/dev/zig/zig-cache --global-cache-dir /home/andy/.cache/zig --name zig --mod build_options::/home/andy/dev/zig/zig-cache/c/58a1564dccbec7b9340b137525e685a4/options.zig --deps build_options --zig-lib-dir /home/andy/dev/zig/lib -fno-emit-bin'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants