Skip to content

introduce a "try" ZIR and AIR instruction #11783

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

Merged
merged 11 commits into from
Jun 6, 2022
Merged

introduce a "try" ZIR and AIR instruction #11783

merged 11 commits into from
Jun 6, 2022

Conversation

andrewrk
Copy link
Member

@andrewrk andrewrk commented Jun 3, 2022

Implements #11772.

Merge Checklist:

  • implement support in the backends
    • x86_64
    • wasm
    • aarch64
    • arm
    • riscv64 not covered by CI tests
    • sparc64 not covered by CI tests
    • C
  • Sema: avoid emitting unused is_non_error AIR instruction

Analysis

Note: master branch at time of writing is e498fb1.

It did not close the bloat gap (#11498). File size stats after this change:

self-hosted binary size, LLVM backend, -Drelease, -Dstrip, LLVM disabled:
  master:
    stage3: 8370744 bytes
    stage2: 6963432 bytes
  this branch:
    stage3: 8343144 bytes
    stage2: 6939800 bytes

So it seems that this branch produces a slightly smaller executable (a delta of 25 KB) in both cases, but did not change the relative size difference.

However it did have a small improvement on how long it took to build in release mode with LLVM:

building self-hosted, LLVM backend, -Drelease, -Dstrip, LLVM disabled:
  master: 
    stage1 build stage2: 3m28.3s
    stage2 build stage3: 3m16.4s
    stage3 build stage3: 3m14.4s
  this branch:
    stage1 build stage2: 3m27.8s
    stage2 build stage3: 3m10.6s
    stage3 build stage3: 3m07.7s

Here's the measurement that should probably be used to decide whether this patch is overall an improvement:

building self-hosted, LLVM backend, debug mode, LLVM disabled:
  master: 
    stage1 build stage2: 0m34.3s
    stage2 build stage3: 0m33.1s
    stage3 build stage3: error: AccessDenied
  this branch:
    stage1 build stage2: 0m30.7s
    stage2 build stage3: 0m31.4s
    stage3 build stage3: error: AccessDenied

I keep getting error: AccessDenied intermittently when running stage3 and I have not diagnosed it yet. This is tracked in #11450. It's unfortunate here because the stage3 measurement is what we are most interested in.

Here's the same measurement but with -Denable-llvm which apparently does not trip the "access denied" error:

building self-hosted, LLVM backend, debug mode, -Denable-llvm
  master: 
    stage1 build stage2: 0m51.0s
    stage2 build stage3: 0m45.9s
    stage3 build stage3: 0m43.7s
  this branch:
    stage1 build stage2: 0m50.8s
    stage2 build stage3: 0m44.5s
    stage3 build stage3: 0m42.4s

Finally, let's look at perf of debug builds:

building self-hosted using debug mode, LLVM backend, debug mode, LLVM disabled:
  master:
    stage2 build stage3: 1m16.9s
    stage3 build stage3: 1m15.0s
  this branch:
    stage2 build stage3: 1m16.3s
    stage3 build stage3: 1m13.8s

Looking at the optimized LLVM IR, did buildOutputType get the desired cleanup pattern?

❌ no

Conclusion

Overall, this seems to have had a small, but noticeable impact. Given that it produces better runtime code for a common zig syntax in debug builds, I am inclined to press forward with this change.

@Luukdegram
Copy link
Contributor

Implemented for Wasm backend in 7fb9650

@andrewrk
Copy link
Member Author

andrewrk commented Jun 3, 2022

This was a pretty big win for the C backend:

behavior test C backend line count:
       master: 72846 lines
  this branch: 60361 lines

@Luukdegram
Copy link
Contributor

Luukdegram commented Jun 4, 2022

Some stats on those changes for the Wasm backend:

behavior test Wasm backend total instruction byte size (code section):
     master: 529749 bytes
this branch: 518855 bytes

This means an improvement of 10894 bytes worth of instructions.

@kubkon
Copy link
Member

kubkon commented Jun 5, 2022

Some stats on those changes for the x64 backend:

behavior test total byte size (entire binary):
master: 2497585 bytes
this branch: 2484940 bytes

This means an improvement of 12645 bytes in total.

andrewrk and others added 9 commits June 5, 2022 10:37
This introduces two ZIR instructions:
 * `try`
 * `try_inline`

This is part of an effort to implement #11772.
Implements semantic analysis for the new try/try_inline ZIR
instruction. Adds the new try/try_ptr AIR instructions and implements
them for the LLVM backend.

Fixes not calling rvalue() for tryExpr in AstGen.

This is part of an effort to implement #11772.
 * Introduce "_ptr" variants of ZIR try instruction to disallow constructs
   such as `try` on a pointer value instead of an error union value.
 * Disable the "_inline" variants of the ZIR try instruction for now because
   we are out of ZIR tags. I will free up some space in an independent commit.
 * AstGen: fix tryExpr calling rvalue() on ResultLoc.ref
This function took is_ptr: bool and then branched on it three times.
Now, instead, each implementation does no branching and the logic is
easier to follow, both for maintainers and compilers.

I also fixed a bug with TryPtr not ensuring enough capacity in the extra
array.
@joachimschmidt557
Copy link
Contributor

Reduction in size for ARM (behavior test binary):

master: 1398205 bytes
stage2-try: 1344952 bytes

reduction: 53253 bytes = 0.0380866897200339 (about 3.8%)

@andrewrk andrewrk merged commit d1bfc83 into master Jun 6, 2022
@andrewrk andrewrk deleted the stage2-try branch June 6, 2022 23:01
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.

4 participants