Skip to content

Preserve -0 in integer multiplication fast paths#2620

Merged
lahma merged 1 commit into
sebastienros:mainfrom
lahma:fix-integer-multiply-negative-zero
Jul 10, 2026
Merged

Preserve -0 in integer multiplication fast paths#2620
lahma merged 1 commit into
sebastienros:mainfrom
lahma:fix-integer-multiply-negative-zero

Conversation

@lahma

@lahma lahma commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Spec bug in the integer-operand fast paths of multiplication: Number::multiply gives a negative-signed zero when a zero product's operand signs differ — Object.is(0 * -5, -0) must be true — but both binary * (TimesBinaryExpression) and compound *= (ComputeCompound) computed integer products in long arithmetic, where the sign of zero cannot be represented, and returned +0:

var zero = 0, negFive = -5;
Object.is(zero * negFive, -0);   // main: false — spec/V8: true
var acc = 0; acc *= -5;
Object.is(acc, -0);              // main: false — spec/V8: true

Literal forms (0 * -5 with both operands literal) were unaffected because constant folding already runs on doubles — which is why Test262's multiplication suites (literal-shaped) never caught it; any runtime integer operands (slots, array elements, properties, call results) hit the bug.

The fix

Compute the long product first; a nonzero product keeps the exact integer path unchanged, a zero product re-derives the result via double multiplication to pick up the sign rule. Zero products are rare in numeric loops, so the extra branch sits on a cold edge.

Audit of the other AreIntegerOperands fast paths: Remainder and DivideInteger already handle their -0 cases explicitly; integer addition/subtraction cannot produce -0 (integer-typed operands can't hold -0, and x + (−x) / x − x are +0 per spec); bitwise ops are ToInt32-domain. Only the two multiplication sites were affected.

Benchmarks

The integer multiply path is hot in numeric workloads — guard A/B (stash-pair, one window):

DromaeoBenchmark Cube (multiply-heavy) at --launchCount 5 (the single-launch run showed a +12.9% swing on one Cube row alongside a −15% swing on another — the classic code-layout signature for these InvocationCount=1 rows, so the decision runs on multi-launch medians):

Row main (median) PR (median) Δ Allocated
Cube classic, source 8.739 ms 8.456 ms −3.2% 1.59 MB (=)
Cube classic, prepared 6.945 ms 6.996 ms +0.7% 1.31 MB (=)
Cube modern, source 7.799 ms 7.966 ms +2.1% 1.59 MB (=)
Cube modern, prepared 7.190 ms 7.104 ms −1.2% 1.3 MB (=)

ObjectArray (integer-arithmetic-heavy, single launch): all four variants within ±3% both directions, allocations byte-identical. Mixed signs inside the documented noise band on every row — the zero-product branch is perf-neutral, as expected for a predicted-not-taken edge.

Tests

New NumberTests.IntegerMultiplicationPreservesNegativeZero pins 11 shapes: slot × slot both orders, positive/zero controls, literal form, array elements, object properties, function returns, and compound *= in three sign combinations.

Full gate: Jint/Jint.csproj all TFMs, Jint.Tests (net10.0 + net472), Jint.Tests.PublicInterface (net10.0 + net472), full Test262.

🤖 Generated with Claude Code

https://claude.ai/code/session_01BY8HoKam5H8vTPn1bMY2Hv

Number::multiply gives a negative-signed zero when a zero product's operand
signs differ (0 * -5 is -0), but the integer-operand fast paths of binary * and
compound *= computed the product in long arithmetic where the sign of zero
cannot be represented, producing +0. Route zero products through double
arithmetic; nonzero products keep the exact integer path. Literal forms were
unaffected (constant folding already runs on doubles) which is why the gap
never showed in Test262.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BY8HoKam5H8vTPn1bMY2Hv
@lahma
lahma merged commit 668cb02 into sebastienros:main Jul 10, 2026
4 checks passed
@lahma
lahma deleted the fix-integer-multiply-negative-zero branch July 10, 2026 08:31
lahma added a commit that referenced this pull request Jul 10, 2026
Full re-run after the 2026-07-10 batch (#2613, #2615-#2620). Highlights vs
the 2026-07-09 refresh: dromaeo-string-base64 allocation -30% (2.30 -> 1.62
MB/op) and -modern -4.3% time, the classic row now ranked first outright
(previously tied with NiL.JS) - loop tests of the form i < s.length read the
length unboxed through the comparison lane (#2617). The batch's engine-reuse
wins (#2613/#2615 definition reuse, #2616 slot-cache lane restoration) target
long-lived-engine embeddings and are not visible in this fresh-engine-per-op
table. Everything else moved within documented noise bands; competitor rows
served as thermal canaries (times within +/-3%, allocations byte-identical).


Claude-Session: https://claude.ai/code/session_01BY8HoKam5H8vTPn1bMY2Hv

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
lahma added a commit that referenced this pull request Jul 13, 2026
* Add unboxed operand lane for arithmetic binary operators

Comparison operators (PR #2578) and bitwise operators read identifier
operands as raw doubles through slot/global caches, but Plus/Minus/Times/
Divide still materialize JsNumber instances via the full identifier read
path for every evaluation, including re-materializing unboxed slot values.

Add NumericOperandLane covering id-op-id, id-op-const and const-op-id leaf
shapes: both operands read via TryGetNumberSlotForRead (declines strings,
objects, BigInt, TDZ - the generic path then owns coercion and errors),
compute on raw doubles, box the result once. A nullable class rather than
an embedded struct so the many non-numeric Plus nodes pay one reference
field instead of two slot caches.

Raw-double multiply preserves -0 for zero products of opposite signs,
matching the boxed integer arm fixed in #2620.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Re-admit multiplication in the numeric assignment fast path

The exclusion predated #2620: the boxed integer-multiply arm used to
produce +0 for 0 * -5, so a raw-double lane would have changed observable
results. #2620 routes zero products through double arithmetic, and an
unboxed slot stores -0 exactly, so x = a * b can now use the same
slot-to-slot lane as + - / %.

Extends IntegerMultiplicationPreservesNegativeZero with the discard-mode
assignment shapes.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Embed the numeric operand lane as a struct in Minus/Times/Divide

The lane object costs an extra dereference per evaluation on hit-heavy
loops (MixedArithmetic +8.7% vs the embedded-struct comparison lanes).
Minus/Times/Divide are almost always numeric and follow the comparison
classes precedent of an embedded lane struct; Plus keeps a heap-allocated
wrapper so string-concat nodes pay one null reference instead of an
embedded slot-cache pair.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant