Skip to content

Commit 16fbe30

Browse files
dsblankclaude
andcommitted
Fix JIT-inlined + diverging from plus() on negative zero
_JitCompiler inlines + as a bare left-associative Python expression (a + b + ...), but plus() -- used by both the classic dispatch and _fast_prim_map for the same call -- folds from an explicit 0 (functools.reduce(operator.add, args, 0)), i.e. 0 + a + b + .... For ordinary numbers 0 + x == x exactly, so this never mattered, except for IEEE-754 negative zero: 0.0 + -0.0 == 0.0 (positive), while -0.0 alone keeps its sign. Confirmed directly, forcing the JIT path via a nested-call warmup: (+ -0.0) gave 0.0 via the classic dispatch but -0.0 via the JIT-inlined version, at both 1 and 2 arguments. Fixed by matching plus()'s exact fold order in the generated source: (0 + a + b + ...) instead of (a + b + ...). -/* don't have this problem (subtraction never folds from an identity value; multiplying by 1 never flips a sign the way adding 0 can). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 32dece6 commit 16fbe30

3 files changed

Lines changed: 124 additions & 0 deletions

File tree

calysto_scheme/scheme.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1552,6 +1552,20 @@ def _app(self, op_exp, arg_list):
15521552
return '1'
15531553
elif n == 1 and sym == '-':
15541554
return f'(-{args[0]})'
1555+
elif sym == '+':
1556+
# plus() folds from an explicit 0
1557+
# (functools.reduce(operator.add, args, 0)), which the
1558+
# classic dispatch and _fast_prim_map both go through
1559+
# for this same call -- for ordinary numbers 0 + x ==
1560+
# x, but IEEE-754 negative zero is the one case where
1561+
# they differ (0.0 + -0.0 == 0.0, positive, while -0.0
1562+
# alone stays negative). Matching the exact fold order
1563+
# here (0 + a + b + ...) keeps JIT-inlined + identical
1564+
# to plus() in that edge case instead of silently
1565+
# flipping a sign bit -- confirmed this diverges
1566+
# without the explicit 0 -- see
1567+
# tests/test_jit_plus_negative_zero.py.
1568+
return '(0 + ' + ' + '.join(args) + ')'
15551569
else:
15561570
return '(' + f' {op} '.join(args) + ')'
15571571
# binary comparisons

calysto_scheme/src/Scheme.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1544,6 +1544,20 @@ def _app(self, op_exp, arg_list):
15441544
return '1'
15451545
elif n == 1 and sym == '-':
15461546
return f'(-{args[0]})'
1547+
elif sym == '+':
1548+
# plus() folds from an explicit 0
1549+
# (functools.reduce(operator.add, args, 0)), which the
1550+
# classic dispatch and _fast_prim_map both go through
1551+
# for this same call -- for ordinary numbers 0 + x ==
1552+
# x, but IEEE-754 negative zero is the one case where
1553+
# they differ (0.0 + -0.0 == 0.0, positive, while -0.0
1554+
# alone stays negative). Matching the exact fold order
1555+
# here (0 + a + b + ...) keeps JIT-inlined + identical
1556+
# to plus() in that edge case instead of silently
1557+
# flipping a sign bit -- confirmed this diverges
1558+
# without the explicit 0 -- see
1559+
# tests/test_jit_plus_negative_zero.py.
1560+
return '(0 + ' + ' + '.join(args) + ')'
15471561
else:
15481562
return '(' + f' {op} '.join(args) + ')'
15491563
# binary comparisons
Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
"""
2+
Regression test for a divergence found while checking whether
3+
_JitCompiler's raw-operator inlining (_NARY/_CMP) actually matches the
4+
numeric semantics of plus()/minus()/multiply()/LessThan()/etc. -- the
5+
same functions the classic dispatch and _fast_prim_map both call for
6+
these names.
7+
8+
_NARY inlines `+` as a bare left-associative Python expression
9+
(`a + b + ...`), but plus() computes
10+
`functools.reduce(operator.add, args, 0)`, i.e. `0 + a + b + ...` -- an
11+
explicit fold starting from 0. For ordinary numbers 0 + x == x exactly,
12+
so this never mattered -- except for IEEE-754 negative zero, where
13+
0.0 + -0.0 == 0.0 (positive), while -0.0 alone keeps its sign. Confirmed
14+
directly before this fix existed, forcing the JIT-compiled path via a
15+
nested-call warmup (a top-level call alone never uses a compiled
16+
function -- see _jit_lookup's callers): (+ -0.0) gave 0.0 (positive) via
17+
the classic dispatch, but -0.0 (negative) via the JIT-inlined version, at
18+
both 1 and 2 arguments.
19+
20+
`-`/`*` don't have this problem: subtraction never folds from an
21+
identity value at the start (minus() begins its reduce from args[0]
22+
itself, not a prepended identity), and multiplying by 1 never flips a
23+
sign the way adding 0 can (1 * -0.0 == -0.0, unlike 0 + -0.0 == 0.0).
24+
25+
Fixed by making the `+` inlining match plus()'s exact fold order:
26+
`(0 + a + b + ...)` instead of `(a + b + ...)`.
27+
"""
28+
import math
29+
30+
import calysto_scheme.scheme as scheme
31+
32+
33+
def _sign(x):
34+
return math.copysign(1, x)
35+
36+
37+
def test_jit_inlined_plus_matches_classic_dispatch_for_negative_zero_one_arg():
38+
classic = scheme.execute_string_rm("(+ -0.0)")
39+
40+
scheme.execute_string_rm("""
41+
(define (add-one-arg x) (+ x))
42+
(define (warmup-add-one x) (add-one-arg x))
43+
(warmup-add-one 1.0)
44+
""")
45+
proc = scheme.binding_value(
46+
scheme.search_env(scheme.toplevel_env, scheme.make_symbol("add-one-arg")))
47+
assert scheme._jit_lookup(proc) is not None, (
48+
"sanity check: add-one-arg should have JIT-compiled on warmup"
49+
)
50+
51+
jit_result = scheme.execute_string_rm("(warmup-add-one -0.0)")
52+
assert _sign(jit_result) == _sign(classic) == 1.0, (
53+
f"classic dispatch gives {classic!r} (sign {_sign(classic)}), JIT "
54+
f"gives {jit_result!r} (sign {_sign(jit_result)}) -- they must "
55+
"agree; the pre-fix bug produced -0.0 (negative) from the JIT "
56+
"path while the classic dispatch correctly gave 0.0 (positive)"
57+
)
58+
59+
60+
def test_jit_inlined_plus_matches_classic_dispatch_for_negative_zero_two_args():
61+
classic = scheme.execute_string_rm("(+ -0.0 -0.0)")
62+
63+
scheme.execute_string_rm("""
64+
(define (add-two-args x y) (+ x y))
65+
(define (warmup-add-two x y) (add-two-args x y))
66+
(warmup-add-two 1.0 1.0)
67+
""")
68+
proc = scheme.binding_value(
69+
scheme.search_env(scheme.toplevel_env, scheme.make_symbol("add-two-args")))
70+
assert scheme._jit_lookup(proc) is not None, (
71+
"sanity check: add-two-args should have JIT-compiled on warmup"
72+
)
73+
74+
jit_result = scheme.execute_string_rm("(warmup-add-two -0.0 -0.0)")
75+
assert _sign(jit_result) == _sign(classic) == 1.0, (
76+
f"classic dispatch gives {classic!r} (sign {_sign(classic)}), JIT "
77+
f"gives {jit_result!r} (sign {_sign(jit_result)}) -- they must agree"
78+
)
79+
80+
81+
def test_jit_inlined_plus_still_computes_ordinary_sums_correctly():
82+
"""Control: the fix (prepending 0 to the fold) must not change the
83+
result for ordinary, non-signed-zero numbers."""
84+
scheme.execute_string_rm("""
85+
(define (add-three x y z) (+ x y z))
86+
(define (warmup-add-three x y z) (add-three x y z))
87+
(warmup-add-three 1 2 3)
88+
""")
89+
proc = scheme.binding_value(
90+
scheme.search_env(scheme.toplevel_env, scheme.make_symbol("add-three")))
91+
assert scheme._jit_lookup(proc) is not None, (
92+
"sanity check: add-three should have JIT-compiled on warmup"
93+
)
94+
95+
result = scheme.execute_string_rm("(warmup-add-three 10 20 30)")
96+
assert result == 60, f"got {result!r}, expected 60 (10+20+30)"

0 commit comments

Comments
 (0)