jameysharp opened PR #8438 from jameysharp:drop-unused-args
to bytecodealliance:main
:
Before this, Cranelift ABI code would emit a stack-load instruction for every stack argument and add all register arguments to the
args
pseudo-instruction, whether those arguments were used or not.However, we already know which arguments are used at that point because we need the analysis for load-sinking, so it's easy to filter the unused arguments out.
This avoids generating loads that are immediately dead, which is good for the generated code. It also slightly reduces the size of the register allocation problem, which is a small win in compile time.
This also changes which registers RA2 chooses in some cases because it no longer considers unused defs from the
args
pseudo-instruction.There was an existing method named
arg_is_needed_in_body
which sounded like it should be the right place to implement this. However, that method was only used for Baldrdash integration and has been a stub since that integration was removed in #4571. Also it didn't have access to thevalue_ir_uses
map needed here. But the place where that method was called does have access to that map and was perfect for this.Thanks to @elliottt for doing the initial investigation of this change with me.
jameysharp requested abrown for a review on PR #8438.
jameysharp requested wasmtime-compiler-reviewers for a review on PR #8438.
jameysharp requested alexcrichton for a review on PR #8438.
jameysharp requested wasmtime-core-reviewers for a review on PR #8438.
cfallin submitted PR review:
Super-clean precision change for a nice result -- thanks very much for thinking through this! I'm excited to see the useless loads go away in a bunch of functions.
jameysharp commented on PR #8438:
So
cargo test -p wasmtime engine::serialization::test::cache_accounts_for_opt_level
fails with this PR. The panic message is:thread '<unnamed>' panicked at cranelift/codegen/src/machinst/compile.rs:76:14: register allocation: SSA(VReg(vreg = 192, class = Int), Inst(1))
This test simply compiles
(module (func))
several times with different optimization settings and with Wasmtime's cache enabled.From experimentation, this problem only seems to occur if Wasmtime's debug-info is enabled. I guess this is the only test that runs in that mode.
I thought
emit_value_label_markers_for_block_args
, or the special case to callemit_value_label_marks_for_value
on the vmctx parameter, seemed like likely candidates for telling RA2 to use an argument that's not otherwise used. But commenting out those calls doesn't fix this.I'm not sure how else to investigate this; any suggestions?
cfallin commented on PR #8438:
Can you dump the vcode from the failure? The most pertinent bit is figuring out what is still using the argument, I guess (as you suggest)...
jameysharp updated PR #8438.
jameysharp merged PR #8438.
Last updated: Dec 23 2024 at 12:05 UTC