-
Notifications
You must be signed in to change notification settings - Fork 19
Two loop-perf folds, and the wasm value-numbering defect the second one exposed #1061
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
Changes from all commits
738e10a
f28eb9a
b5bf3d3
fd45a4f
ac77a3b
d4dc416
d1d8d52
bacbd27
dcc7bef
a39b76c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -669,8 +669,10 @@ pub fn label_ref_capture_slots(inputargs: &[InputArg], ops: &[Op]) -> usize { | |
| LabelResumeData::collect(inputargs, ops).ref_slots | ||
| } | ||
|
|
||
| /// First free value position — one past the highest id any input arg or op | ||
| /// result occupies. `majit_gc::rewrite::remove_ref_constants` numbers the | ||
| /// First free value position — one past the highest id any value reference in | ||
| /// the trace occupies (input args, op results, and every op argument, including | ||
| /// a folded value the constants pool alone binds). | ||
| /// `majit_gc::rewrite::remove_ref_constants` numbers the | ||
| /// `LoadFromGcTable` results it emits from here upward, so the operand | ||
| /// numbering the optimizer produced stays untouched. Same id set | ||
| /// `collect_guards_and_vars` sizes `num_vars` from, so the loads land inside | ||
|
|
@@ -1303,12 +1305,25 @@ fn collect_guards_and_vars(inputargs: &[InputArg], ops: &[Op]) -> (Vec<GuardExit | |
| { | ||
| max_var = op.pos.get().raw() + 1; | ||
| } | ||
| if op.opcode == OpCode::Label { | ||
| for a in op.getarglist().iter() { | ||
| let a = a.to_opref(); | ||
| if a != OpRef::NONE && !a.is_constant() && a.raw() + 1 > max_var { | ||
| max_var = a.raw() + 1; | ||
| } | ||
| // Every value an op reads occupies a local, whether or not the trace | ||
| // also contains an op that produces it: constant folding and the short | ||
| // preamble leave a folded value bound only by the constants pool, and | ||
| // `unbound_pool_const_seeds` materializes it in the prologue. Counting | ||
| // only op results would under-size `num_vars` for such a value and, | ||
| // through `next_value_pos`, let `remove_ref_constants` reuse its id for | ||
| // a `LoadFromGcTable` — whose store then lands after the read, so the | ||
| // read returns the zero wasm initializes the local to. | ||
| let widen = |a: OpRef, max_var: &mut u32| { | ||
| if a != OpRef::NONE && !a.is_constant() && a.raw() + 1 > *max_var { | ||
| *max_var = a.raw() + 1; | ||
| } | ||
| }; | ||
| for a in op.getarglist().iter() { | ||
| widen(a.to_opref(), &mut max_var); | ||
| } | ||
|
Comment on lines
+1321
to
+1323
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When this loop now admits non-constant operand ids that have no producer, only AGENTS.md reference: AGENTS.md:L14-L20 Useful? React with 👍 / 👎. |
||
| if let Some(fa) = op.getfailargs() { | ||
| for a in fa.iter() { | ||
| widen(a.to_opref(), &mut max_var); | ||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,9 @@ | ||
| bridges_compiled=8 | ||
| bridges_compiled=18 | ||
| descr_set_absent=0 | ||
| descr_set_ambiguous=0 | ||
| descr_set_stale_absent=0 | ||
| fbw_rolled_back_with_effects=0 | ||
| guard_failures=1605 | ||
| guard_failures=3610 | ||
| internal_compile_panics=0 | ||
| loops_aborted=0 | ||
| loops_compiled=6 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,9 @@ | ||
| bridges_compiled=8 | ||
| bridges_compiled=18 | ||
| descr_set_absent=0 | ||
| descr_set_ambiguous=0 | ||
| descr_set_stale_absent=0 | ||
| fbw_rolled_back_with_effects=0 | ||
| guard_failures=1605 | ||
| guard_failures=3610 | ||
| internal_compile_panics=0 | ||
| loops_aborted=0 | ||
| loops_compiled=6 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,9 @@ | ||
| bridges_compiled=8 | ||
| bridges_compiled=18 | ||
| descr_set_absent=0 | ||
| descr_set_ambiguous=0 | ||
| descr_set_stale_absent=0 | ||
| fbw_rolled_back_with_effects=0 | ||
| guard_failures=1605 | ||
| guard_failures=3610 | ||
| internal_compile_panics=0 | ||
| loops_aborted=0 | ||
| loops_compiled=6 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,9 @@ | ||
| bridges_compiled=7 | ||
| bridges_compiled=9 | ||
| descr_set_absent=0 | ||
| descr_set_ambiguous=0 | ||
| descr_set_stale_absent=0 | ||
| fbw_rolled_back_with_effects=0 | ||
| guard_failures=1403 | ||
| guard_failures=1804 | ||
| internal_compile_panics=0 | ||
| loops_aborted=0 | ||
| loops_compiled=3 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,9 @@ | ||
| bridges_compiled=7 | ||
| bridges_compiled=9 | ||
| descr_set_absent=0 | ||
| descr_set_ambiguous=0 | ||
| descr_set_stale_absent=0 | ||
| fbw_rolled_back_with_effects=0 | ||
| guard_failures=1403 | ||
| guard_failures=1804 | ||
| internal_compile_panics=0 | ||
| loops_aborted=0 | ||
| loops_compiled=3 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,9 @@ | ||
| bridges_compiled=7 | ||
| bridges_compiled=9 | ||
| descr_set_absent=0 | ||
| descr_set_ambiguous=0 | ||
| descr_set_stale_absent=0 | ||
| fbw_rolled_back_with_effects=0 | ||
| guard_failures=1404 | ||
| guard_failures=1804 | ||
| internal_compile_panics=0 | ||
| loops_aborted=0 | ||
| loops_compiled=3 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| bridges_compiled=2 | ||
| bridges_compiled=3 | ||
| descr_set_absent=0 | ||
| descr_set_ambiguous=0 | ||
| descr_set_stale_absent=0 | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| bridges_compiled=2 | ||
| bridges_compiled=3 | ||
| descr_set_absent=0 | ||
| descr_set_ambiguous=0 | ||
| descr_set_stale_absent=0 | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,9 @@ | ||
| bridges_compiled=2 | ||
| bridges_compiled=3 | ||
| descr_set_absent=0 | ||
| descr_set_ambiguous=0 | ||
| descr_set_stale_absent=0 | ||
| fbw_rolled_back_with_effects=0 | ||
| guard_failures=401 | ||
| guard_failures=601 | ||
| internal_compile_panics=0 | ||
| loops_aborted=0 | ||
| loops_compiled=4 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| bridges_compiled=5 | ||
| descr_set_absent=0 | ||
| descr_set_ambiguous=0 | ||
| descr_set_stale_absent=0 | ||
| fbw_rolled_back_with_effects=0 | ||
| guard_failures=1016 | ||
| internal_compile_panics=0 | ||
| loops_aborted=0 | ||
| loops_compiled=15 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,9 @@ | ||
| bridges_compiled=2 | ||
| bridges_compiled=4 | ||
| descr_set_absent=0 | ||
| descr_set_ambiguous=0 | ||
| descr_set_stale_absent=0 | ||
| fbw_rolled_back_with_effects=0 | ||
| guard_failures=406 | ||
| guard_failures=606 | ||
| internal_compile_panics=0 | ||
| loops_aborted=0 | ||
| loops_compiled=6 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,9 @@ | ||
| bridges_compiled=2 | ||
| bridges_compiled=4 | ||
| descr_set_absent=0 | ||
| descr_set_ambiguous=0 | ||
| descr_set_stale_absent=0 | ||
| fbw_rolled_back_with_effects=0 | ||
| guard_failures=406 | ||
| guard_failures=606 | ||
| internal_compile_panics=0 | ||
| loops_aborted=0 | ||
| loops_compiled=6 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,9 @@ | ||
| bridges_compiled=3 | ||
| bridges_compiled=5 | ||
| descr_set_absent=0 | ||
| descr_set_ambiguous=0 | ||
| descr_set_stale_absent=0 | ||
| fbw_rolled_back_with_effects=0 | ||
| guard_failures=608 | ||
| guard_failures=1008 | ||
| internal_compile_panics=0 | ||
| loops_aborted=0 | ||
| loops_compiled=6 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,9 @@ | ||
| bridges_compiled=11 | ||
| bridges_compiled=12 | ||
| descr_set_absent=0 | ||
| descr_set_ambiguous=0 | ||
| descr_set_stale_absent=0 | ||
| fbw_rolled_back_with_effects=0 | ||
| guard_failures=2261 | ||
| guard_failures=2461 | ||
| internal_compile_panics=0 | ||
| loops_aborted=0 | ||
| loops_compiled=3 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,9 @@ | ||
| bridges_compiled=11 | ||
| bridges_compiled=12 | ||
| descr_set_absent=0 | ||
| descr_set_ambiguous=0 | ||
| descr_set_stale_absent=0 | ||
| fbw_rolled_back_with_effects=0 | ||
| guard_failures=2261 | ||
| guard_failures=2461 | ||
| internal_compile_panics=0 | ||
| loops_aborted=0 | ||
| loops_compiled=3 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,9 @@ | ||
| bridges_compiled=11 | ||
| bridges_compiled=12 | ||
| descr_set_absent=0 | ||
| descr_set_ambiguous=0 | ||
| descr_set_stale_absent=0 | ||
| fbw_rolled_back_with_effects=0 | ||
| guard_failures=2261 | ||
| guard_failures=2461 | ||
| internal_compile_panics=0 | ||
| loops_aborted=0 | ||
| loops_compiled=3 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,9 @@ | ||
| bridges_compiled=2 | ||
| bridges_compiled=4 | ||
| descr_set_absent=0 | ||
| descr_set_ambiguous=0 | ||
| descr_set_stale_absent=0 | ||
| fbw_rolled_back_with_effects=0 | ||
| guard_failures=404 | ||
| guard_failures=607 | ||
| internal_compile_panics=0 | ||
| loops_aborted=0 | ||
| loops_compiled=5 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,9 @@ | ||
| bridges_compiled=2 | ||
| bridges_compiled=4 | ||
| descr_set_absent=0 | ||
| descr_set_ambiguous=0 | ||
| descr_set_stale_absent=0 | ||
| fbw_rolled_back_with_effects=0 | ||
| guard_failures=404 | ||
| guard_failures=607 | ||
| internal_compile_panics=0 | ||
| loops_aborted=0 | ||
| loops_compiled=5 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,9 @@ | ||
| bridges_compiled=2 | ||
| bridges_compiled=4 | ||
| descr_set_absent=0 | ||
| descr_set_ambiguous=0 | ||
| descr_set_stale_absent=0 | ||
| fbw_rolled_back_with_effects=0 | ||
| guard_failures=404 | ||
| guard_failures=804 | ||
| internal_compile_panics=0 | ||
| loops_aborted=0 | ||
| loops_compiled=5 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,9 @@ | ||
| bridges_compiled=7 | ||
| bridges_compiled=8 | ||
| descr_set_absent=0 | ||
| descr_set_ambiguous=0 | ||
| descr_set_stale_absent=0 | ||
| fbw_rolled_back_with_effects=0 | ||
| guard_failures=1654 | ||
| guard_failures=1670 | ||
| internal_compile_panics=0 | ||
| loops_aborted=0 | ||
| loops_compiled=4 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,9 @@ | ||
| bridges_compiled=7 | ||
| bridges_compiled=8 | ||
| descr_set_absent=0 | ||
| descr_set_ambiguous=0 | ||
| descr_set_stale_absent=0 | ||
| fbw_rolled_back_with_effects=0 | ||
| guard_failures=1654 | ||
| guard_failures=1670 | ||
| internal_compile_panics=0 | ||
| loops_aborted=0 | ||
| loops_compiled=4 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,9 @@ | ||
| bridges_compiled=8 | ||
| bridges_compiled=9 | ||
| descr_set_absent=0 | ||
| descr_set_ambiguous=0 | ||
| descr_set_stale_absent=0 | ||
| fbw_rolled_back_with_effects=0 | ||
| guard_failures=1836 | ||
| guard_failures=2037 | ||
| internal_compile_panics=0 | ||
| loops_aborted=0 | ||
| loops_compiled=4 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,9 @@ | ||
| bridges_compiled=2 | ||
| bridges_compiled=6 | ||
| descr_set_absent=0 | ||
| descr_set_ambiguous=0 | ||
| descr_set_stale_absent=0 | ||
| fbw_rolled_back_with_effects=0 | ||
| guard_failures=401 | ||
| guard_failures=1202 | ||
| internal_compile_panics=0 | ||
| loops_aborted=0 | ||
| loops_compiled=2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the folded value this loop now admits is a Float that survives only as an operand/constant-pool seed,
num_varsis widened for its raw id butcollect_value_typesstill marks only input args and op results asF64, leaving this new local asI64. A laterFloat*op resolves the same OpRef viaemit_resolve_f64and emitslocal.getof an i64 local where wasm expects f64, so folded-float short-preamble traces are rejected/invalid instead of compiling correctly; the operand census needs to propagate the value type (or decline) along with increasingnum_vars.Useful? React with 👍 / 👎.