diff --git a/changelog.d/codegen-unsigned-bound.md b/changelog.d/codegen-unsigned-bound.md new file mode 100644 index 0000000000..39187e2188 --- /dev/null +++ b/changelog.d/codegen-unsigned-bound.md @@ -0,0 +1 @@ +Performance: an element read or store whose key is not proven non-negative no longer tests the key's sign in front of its bound. The element tiers already bound every element arm with one unsigned compare (`idx , + arr_box: &str, + idx_i32: &str, + block_prefix: &str, + require_numeric_layout: bool, + coerce_numeric_fallback: bool, + receiver_slot: Option<&str>, + negative_exit: Option<&str>, +) -> Result { + lower_guarded_array_index_get_with_index( + ctx, + arr_box, + GuardedArrayIndex::Signed(idx_i32), + block_prefix, + require_numeric_layout, + coerce_numeric_fallback, + receiver_slot, + negative_exit, + ) +} + +/// N10 (CENSUS-residual cause i): the element tier a split key enters has +/// an unsigned bound in front of every element arm, so the split's sign +/// test can move to the tier's miss arm. The guarded tier bounds every hit +/// (`idx ) -> bool { + crate::expr::index_key::unsigned_bound() + && match proven { + Some(fact) => super::proven_array::bounds_key(fact), + None => !typed_feedback_emission_enabled(), + } +} + +/// The start of a tier's miss arm under N10: a negative key leaves for +/// `negative_exit` (the split's runtime-key arm, which reads the original +/// key), so only a non-negative key reaches the cold read that follows, +/// exactly as when the split tested the sign first. Check mode +/// (`PERRY_VERIFY_FACTS=1`) tests that claim on the cold read's key; the +/// negative control `PERRY_FACTS_SABOTAGE=unsignedbound` drops the exit. +pub(super) fn emit_negative_exit(ctx: &mut FnCtx<'_>, idx_i32: &str, negative_exit: Option<&str>) { + let Some(exit) = negative_exit else { + return; + }; + crate::verify_facts::consumed( + crate::verify_facts::Fact::UnsignedBound, + || serde_json::json!({ "bound": "unsigned" }), + ); + if !super::field_base::sabotaged("unsignedbound") { + let cold = ctx.new_block("ubound.cold"); + let cold_l = ctx.block_label(cold); + let negative = ctx.block().icmp_slt(I32, idx_i32, "0"); + ctx.block().cond_br(&negative, exit, &cold_l); + ctx.current_block = cold; + } + if !crate::verify_facts::enabled() { + return; + } + crate::verify_facts::covered( + crate::verify_facts::Fact::UnsignedBound, + || serde_json::json!({ "bound": "unsigned" }), + ); + let miss = ctx.new_block("ubound.verify_miss"); + let next = ctx.new_block("ubound.admitted"); + let (miss_l, next_l) = (ctx.block_label(miss), ctx.block_label(next)); + crate::verify_facts::tick(ctx); + let nonnegative = ctx.block().icmp_sge(I32, idx_i32, "0"); + ctx.block().cond_br(&nonnegative, &next_l, &miss_l); + ctx.current_block = miss; + let site = crate::verify_facts::site_arg( + ctx, + "a negative key at an element tier's cold read", + crate::verify_facts::Fact::UnsignedBound, + &[], + ); + let key = ctx.block().sitofp(I32, idx_i32, DOUBLE); + let _ = crate::verify_facts::check( + ctx, + I32, + "perry_wpfacts_check_miss", + &[(DOUBLE, &key)], + &site, + ); + ctx.block().unreachable(); + ctx.current_block = next; +} + /// The same indexed read with a compiler proof of a nonnegative u32 index. /// Retain its original JS number on the cold path, including indices > i32::MAX. pub(super) fn lower_unsigned_array_index_get( @@ -376,6 +471,7 @@ pub(super) fn lower_unsigned_array_index_get( false, false, receiver_slot, + None, ) } @@ -384,6 +480,7 @@ enum GuardedArrayIndex<'a> { Unsigned { i32_value: &'a str, number: &'a str }, } +#[allow(clippy::too_many_arguments)] fn lower_guarded_array_index_get_with_index( ctx: &mut FnCtx<'_>, arr_box: &str, @@ -392,6 +489,7 @@ fn lower_guarded_array_index_get_with_index( require_numeric_layout: bool, coerce_numeric_fallback: bool, receiver_slot: Option<&str>, + negative_exit: Option<&str>, ) -> Result { let canonical = matches!(index, GuardedArrayIndex::Signed(_)); let (idx_i32, unsigned_index) = match index { @@ -577,6 +675,7 @@ fn lower_guarded_array_index_get_with_index( ]; ctx.current_block = fallback_idx; let cold_mode = ctx.block().phi(I64, &cold_inputs); + emit_negative_exit(ctx, idx_i32, negative_exit); crate::expr::store_census::bump(ctx, crate::expr::store_census::ELEM_READ_FALLBACK); let idx_box = unsigned_index .map(str::to_owned) diff --git a/crates/perry-codegen/src/expr/index_get/proven_array.rs b/crates/perry-codegen/src/expr/index_get/proven_array.rs index dbcfa5f96b..a171db81f1 100644 --- a/crates/perry-codegen/src/expr/index_get/proven_array.rs +++ b/crates/perry-codegen/src/expr/index_get/proven_array.rs @@ -222,7 +222,7 @@ pub(crate) fn lower_in_floor( number: rep.elem != crate::codegen::FactsElemRep::Boxed, receiver: Receiver::Field, }; - lower_rep(ctx, arr_box, idx_i32, read, false) + lower_rep(ctx, arr_box, idx_i32, read, false, None) } /// `e` is an element read the tier loads below a dense floor and whose @@ -287,9 +287,35 @@ pub(super) fn lower( fact: Proven, numeric: bool, ) -> String { + lower_bounded(ctx, arr_box, idx_i32, fact, numeric, None) +} + +/// The read under `fact` has an unsigned bound in front of its load: the +/// key is not proven below the floor (`idx bool { + match fact { + Proven::Rep(read) => !read.in_floor, + Proven::Fixed(fact) => !fact.key_in_bounds, + } +} + +/// [`lower`] whose key is not sign-tested when `negative_exit` is set +/// (`bounds_key` holds): the miss arm sends a negative key there first +/// (`guarded_array::emit_negative_exit`). +pub(super) fn lower_bounded( + ctx: &mut FnCtx<'_>, + arr_box: &str, + idx_i32: &str, + fact: Proven, + numeric: bool, + negative_exit: Option<&str>, +) -> String { + debug_assert!(negative_exit.is_none() || bounds_key(fact)); match fact { - Proven::Rep(read) => lower_rep(ctx, arr_box, idx_i32, read, numeric), - Proven::Fixed(fact) => lower_fixed(ctx, arr_box, idx_i32, fact, numeric), + Proven::Rep(read) => lower_rep(ctx, arr_box, idx_i32, read, numeric, negative_exit), + Proven::Fixed(fact) => lower_fixed(ctx, arr_box, idx_i32, fact, numeric, negative_exit), } } @@ -300,6 +326,7 @@ fn lower_rep( idx_i32: &str, read: RepRead, numeric: bool, + negative_exit: Option<&str>, ) -> String { let r = read.rep; crate::expr::store_census::bump(ctx, crate::expr::store_census::ELEM_READ_OTHER_TIER); @@ -374,6 +401,7 @@ fn lower_rep( ctx.block().br(&merge_label); ctx.current_block = cold_idx; + super::guarded_array::emit_negative_exit(ctx, idx_i32, negative_exit); let idx_box = ctx.block().sitofp(I32, idx_i32, DOUBLE); // Mode 2: the exact arguments, no absence proof (`emit_read_cold`). let cold = super::guarded_array::emit_read_cold( @@ -394,6 +422,7 @@ fn lower_fixed( idx_i32: &str, fact: FactsElementRead, numeric: bool, + negative_exit: Option<&str>, ) -> String { crate::expr::store_census::bump(ctx, crate::expr::store_census::ELEM_READ_OTHER_TIER); crate::verify_facts::consumed( @@ -438,6 +467,7 @@ fn lower_fixed( blk.cond_br(&hit, &merge_label, &cold_label); ctx.current_block = cold_idx; + super::guarded_array::emit_negative_exit(ctx, idx_i32, negative_exit); let idx_box = ctx.block().sitofp(I32, idx_i32, DOUBLE); // Mode 2: the exact arguments, no absence proof (`emit_read_cold`). let cold = super::guarded_array::emit_read_cold( diff --git a/crates/perry-codegen/src/expr/index_get/runtime_key.rs b/crates/perry-codegen/src/expr/index_get/runtime_key.rs index 542b673eff..20b2301ee3 100644 --- a/crates/perry-codegen/src/expr/index_get/runtime_key.rs +++ b/crates/perry-codegen/src/expr/index_get/runtime_key.rs @@ -18,7 +18,7 @@ use anyhow::Result; use crate::types::{DOUBLE, I32}; -use super::{lower_guarded_array_index_get, FnCtx}; +use super::FnCtx; pub(super) fn lower_array_index_get_via_runtime_key( ctx: &mut FnCtx<'_>, @@ -70,19 +70,39 @@ pub(super) fn lower_array_index_get_via_canonical_i32_split( let runtime_label = ctx.block_label(runtime_idx); let merge_label = ctx.block_label(merge_idx); - let idx_i32 = crate::expr::index_key::normalize(ctx, idx_double, Some(index)); - let is_canonical_i32 = ctx.block().icmp_sge(I32, &idx_i32, "0"); - ctx.block() - .cond_br(&is_canonical_i32, &element_label, &runtime_label); + // N10: when every element arm of the tier is behind `idx { - super::proven_array::lower(ctx, arr_box, &idx_i32, fact, coerce_numeric_fallback) - } - None => lower_guarded_array_index_get( + Some(fact) => super::proven_array::lower_bounded( + ctx, + arr_box, + &idx_i32, + fact, + coerce_numeric_fallback, + negative_exit.as_deref(), + ), + None => super::guarded_array::lower_guarded_array_index_get_bounded( ctx, arr_box, &idx_i32, @@ -90,6 +110,7 @@ pub(super) fn lower_array_index_get_via_canonical_i32_split( require_numeric_layout, coerce_numeric_fallback, receiver_slot, + negative_exit.as_deref(), )?, }; let element_end = ctx.block().label.clone(); diff --git a/crates/perry-codegen/src/expr/index_get_claim_tests.rs b/crates/perry-codegen/src/expr/index_get_claim_tests.rs index 8c2ea0b148..3ac7d84be4 100644 --- a/crates/perry-codegen/src/expr/index_get_claim_tests.rs +++ b/crates/perry-codegen/src/expr/index_get_claim_tests.rs @@ -830,8 +830,16 @@ fn any_typed_dynamic_key_takes_the_numeric_tiers_when_it_is_an_array_index() { assert!(ir.contains("aidx.canonical") && ir.contains("aidx.runtime_key")); // The "integer-valued double in [0, 2^32)" test is the shared pure // normalizer (`memory(none)`, a GC leaf) and a sign test of its result. + // N10: the tier's unsigned bound declines the normalizer's `-1`, and the + // sign test sits on the tier's miss arm (`icmp slt`, `ubound.cold`); with + // `PERRY_UNSIGNED_BOUND=0` it is the split's `icmp sge` again. + let sign_test = if crate::expr::index_key::unsigned_bound() { + ir.contains("icmp slt i32 %") && ir.contains("ubound.cold") + } else { + ir.contains("icmp sge i32 %") + }; assert!( - ir.contains("call i32 @perry_array_index_normalize_i32(") && ir.contains("icmp sge i32 %"), + ir.contains("call i32 @perry_array_index_normalize_i32(") && sign_test, "the dynamic key must be normalized by the shared index helper:\n{ir}" ); assert!(ir.contains("aidx.dynamic.guard.deref") && ir.contains("aidx.dynamic.fast")); diff --git a/crates/perry-codegen/src/expr/index_key.rs b/crates/perry-codegen/src/expr/index_key.rs index 4576b05013..5b28324ce8 100644 --- a/crates/perry-codegen/src/expr/index_key.rs +++ b/crates/perry-codegen/src/expr/index_key.rs @@ -65,6 +65,35 @@ pub(super) fn is_char_code(ctx: &FnCtx<'_>, expr: &Expr) -> bool { /// another block. The cast's input dominates exactly the same uses as its /// output; signed negatives and unsigned values above i32::MAX decline. pub(super) fn normalize(ctx: &mut FnCtx<'_>, key: &str, expr: Option<&Expr>) -> String { + normalize_keyed(ctx, key, expr, false) +} + +/// [`normalize`] for a consumer whose every element arm is behind an +/// unsigned bound (`idx , key: &str, expr: Option<&Expr>) -> String { + normalize_keyed(ctx, key, expr, unsigned_bound()) +} + +/// `PERRY_UNSIGNED_BOUND=0`: every element read and store keeps its sign +/// test in front of the bound (N10 off). +pub(crate) fn unsigned_bound() -> bool { + static ON: std::sync::OnceLock = std::sync::OnceLock::new(); + *ON.get_or_init(|| { + !matches!( + std::env::var("PERRY_UNSIGNED_BOUND").as_deref(), + Ok("0") | Ok("off") | Ok("false") + ) + }) +} + +fn normalize_keyed(ctx: &mut FnCtx<'_>, key: &str, expr: Option<&Expr>, wrap: bool) -> String { let source = ctx .func .blocks() @@ -72,6 +101,9 @@ pub(super) fn normalize(ctx: &mut FnCtx<'_>, key: &str, expr: Option<&Expr>) -> .rev() .find_map(|b| b.widened_i32_source(key)); if let Some(index) = source { + if wrap { + return index; + } let valid = ctx.block().icmp_sge(I32, &index, "0"); return ctx.block().select(I1, &valid, I32, &index, "-1"); } @@ -516,4 +548,82 @@ mod tests { ); } } + + /// `key | 0`: a native i32 widened to the key's double. + fn int_key() -> Expr { + Expr::Binary { + op: BinaryOp::BitOr, + left: Box::new(Expr::NumberCoerce(Box::new(Expr::LocalGet(2)))), + right: Box::new(Expr::Integer(0)), + } + } + + /// N10: a widened native key enters the bounded tier as it is. No + /// select maps it to -1 and the split does not branch on its sign; the + /// tier's miss arm sends a negative key to the runtime-key arm. + #[test] + fn widened_key_read_has_its_sign_test_on_the_miss_arm_only() { + let ir = read_ir(int_key(), vec![]); + assert!(!ir.contains(", i32 -1"), "the -1 select is gone: {ir}"); + let entry = ir + .split_once("define double @") + .and_then(|(_, f)| f.split("\naidx.canonical.").next()) + .unwrap_or_else(|| panic!("no split: {ir}")); + assert!(!entry.contains("icmp sge i32"), "{entry}"); + // The miss arm's exit: a negative key to the runtime-key arm, the + // rest to the tier's cold read. + let exit = ir + .lines() + .find(|l| l.contains("label %aidx.runtime_key.") && l.contains("label %ubound.cold.")) + .unwrap_or_else(|| panic!("no negative exit: {ir}")); + let cond = exit + .split_whitespace() + .nth(2) + .map(|c| c.trim_end_matches(',')) + .unwrap_or_else(|| panic!("{exit}")); + assert!( + ir.contains(&format!("{cond} = icmp slt i32")), + "the exit tests the sign: {ir}" + ); + } + + /// N10, negative controls: a key the normalizer admits still declines + /// NaN, fractions and tags as -1 (now behind the unsigned bound), and + /// the runtime-key arm keeps the original double. + #[test] + fn number_key_keeps_its_normalizer_and_the_exact_runtime_arm() { + let ir = read_ir(Expr::LocalGet(2), vec![]); + assert!( + ir.contains("call i32 @perry_array_index_normalize_i32"), + "{ir}" + ); + assert!(ir.contains("ubound.cold"), "{ir}"); + let runtime_block = ir + .split_once("\naidx.runtime_key.") + .map(|(_, rest)| rest.split("\n\n").next().unwrap_or(rest)) + .unwrap_or_else(|| panic!("no runtime-key block: {ir}")); + // Mode 0: the original key, the exact property-key read. + assert!( + runtime_block.contains("call double @perry_array_read_cold_shared(") + && runtime_block.contains(", i64 0)"), + "{runtime_block}" + ); + } + + /// N10 on the store side: the guarded store's one bound is + /// `idx , idx_double: &str) -> String { - super::index_key::normalize(ctx, idx_double, None) + super::index_key::normalize_unsigned(ctx, idx_double, None) } /// The raw-f64 element-kind bits in `_reserved`: `GC_ARRAY_RAW_F64_LAYOUT` diff --git a/crates/perry-codegen/src/verify_facts/coverage.rs b/crates/perry-codegen/src/verify_facts/coverage.rs index 8a28d85931..a490262ef1 100644 --- a/crates/perry-codegen/src/verify_facts/coverage.rs +++ b/crates/perry-codegen/src/verify_facts/coverage.rs @@ -44,6 +44,7 @@ pub(crate) enum Fact { Signature, ArrayRep, ConstArray, + UnsignedBound, } impl Fact { pub(super) fn name(self) -> &'static str { @@ -87,6 +88,7 @@ impl Fact { Self::Signature => "signature", Self::ArrayRep => "array rep", Self::ConstArray => "constant array", + Self::UnsignedBound => "unsigned bound", } } } diff --git a/crates/perry/src/commands/compile/compile_env.rs b/crates/perry/src/commands/compile/compile_env.rs index 110874c04c..18c6cb2df5 100644 --- a/crates/perry/src/commands/compile/compile_env.rs +++ b/crates/perry/src/commands/compile/compile_env.rs @@ -244,6 +244,7 @@ pub(super) const COMPILE_ENV_VARS: &[&str] = &[ "PERRY_TYPED_LOCALS", "PERRY_U8_INLINE_READ", "PERRY_UNBOXED_CLASS_FIELDS", + "PERRY_UNSIGNED_BOUND", "PERRY_VERIFY_FACTS", "PERRY_VERIFY_NATIVE_REGIONS", "PERRY_VISIONOS_SYSROOT", diff --git a/crates/perry/src/commands/compile/compile_env/tests.rs b/crates/perry/src/commands/compile/compile_env/tests.rs index ad1fe7d6d8..e4928fb0b1 100644 --- a/crates/perry/src/commands/compile/compile_env/tests.rs +++ b/crates/perry/src/commands/compile/compile_env/tests.rs @@ -363,6 +363,17 @@ fn range_tightening_switches_are_cache_inputs() { } } +/// N10: the unsigned-bound switch changes the element reads' and stores' +/// key admission (the sign test in front of the bound), so `=0` must never +/// reuse an `=1` object. +#[test] +fn unsigned_bound_switch_is_a_cache_input() { + assert!( + COMPILE_ENV_VARS.contains(&"PERRY_UNSIGNED_BOUND"), + "PERRY_UNSIGNED_BOUND must key the compile caches" + ); +} + /// N4: the cold-path switch changes helper declarations, call-site /// attributes and branch weights, so `=0` must never reuse an `=1` object. #[test] diff --git a/scripts/test-verify-facts-s0.py b/scripts/test-verify-facts-s0.py index 6b9966c5a0..84652dcfce 100644 --- a/scripts/test-verify-facts-s0.py +++ b/scripts/test-verify-facts-s0.py @@ -126,6 +126,7 @@ def oracle(source, *argv): ("array-base-negative", "arrbase", "array rep"), ("array-base-admit-negative", "arrbaseadm", "array rep"), ("fn-base-admit-negative", "fnbaseadm", "array rep"), + ("unsigned-bound-negative", "unsignedbound", "unsigned bound"), ("number-call-negative", "numbercall", "numeric result"), ("field-read-call-negative", "fieldreadcall", "field read call"), ("array-undef-negative", "arrundef", "element floor"), diff --git a/scripts/verify_facts_s0/unsigned-bound-negative.ts b/scripts/verify_facts_s0/unsigned-bound-negative.ts new file mode 100644 index 0000000000..e2a07672d8 --- /dev/null +++ b/scripts/verify_facts_s0/unsigned-bound-negative.ts @@ -0,0 +1,23 @@ +// N10: `at` reads `this.board[k | 0]` with no sign test in front of the +// bound. `idx 100 ? 5 : 3; + at(sq: number): number { + return this.board[sq | 0] ?? 0; + } + probe(): number { + const ep = this.ep; + return this.at(ep) * 100 + this.at(ep - 16) * 10 + this.at(ep + 16); + } +} +const b = new Board(); +let s = 0; +for (let i = 0; i < 3; i++) s += b.probe(); +const plain: number[] = [4, 5, 6]; +const k = process.argv.length > 100 ? 1 : -2; +console.log(s, plain[k | 0] ?? 7); diff --git a/tests/parity/language/array/test_unsigned_bound.ts b/tests/parity/language/array/test_unsigned_bound.ts new file mode 100644 index 0000000000..4ff86ffefb --- /dev/null +++ b/tests/parity/language/array/test_unsigned_bound.ts @@ -0,0 +1,106 @@ +// N10 (unsigned bound): an element read or store whose key is not tested for +// sign before the bound. `idx