Skip to content

Commit 67cbb86

Browse files
cristianocclaude
andcommitted
Route mk_builtin through the folding constructors
mk_builtin's primitive case built its node directly, so a builtin was the one place a term entered Lambda without passing the constructor that normalizes it. It now goes through prim. That moves folding from the optimizer passes to construction, and the integer switcher plans over the set of distinct actions it is handed, so what it sees changes. The two cases added here are what that cost and bought. In the first, `10 + 10` merges with two `20` arms and four branches with both results duplicated collapse to two with neither. In the second the same merge drops the action count below the density the switcher wants for a jump table, and a switch becomes a chain of comparisons. Both plans are correct in both cases; only the emitted code differs, and the snapshots are here so a future change to when folding happens shows its win and its cost in the same diff. Signed-off-by: Cristiano Calcagno <ccrisccris@gmail.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W8g8qwBARAcvW9MyuKQq8H
1 parent 55d44fd commit 67cbb86

4 files changed

Lines changed: 85 additions & 1 deletion

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@
6868

6969
#### :house: Internal
7070

71+
- Normalize Lambda terms where they are built: a match guard stays structured data until its fallthrough is known, and `apply` and `mk_builtin` go through the folding constructors. https://github.com/rescript-lang/rescript/pull/8615
7172
- Merge the duplicate Lam intermediate representation into Lambda, removing the conversion layer and obsolete supporting infrastructure. Lambda is now a single private, normalized representation, with generated JavaScript remaining semantically unchanged. https://github.com/rescript-lang/rescript/pull/8608
7273
- Add genType and source map controls and output to the developer playground. https://github.com/rescript-lang/rescript/pull/8448
7374
- Rework the object-type representation end to end: object rows are plain field chains carrying a per-field mutability state (no phantom setter members), object literals are typed directly and property access and assignment are first-class AST and Lambda nodes shared between the Lambda and JS pipelines, and dead class-system remnants (the field-presence lattice, the class-abbreviation memo on object types, method-send typing) are removed. https://github.com/rescript-lang/rescript/pull/8597

compiler/ml/lambda.ml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1264,7 +1264,7 @@ let sequand l r = if_ l r lambda_false
12641264

12651265
let mk_builtin b args loc =
12661266
match b with
1267-
| Primitive p -> Lprim {primitive = p; args; loc}
1267+
| Primitive p -> prim ~primitive:p ~args loc
12681268
| Constant c -> (
12691269
match args with
12701270
| [] -> Lconst c
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
// Generated by ReScript, PLEASE EDIT WITH CARE
2+
3+
4+
function improves_when_merged(value) {
5+
if (value > 3 || value < 1) {
6+
return 99;
7+
} else {
8+
return 20;
9+
}
10+
}
11+
12+
function effect(s) {
13+
console.log(s);
14+
}
15+
16+
function regresses_when_merged(x) {
17+
if (x > 4 || x < 1) {
18+
if (x !== 5) {
19+
console.log("s");
20+
} else {
21+
console.log("r");
22+
}
23+
} else if (x >= 4) {
24+
console.log("q");
25+
} else {
26+
console.log("d");
27+
}
28+
}
29+
30+
export {
31+
improves_when_merged,
32+
effect,
33+
regresses_when_merged,
34+
}
35+
/* No side effect */
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
// The integer switcher plans over the set of *distinct* actions it is handed:
2+
// given enough of them across a dense range it emits a jump table, otherwise
3+
// it tests intervals. That count depends on how far the term has been
4+
// normalized when it arrives, because normalization merges arms that were
5+
// written apart.
6+
//
7+
// Folding now happens at construction, so the switcher sees merged arms.
8+
// These two cases are what that cost, and bought, when mk_builtin started
9+
// folding: across the runtime, Belt and every other test module the output
10+
// was unchanged, and only these two moved. Both plans are correct in each
11+
// case; only the emitted code differs.
12+
//
13+
// They stay here because the same sensitivity applies to any future change in
14+
// where normalization happens, and to the switcher's own thresholds - see the
15+
// `dense` predicate in switch.ml, where `switch_min` is what refuses the jump
16+
// table below.
17+
18+
// Improvement: `10 + 10` merges with the two `20` arms, so three actions
19+
// become one, and four branches with `20` and `99` each duplicated collapse
20+
// to two branches with neither duplicated.
21+
let improves_when_merged = value =>
22+
switch value {
23+
| 1 => 10 + 10
24+
| 2 => 20
25+
| 3 => 20
26+
| _ => 99
27+
}
28+
29+
// Regression: the same merge drops the test count from three to two, below
30+
// `switch_min`, so `dense` refuses the jump table and this becomes a chain of
31+
// comparisons. The density check itself still passes; it is the minimum-tests
32+
// floor that rejects it.
33+
let effect = s => Console.log(s)
34+
35+
let regresses_when_merged = x =>
36+
switch x {
37+
| 1 => effect("d")
38+
| 2 => effect("d")
39+
| 3 =>
40+
if 3 > 2 {
41+
effect("d")
42+
} else {
43+
effect("z")
44+
}
45+
| 4 => effect("q")
46+
| 5 => effect("r")
47+
| _ => effect("s")
48+
}

0 commit comments

Comments
 (0)