-
Notifications
You must be signed in to change notification settings - Fork 19
jit(wasm): panic on missing pool-indexed const at emit time instead of emitting 0 #691
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
845a353
400aa9d
8b133eb
3cd286e
5c01ae7
f3ae6d8
4f40988
958e8b9
ae979ee
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 |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| # A module-scope hot loop calling a helper that returns an inlined list | ||
| # comprehension over `range(n)` with `n` a parameter. Once the loop reaches the | ||
| # trace threshold it records the `len(f(<const>))` body; recording the CALL to | ||
| # `f` aborts and the walk forward-flushes the caller (module) frame at the CALL | ||
| # boundary so the interpreter re-runs the call from there. That flush rebuilds | ||
| # the caller's operand stack from the walk's live/shadow sources — but the | ||
| # CALL's `LOAD_CONST`'d argument has no concrete Ref shadow, so its slot | ||
| # resolves to NULL. The flush must decline (fall back to the legacy replay) | ||
| # rather than commit the NULL; committing it left the next call's argument slot | ||
| # unbound, so `f` raised `UnboundLocalError` on its parameter (`n`). | ||
| # | ||
| # The trigger is specific: the caller loop must be at MODULE scope (its CALL | ||
| # operands come from LOAD_NAME / LOAD_CONST, not LOAD_FAST), the inner | ||
| # `range(n)` must be large enough to compile + bridge the comprehension loop, | ||
| # and the outer loop must run enough to reach the trace threshold so the two | ||
| # transitions coincide. | ||
|
|
||
|
|
||
| def f(n): | ||
| return [i for i in range(n)] | ||
|
|
||
|
|
||
| t = 0 | ||
| k = 0 | ||
| while k < 220: | ||
| t += len(f(300)) | ||
| k += 1 | ||
| print(t) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| # pyre-check: max-pypy-ratio=30 | ||
|
|
||
| # Overflow-crossing int multiply on a JIT-hot path. The inner loop is traced | ||
| # while `scale` is small (a*a stays in machine-int range, so the recorded | ||
| # GUARD_NO_OVERFLOW passes), then a large `scale` makes a*a overflow a 64-bit | ||
| # int and it must promote to a big int. A backend that drops the overflow check | ||
| # silently wraps the product instead of promoting, giving a wrong answer. | ||
| def hot(scale, n): | ||
| acc = 0 | ||
| i = 0 | ||
| while i < n: | ||
| a = scale + (i & 1) # loop-variant: cannot fold to a constant | ||
| acc = acc + a * a | ||
| i = i + 1 | ||
| return acc | ||
|
|
||
|
|
||
| def main(): | ||
| warm = 0 | ||
| for _ in range(120): | ||
| warm = warm + hot(3, 20000) # a in {3,4}; a*a tiny, never overflows | ||
| # Big scale: a ~ 5e9, a*a = 2.5e19 overflows int64 (and uint64) -> big int. | ||
| print(hot(5000000000, 20000)) | ||
| print(warm) | ||
|
|
||
|
|
||
| main() |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4249,6 +4249,17 @@ thread_local! { | |
| static FBW_FORITER_INFLIGHT: std::cell::RefCell<Vec<InflightForiter>> = | ||
| const { std::cell::RefCell::new(Vec::new()) }; | ||
|
|
||
| /// Undo log for a bridge/retrace recording walk's eager range-iterator | ||
| /// cursor advance. The main walk leaves the advance unjournaled and relies | ||
| /// on in-flight FOR_ITER forward-delivery to recover the consumed item on | ||
| /// abort; the bridge/retrace abort path has no such delivery, so a bridge | ||
| /// walk records `(iter, pre_current, pre_remaining)` here and restores the | ||
| /// cursor when it does NOT commit — leaving the recording side-effect | ||
| /// neutral so the interpreter resume re-consumes the item exactly once. | ||
| /// Only populated while `is_bridge_trace`; empty (no-op) on the main walk. | ||
| static FBW_BRIDGE_ITER_JOURNAL: std::cell::RefCell<Vec<(pyre_object::PyObjectRef, i64, i64)>> = | ||
| const { std::cell::RefCell::new(Vec::new()) }; | ||
|
Comment on lines
+4260
to
+4261
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.
This new TLS stores raw AGENTS.md reference: AGENTS.md:L153-L155 Useful? React with 👍 / 👎. |
||
|
|
||
| static FBW_UNJOURNALED_VALUE_UNAVAILABLE: std::cell::Cell<bool> = | ||
| const { std::cell::Cell::new(false) }; | ||
| static FBW_UNJOURNALED_SYMBOLIC: std::cell::Cell<bool> = | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5022,6 +5022,12 @@ pub(crate) fn try_walker_specialize_for_iter_next<Sym: WalkSym>( | |
| ctx.trace_ctx | ||
| .set_opref_concrete(current, Value::Int(concrete_current)); | ||
|
|
||
| if ctx.trace_ctx.is_bridge_trace { | ||
| // A bridge/retrace recording walk has no in-flight forward-delivery on | ||
| // abort, so journal the pre-advance cursor for restore if the walk does | ||
| // not commit (keeps the aborted recording side-effect neutral). | ||
| fbw_bridge_iter_journal_push(iter_obj, concrete_current, concrete_remaining); | ||
|
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 specialization runs inside Useful? React with 👍 / 👎. |
||
| } | ||
| fbw_foriter_inflight_capture(concrete_item_ptr, body); | ||
| // Range iteration stays at the C level, so the operand-stack mirror | ||
| // remains valid and must receive the item produced by FOR_ITER. Its | ||
|
|
||
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.
On wasm32 this emits a 4-byte load because
size_of::<usize>()is 4, but the runtime layouts read by this guard arePyType::subclassrange_min/ClassTypeLayout::subclassrange_min, both 8-bytei64fields. If a range value ever needs the upper 32 bits,GuardSubclasstruncates/sign-extends the object's min and can accept or reject the guard incorrectly; use the actual field width for both this vtable path and the gcremovetypeptr path below.AGENTS.md reference: AGENTS.md:L15-L18
Useful? React with 👍 / 👎.