Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions changelog.d/6840-typed-feedback-entry-return-ordering.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
### Fixed

- **`typed_feedback_trace_dump_runs_before_entry_return` has been red on `main`.**
Codegen was never wrong: `add_pre_return_void_call` splices
`js_typed_feedback_maybe_dump_trace()` in front of every `ret` in `main`, and
it still does. The test was measuring the wrong thing. It compared
`ir.rfind("call void @js_typed_feedback_maybe_dump_trace()")` against
`ir.rfind("ret i32 0")` over the whole module text. `main` has more than one
return: the host-return early exit returns a literal `i32 0`, the event-loop
exit returns the pending exit code. So the two `rfind`s landed on unrelated
sites — the last dump sat after the first return, and the assertion failed
while the property it names held.

The test now slices the IR to `main`'s body and checks every return site: each
`ret` must be immediately preceded by the dump call. Removing the
`add_pre_return_void_call` in `crates/perry-codegen/src/codegen/entry.rs`
makes it fail, which the old shape could not guarantee.

The PR workflow now uses diff-based selection in `e2e-scoped`, so changing
this integration test pulls its suite into the per-PR run.
4 changes: 1 addition & 3 deletions crates/perry-codegen/src/loop_purity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,7 @@ fn expr_alloc_free(e: &Expr) -> bool {
// Element READS never allocate — they return an existing element / a
// number. Recurse so the object and index are themselves alloc-free.
Expr::IndexGet { object, index } => expr_alloc_free(object) && expr_alloc_free(index),
Expr::BufferIndexGet { buffer, index } => {
expr_alloc_free(buffer) && expr_alloc_free(index)
}
Expr::BufferIndexGet { buffer, index } => expr_alloc_free(buffer) && expr_alloc_free(index),
Expr::Uint8ArrayGet { array, index } => expr_alloc_free(array) && expr_alloc_free(index),
// `arr[i]++` / `--`: read-modify-write of an existing numeric slot, no
// growth, no allocation.
Expand Down
57 changes: 37 additions & 20 deletions crates/perry-codegen/tests/typed_feedback.rs
Original file line number Diff line number Diff line change
Expand Up @@ -218,27 +218,44 @@ fn typed_feedback_trace_dump_runs_before_entry_return() {
));

assert!(ir.contains("declare void @js_typed_feedback_maybe_dump_trace()"));
// The entry epilogue now has TWO exit paths: the host-return path
// (`ret i32 0`) and the event-loop exit path, which returns the dynamic
// `js_process_pending_exit_code` result (`ret i32 %rN`). The old
// rfind(dump) < rfind("ret i32 0") comparison broke the day the second
// path appeared — its dump call sits after the literal `ret i32 0`. The
// real invariant is stronger: EVERY entry return must be immediately
// preceded by the trace dump, so no exit path can skip the dump.
let mut ret_count = 0;
let mut search_from = 0;
while let Some(rel) = ir[search_from..].find("ret i32") {
let ret_pos = search_from + rel;
ret_count += 1;
let preceding = &ir[ret_pos.saturating_sub(200)..ret_pos];
assert!(
preceding.contains("call void @js_typed_feedback_maybe_dump_trace()"),
"entry return at byte {ret_pos} is not preceded by the typed-feedback \
trace dump:\n...{preceding}"
);
search_from = ret_pos + 1;

// Scope the ordering check to `main`'s body, and check every return site.
// `main` has more than one `ret` (the host-return early exit returns
// `i32 0`, the event-loop exit returns the pending exit code), and later
// functions in the module carry their own returns. A positional
// `rfind(dump) < rfind("ret i32 0")` over the whole module text therefore
// compares two unrelated sites and proves nothing about the ordering.
let body = entry_fn_body(&ir);
let mut returns = 0;
let mut prev = "";
for line in body.lines() {
let trimmed = line.trim();
if trimmed.starts_with("ret ") {
returns += 1;
assert_eq!(
prev, "call void @js_typed_feedback_maybe_dump_trace()",
"every `main` return must dump the typed-feedback trace first, \
else a PERRY_TYPED_FEEDBACK trace is truncated at exit; \
found `{trimmed}` preceded by `{prev}`"
);
}
prev = trimmed;
}
assert!(ret_count >= 1, "entry should return i32");
assert!(returns > 0, "expected at least one return in `main`");
}

/// Body of the entry `main` function, without its `define`/`}` lines.
fn entry_fn_body(ir: &str) -> &str {
let header = "define i32 @main() {\n";
let start = ir
.find(header)
.expect("entry module should define `i32 @main()`")
+ header.len();
let rest = &ir[start..];
let end = rest
.find("\n}\n")
.expect("`main` should be terminated by a closing brace");
&rest[..end]
}

#[test]
Expand Down
Loading