Optimize ISLE compilation

[bongjunj]

Overview

The current implementation of ISLE code generation emits a single Rust function for each ISLE term, and rustc compiles the Rust code to integrate ISLE with other modules of Cranelift.

This can cause a compilation bottleneck when a ISLE term contains numerous rules ending up with producing a very very large Rust function, since rustc cannot efficiently compile such functions. Notably, the term simplify which implements the mid-end peephole optimizations has about 500 rules, and the symptom is already visible. (And the ruleset is growing!) You can check, compiling Cranelift, cranelift-codegen takes most of the time in the following report: cargo-timing.html

Plus: a timing report for compiling wasmtime:

With this PR, the ISLE codegen helps rustc by wrapping a large match statement generated by islec in a closure. By introducing closures, a portion of a huge Rust function (for a ISLE term) can be split into several smaller compilation units for rustc, reducing the compilation time.

Evaluation

On the main branch, the build times for cranelift-codegen before/after this optimization are measured. This PR saves ~2 seconds on my machine (x86-64, 64 core, 512GB memory)

• Before: 23.81 sec

~/wasmtime (optimize-isle-compilation)> cargo clean
     Removed 10031 files, 12.6GiB total
~/wasmtime (optimize-isle-compilation)> time cargo build -q -p cranelift-codegen

________________________________________________________
Executed in   23.81 secs    fish           external
   usr time   43.59 secs    0.00 micros   43.59 secs
   sys time    5.04 secs  592.00 micros    5.04 secs

• After: 26.04 sec

~/wasmtime ((dev))> cargo clean
t     Removed 1356 files, 1.0GiB total
~/wasmtime ((dev))> time cargo build -q -p cranelift-codegen

________________________________________________________
Executed in   26.04 secs    fish           external
   usr time   46.58 secs    0.00 micros   46.58 secs
   sys time    5.15 secs  583.00 micros    5.15 secs

• Cargo Timing Reports
  • Before: cargo-timing-old.html
  • After: cargo-timing-new.html

Extra

I am experimenting with over 6,000 ISLE simplify rules.
The compilation time reduced from 841.37 seconds to 69.34 seconds with this PR. (+11x speedup)

~/w/cranelift ((many-rules-no-opt))> cargo clean && time cargo build -p cranelift-codegen
________________________________________________________
Executed in  841.37 secs    fish           external
   usr time  826.92 secs  325.00 micros  826.92 secs
   sys time   49.98 secs  135.00 micros   49.98 secs
   
 ~/w/cranelift (many-rules-opt)> cargo clean && time cargo build -q -p cranelift-codegen
________________________________________________________
Executed in   69.34 secs    fish           external
   usr time   98.32 secs  362.00 micros   98.32 secs
   sys time    6.42 secs  149.00 micros    6.42 secs

[cfallin]

@bongjunj thanks for this experimentation!

Before moving further, would you mind benchmarking compilation time as well? One of the reasons for putting all of the codegen for an ISLE term in one function is so that the compiler can optimize the code together; splitting that code between functions and (especially) introducing ABI boundaries may produce slowdowns when Cranelift runs. IMHO, it's worth it to spend a few extra seconds when compiling Cranelift if it makes Cranelift itself run faster. If no effect, of course, then no problem and I'll be happy to review this. Thanks!

[github-actions]

Subscribe to Label Action

cc @cfallin, @fitzgen

Details

This issue or pull request has been labeled: "cranelift", "isle"

Thus the following users have been cc'd because of the following labels:

• cfallin: isle
• fitzgen: isle

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

[alexcrichton]

An 11x reduction in compile time is quite massive, so even if this is a ~NN% regression in compile-time-of-wasm-code I think this would be a great thing to land at least for debug builds and maybe optionally for release builds with some sort of tunable too.

[cfallin]

Note, that's an 11x reduction with Bongjun's huge new ruleset; ~10% on main. That's neat but I wouldn't regress compile time for it IMHO.

[fitzgen]

If it does result in Wasm compilation time regressions, it may still make sense to have this as an option of the ISLE compiler that we can enable during development or something, if we can make the maintenance burden minimal.

[fitzgen]

Regarding my previous comment: it could potentially make sense to make MATCH_ARM_BODY_CLOSURE_THRESHOLD a dynamic ISLE compilation option, rather than a constant, and then tweak that value in Cranelift's invocation of the ISLE compiler depending on if a cargo feature is enabled or whether this is a release vs debug build of Cranelift or something.