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
15 changes: 2 additions & 13 deletions .github/workflows/gc-native-roots.yml
Original file line number Diff line number Diff line change
Expand Up @@ -535,26 +535,15 @@ jobs:
# Same answer as the shadow stack, and a collection that actually
# moved something. Without the movement assert this passes with the
# conservative scan doing all the rooting (#7336, #7338).
#
# PERRY_GC_DIAG=1 is what makes the evacuation assert able to see
# anything: `[gc-copy-minor] ran copied_objects=...` is printed only
# under that variable (`gc/copying.rs`). Without it the trace holds
# nothing but the probe's own `#gcmetric` lines, the assert reads
# 0 copying minors / 0 objects copied off an empty file, and the step
# fails no matter how the collector behaved — which is how this arm
# read on its first-ever execution (three of four arms in this matrix
# were permanently queued until #7393). Diagnostics go to stderr only,
# so the control diff below is unaffected.
./target/perry-dev/perry "$probe" -o /tmp/inproc-09-control
PERRY_GC_HEAP_LIMIT=8 PERRY_GC_INCREMENTAL=0 PERRY_CONSERVATIVE_STACK_SCAN=off \
/tmp/inproc-09-control > /tmp/inproc-09.control.out 2>/dev/null
PERRY_GC_DIAG=1 PERRY_GC_FORCE_EVACUATE=1 PERRY_GC_VERIFY_EVACUATION=1 \
PERRY_GC_FORCE_EVACUATE=1 PERRY_GC_VERIFY_EVACUATION=1 \
PERRY_GC_HEAP_LIMIT=8 PERRY_GC_INCREMENTAL=0 PERRY_CONSERVATIVE_STACK_SCAN=off \
/tmp/inproc-09 > /tmp/inproc-09.out 2> /tmp/inproc-09.err
diff /tmp/inproc-09.control.out /tmp/inproc-09.out \
|| { echo "::error::in-process RS4GC diverged from the shadow-stack control"; exit 1; }
python3 scripts/gc_evacuation_liveness_assert.py /tmp/inproc-09.err \
--probe "09_try_catch_roots (in-process)"
python3 scripts/gc_evacuation_liveness_assert.py /tmp/inproc-09.err

# And it must be RS4GC doing the lowering, not a per-function bail to
# the bridge -- which would make this arm green while testing the
Expand Down
12 changes: 0 additions & 12 deletions changelog.d/7414-macos-rs4gc-inprocess-gc-diag.md

This file was deleted.

27 changes: 0 additions & 27 deletions changelog.d/7415-dominance-corpus.md

This file was deleted.

40 changes: 40 additions & 0 deletions changelog.d/7416-i64-integer-locals.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
**Fixed** JS `%` on an integer-valued local lowered to `frem`, which is not an
aarch64 instruction and becomes an `fmod` library call. `bench_bitwise` was
**20.4x slower than Node**; it is now **1.39x** — a 14.6x improvement, 55108ms to
3763ms, with the Node-verified `CHECKSUM:525000000` unchanged.

A guarded `srem` fast path already existed, but every gate in front of it asked
the wrong question. `is_integer_valued_expr` resolves a `LocalGet` through
`integer_locals`, which is an **i32-range** property — it also gates i32 shadow
slots, so widening it would have placed an i32-overflowing value into an i32 slot.
The `%` path converts to **i64** and only needs integer-valued-within-i64.

A new `collect_int_valued_i64_locals` supplies that weaker property as a
magnitude lattice, read only by the `%` gate. `integer_locals` is untouched.

Three holes were found and closed while building it, each of which would have
produced silently wrong arithmetic:

* **`Mul` blowup.** A pure integrality predicate let `(a*b*c) % n` reach
`fptosi ... to i64` with a 93-bit product — poison. The lattice tracks
magnitude and gates at 2^62. This also closes the same pre-existing hole for
i32 locals.
* **Zero divisor.** An early version routed `1000 % d` into `srem` where `d`
decrements through zero: `srem(x, 0)` is UB where JS requires NaN. The divisor
is now restricted to a non-zero integer literal.
* **The saturation argument.** "±constant cannot leave i64 in finite time" is
false for large deltas — `a = a + 1e18` escapes in ~10 iterations. Replaced
with a hard IEEE-754 bound: once `ulp(v) >= 4D`, `v ± d` rounds back to `v`
exactly, so `|L| <= 2^(55+log2 D)` forever. Deltas are capped at 64.

Also of note: the gate that actually decides this is
`expr/mod.rs::lower_numeric_binary_value`, which intercepts numeric binary ops
before `binary::lower` and only handed `Mod` off when the dividend had an i32
counter slot. There are six `frem` emission sites; widening the predicate alone
changed nothing, and the IR acceptance test caught that.

Verified: hot-function IR goes `frem 4 -> 0`, `srem 0 -> 4`; the edge-case
differential (including both `-0` cases, via `Object.is` rather than `===`) is
byte-identical to Node 26.5.1; `cargo test -p perry-codegen --lib` 632 passed;
26/27 math/number/int gap tests pass with the one failure pre-existing and
unrelated.
1 change: 1 addition & 0 deletions crates/perry-codegen/src/codegen/closure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -967,6 +967,7 @@ pub(super) fn compile_closure(
try_depth: 0,
pending_declares: Vec::new(),
integer_locals: native_facts.integer_locals(),
int_valued_i64_locals: native_facts.int_valued_i64_locals(),
not_bigint_locals: native_facts.not_bigint_locals(),
unsigned_i32_locals: native_facts.unsigned_i32_locals(),
// Conservative: treat every slot as possibly-bound (param binds are
Expand Down
2 changes: 2 additions & 0 deletions crates/perry-codegen/src/codegen/entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -776,6 +776,7 @@ pub(super) fn compile_module_entry(
try_depth: 0,
pending_declares: Vec::new(),
integer_locals: main_native_facts.integer_locals(),
int_valued_i64_locals: main_native_facts.int_valued_i64_locals(),
not_bigint_locals: main_native_facts.not_bigint_locals(),
unsigned_i32_locals: main_native_facts.unsigned_i32_locals(),
shadow_slots_bound: main_shadow_slot_map.values().copied().collect(),
Expand Down Expand Up @@ -1439,6 +1440,7 @@ pub(super) fn compile_module_entry(
try_depth: 0,
pending_declares: Vec::new(),
integer_locals: init_native_facts.integer_locals(),
int_valued_i64_locals: init_native_facts.int_valued_i64_locals(),
not_bigint_locals: init_native_facts.not_bigint_locals(),
unsigned_i32_locals: init_native_facts.unsigned_i32_locals(),
shadow_slots_bound: init_shadow_slot_map.values().copied().collect(),
Expand Down
1 change: 1 addition & 0 deletions crates/perry-codegen/src/codegen/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -763,6 +763,7 @@ pub(super) fn compile_function(
try_depth: 0,
pending_declares: Vec::new(),
integer_locals: native_facts.integer_locals(),
int_valued_i64_locals: native_facts.int_valued_i64_locals(),
not_bigint_locals: native_facts.not_bigint_locals(),
unsigned_i32_locals: native_facts.unsigned_i32_locals(),
shadow_slot_map,
Expand Down
2 changes: 2 additions & 0 deletions crates/perry-codegen/src/codegen/method.rs
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,7 @@ pub(super) fn compile_method(
try_depth: 0,
pending_declares: Vec::new(),
integer_locals: native_facts.integer_locals(),
int_valued_i64_locals: native_facts.int_valued_i64_locals(),
not_bigint_locals: native_facts.not_bigint_locals(),
unsigned_i32_locals: native_facts.unsigned_i32_locals(),
// Conservative: treat every slot as possibly-bound (param binds are
Expand Down Expand Up @@ -1555,6 +1556,7 @@ pub(super) fn compile_static_method(
try_depth: 0,
pending_declares: Vec::new(),
integer_locals: native_facts.integer_locals(),
int_valued_i64_locals: native_facts.int_valued_i64_locals(),
not_bigint_locals: native_facts.not_bigint_locals(),
unsigned_i32_locals: native_facts.unsigned_i32_locals(),
// Conservative: treat every slot as possibly-bound (param binds are
Expand Down
18 changes: 18 additions & 0 deletions crates/perry-codegen/src/collectors/hir_facts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,15 @@ pub(crate) type NativeRegionFactGraph = TypeFacts;
#[derive(Debug, Clone, Default)]
pub(crate) struct RepresentationFacts {
pub integer_locals: HashSet<u32>,
/// Locals that are integer-valued within **i64** range but NOT provably
/// within i32 range, mapped to a conservative `log2(|value|)` bound.
///
/// Deliberately separate from `integer_locals`, which is an i32-RANGE set
/// feeding i32 shadow slots (`needs_i32_slot`) — widening that would place
/// an i32-overflowing value into an i32 slot. The `%` fast path converts to
/// i64 and only needs i64-range integrality, so it consults this set too.
/// See `collectors/int_valued_i64_locals.rs`. This is the ONLY consumer.
pub int_valued_i64_locals: std::collections::HashMap<u32, u32>,
pub unsigned_i32_locals: HashSet<u32>,
/// Locals whose runtime value provably can never be a BigInt (every write
/// is a non-BigInt expression). Seeds `is_provably_not_bigint`, which gates
Expand Down Expand Up @@ -164,6 +173,10 @@ impl TypeFacts {
&self.representation.integer_locals
}

pub(crate) fn int_valued_i64_locals(&self) -> &std::collections::HashMap<u32, u32> {
&self.representation.int_valued_i64_locals
}

pub(crate) fn unsigned_i32_locals(&self) -> &HashSet<u32> {
&self.representation.unsigned_i32_locals
}
Expand Down Expand Up @@ -564,9 +577,14 @@ pub(crate) fn collect_type_facts(
compile_time_constants,
&integer_locals,
);
// i64-range integer-valued locals for the `%` fast path. Independent of
// `integer_locals` (which is i32-RANGE and drives i32 shadow slots); this
// one is consumed only by `type_analysis::numeric::integer_magnitude_bits`.
let int_valued_i64_locals = super::int_valued_i64_locals::collect_int_valued_i64_locals(stmts);
let graph = TypeFacts {
representation: RepresentationFacts {
integer_locals: integer_locals.clone(),
int_valued_i64_locals,
unsigned_i32_locals,
not_bigint_locals,
int_valued_ta_locals,
Expand Down
Loading
Loading