perf(codegen): unary + proves a Number by construction — const v = +o.a goes 20 to 9 instructions (#10777) - #10781
proggeramlug wants to merge 1 commit into
Conversation
…nd condition (PerryTS#10777) `expr_numeric_by_construction` required `rec(operand)` for `Pos`, the same as for `Neg` and `BitNot`. That was not a soundness guard, it was a missed proof. Unary `+` is ToNumber, which either completes holding a Number or throws: a BigInt and a Symbol both throw a TypeError, an object goes through ToPrimitive and then ToNumber again, `undefined` is NaN, and NaN is a Number. A throw stores no value, so the store-universe question this fixpoint asks is vacuous on that path — there is no input for which `+x` finishes holding something other than a Number. `Neg` and `BitNot` keep their operand condition, because ToNumeric is BigInt-preserving: `-1n` is `-1n` and `~1n` is `-2n`, neither a Number. The missed proof left the ACCUMULATOR unproven, so its add kept a per-iteration tag test: const v = +o.a; for (…) h += v 20 -> 9 Ir/iteration const v = +a[0]; for (…) h += v 20 -> 9 (Float64Array) which is exactly where `o.a * 1` and `o.a - 0` already sat. node is 7.03 and 7.50 on the same fixtures, bun 4.27 and 4.48, so this closes the perry-versus-perry gap and does not reach parity. Claude-Session: https://claude.ai/code/session_01YaNfLEjMRdhCLtk3SB5MdJ
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe numeric-expression analysis now treats unary ChangesNumeric-by-construction analysis
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~5 minutes Change: Refactor 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🧪 Generate unit tests (beta)
🛠️ Fix failing CI checks 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Landed via merge train 235 (#10788) as v0.5.1614 — Your commits are on The fix is verified closed against the train's own pinned artifacts, not just against your measurements. Same four-pattern probe I ran on
Gap sweep weighted at the blast radius — every read of a double out of binary data: Both behavioural notes are in the train body rather than only here, since the body is what becomes the release note: NaN payload bits are no longer preserved (101 of 144 cases differ in bits only — matches JSC and SpiderMonkey, differs from node), and the two regressing rows are named explicitly with the Your rejected-alternatives table is preserved in the body verbatim in substance, particularly that canonicalising only in-band NaNs is unsound because a signalling NaN quiets into the band and One thing still open, not blocking: |
2 participants
Part of #10777.
expr_numeric_by_constructionrequiredrec(operand)forPos, the same condition it applies toNegandBitNot. That was not a soundness guard — it was a missed proof.Unary
+is ToNumber, which either completes holding a Number or throws. A BigInt and a Symbol both throw aTypeError; an object goes through ToPrimitive and then ToNumber again, so avalueOfreturning a string yields a Number and one returning a BigInt throws;undefinedisNaN, andNaNis a Number. A throw stores no value, so the store-universe question this fixpoint asks is vacuous on that path. There is no input for which+xfinishes holding something other than a Number, soPosneeds no operand condition at all.NegandBitNotkeep theirs, because ToNumeric is BigInt-preserving:-1nis-1nand~1nis-2n, neither a Number.The missed proof left the accumulator unproven, so its add kept a per-iteration tag test:
const v = +o.a; h += vconst v = +a[0]; h += v(Float64Array)const v = -o.aconst v = o.a * 1(control)Flat at both fit ranges, float accumulator, output identical to node. It closes the perry-versus-perry gap exactly onto the
*1/-0control. It does not reach parity — perry at 9 still trails node's ~7.One behavioural line; the rest is the comment recording why
Posdiffers from its neighbours.Half of my own patch was cut
I had also relaxed
Neg | BitNotto the "either side not BigInt" shape the neighbouringSub | Mul | Div | Modarm uses. Building a third distinct binary with that guard removed entirely produced zero behavioural change —-5nstill printedbigint|-5. So the guard is unwitness-able through that route, and the relaxation also measured zero (up_negandup_bitnotat 29 on every arm).Shipping an unwitnessed, unmeasured relaxation alongside a measured one is the part that should be cut, so
NegandBitNotare byte-identical to upstream.Pos => truehas no guard to witness because the specification gives it none.Real programs: an absence, not a zero
No fixture in the suites exercises this. 0 of 52
realsuitefiles, 0 of 97rungs, 0 of 14residcontain a unary-plus binding. The one real-code site isclisuite/csv.ts:6(amt = +parts[2]), and there the readings were +2,043 / +8,275 / +13,964 / −8,372 / +10,974 instructions on a 51.1 M baseline — noise in both directions. An earlier single run read −6,146; I am not quoting it as a result.Two greens discarded rather than reported
The CLI wall-clock harness first returned
0.00s / 0.00s OKon all four programs —/usr/bin/time -f %ehas 0.01 s resolution and these run in 3–9 ms. Re-measured withperf_counter_ns, min of 60 interleaved: +1.14 / −2.03 / −0.31 / −1.09 %, mixed signs, noise.Gates
cargo fmt --checkclean,check_file_size.shOK,local_binding_type_audit.pyOK with no new allowlist entry,gc_runtime_root_holders.pyran and passed.perry-codegen1648 passed / 0 failed;perry-runtime4097 passed / 0 failed;perry-transform157 passed.perry-hirhas one failure,eval_classifier::tests::remedy_is_scoped_to_bundled_npm_shims, verified failing identically on the untouched base tree — pre-existing, and this change is inperry-codegen. Node identity:clisuite4/4,realsuite51/1 (the knownnestdiff, #10733), both arms unchanged.https://claude.ai/code/session_01YaNfLEjMRdhCLtk3SB5MdJ
Summary by CodeRabbit
Performance
+numeric expressions, including property and array access patterns.Documentation
+, while clarifying the distinct handling of unary-and bitwise complement.