From 8928eaa3173e8efc85110bb79e3b30aec02a4af0 Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Wed, 21 Oct 2026 11:25:25 +0000 Subject: [PATCH 1/2] Use constant arguments directly when inlining a function in JavaScript Why: When the JavaScript pass inlines a function called once, it replaced a parameter by a variable argument but bound any other argument with `let`, so a constant argument stayed a variable nothing folds away. The pass now also substitutes numbers, booleans, `undefined` and `null`, the constants the Lambda inliner substitutes too, under the same condition as variables: the parameter is never assigned by a tail call. Strings and anything that allocates stay bound once. JS modified (inlined partial application): Example: let g1 = (x, y) => { let u = x + y (xx, yy) => xx + yy + u } let v = g1(3, 4)(6, ...) Before (js): function v(extra) { let xx = 6; return (xx + extra | 0) + 7 | 0; } After (js): function v(extra) { return (6 + extra | 0) + 7 | 0; } Review: Can a substituted parameter be assigned? No: a parameter a tail call assigns is unmasked in the function's immutable mask and stays bound with `let`, as before for variable arguments (`count` in the new test). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Kr4cjoUi2f5VpMvE6wT9bo Signed-off-by: Christoph Knittel --- changes/inline-constant-arguments.polish.md | 1 + compiler/core/js_pass_tailcall_inline.ml | 53 ++++++++++++++----- tests/tests/src/inline_constant_args_test.mjs | 46 ++++++++++++++++ tests/tests/src/inline_constant_args_test.res | 50 +++++++++++++++++ tests/tests/src/loop_regression_test.mjs | 3 +- tests/tests/src/test_demo.mjs | 3 +- 6 files changed, 138 insertions(+), 18 deletions(-) create mode 100644 changes/inline-constant-arguments.polish.md create mode 100644 tests/tests/src/inline_constant_args_test.mjs create mode 100644 tests/tests/src/inline_constant_args_test.res diff --git a/changes/inline-constant-arguments.polish.md b/changes/inline-constant-arguments.polish.md new file mode 100644 index 00000000000..d8890b62f49 --- /dev/null +++ b/changes/inline-constant-arguments.polish.md @@ -0,0 +1 @@ +- Use constant arguments directly when inlining a function in generated JavaScript, instead of binding them to variables first. diff --git a/compiler/core/js_pass_tailcall_inline.ml b/compiler/core/js_pass_tailcall_inline.ml index 16dd68fbf0f..df0f2a5c072 100644 --- a/compiler/core/js_pass_tailcall_inline.ml +++ b/compiler/core/js_pass_tailcall_inline.ml @@ -37,16 +37,36 @@ module S = Js_stmt_make let super = Js_record_map.super -let substitue_variables (map : Ident.t Map_ident.t) = +(* [vars] maps a parameter to the variable argument that replaces it, [consts] + to the constant argument that replaces it. Both apply only to parameters + that are never assigned, so every occurrence of such a parameter is a read. *) +let substitute_arguments (vars : Ident.t Map_ident.t) + (consts : J.expression Map_ident.t) = { super with ident = (fun _ id -> - match Map_ident.find_opt id map with + match Map_ident.find_opt id vars with | Some id -> id | None -> id); + expression = + (fun self e -> + match e.expression_desc with + | Var (Id id) -> ( + match Map_ident.find_opt id consts with + | Some c -> c + | None -> super.expression self e) + | _ -> super.expression self e); } +(* Constants that are as cheap to repeat as the variable they replace: they + allocate nothing, have no side effect, and print short. The Lambda inliner + substitutes the same kinds of constants for let-bound variables. *) +let is_substitutable_constant (e : J.expression) = + match e.expression_desc with + | Number _ | Bool _ | Undefined _ | Null -> true + | _ -> false + (* 1. recursive value ? let rec x = 1 :: x non-terminating 2. duplicative identifiers .. @@ -66,23 +86,28 @@ let substitue_variables (map : Ident.t Map_ident.t) = let inline_call (immutable_list : bool list) params (args : J.expression list) processed_blocks = - let map, block = + let substitute param (arg : J.expression) (vars, consts, acc) = + match arg.expression_desc with + | Var (Id id) -> (Map_ident.add param id vars, consts, acc) + | _ when is_substitutable_constant arg -> + (vars, Map_ident.add param arg consts, acc) + | _ -> (vars, consts, S.define_variable ~kind:Variable param arg :: acc) + in + let vars, consts, block = if immutable_list = [] then - Ext_list.fold_right2 params args (Map_ident.empty, processed_blocks) - (fun param arg (map, acc) -> - match arg.expression_desc with - | Var (Id id) -> (Map_ident.add param id map, acc) - | _ -> (map, S.define_variable ~kind:Variable param arg :: acc)) + Ext_list.fold_right2 params args + (Map_ident.empty, Map_ident.empty, processed_blocks) + substitute else Ext_list.fold_right3 params args immutable_list - (Map_ident.empty, processed_blocks) (fun param arg mask (map, acc) -> - match (mask, arg.expression_desc) with - | true, Var (Id id) -> (Map_ident.add param id map, acc) - | _ -> (map, S.define_variable ~kind:Variable param arg :: acc)) + (Map_ident.empty, Map_ident.empty, processed_blocks) + (fun param arg mask ((vars, consts, acc) as state) -> + if mask then substitute param arg state + else (vars, consts, S.define_variable ~kind:Variable param arg :: acc)) in - if Map_ident.is_empty map then block + if Map_ident.is_empty vars && Map_ident.is_empty consts then block else - let obj = substitue_variables map in + let obj = substitute_arguments vars consts in obj.block obj block (** There is a side effect when traversing dead code, since diff --git a/tests/tests/src/inline_constant_args_test.mjs b/tests/tests/src/inline_constant_args_test.mjs new file mode 100644 index 00000000000..ea9c7d209c1 --- /dev/null +++ b/tests/tests/src/inline_constant_args_test.mjs @@ -0,0 +1,46 @@ +// Generated by ReScript, PLEASE EDIT WITH CARE + +import * as Mocha from "mocha"; +import * as Test_utils from "./test_utils.mjs"; + +function scaled(extra) { + let label = "value: "; + let a = 1 * 2.5 + -1; + let b = extra * 2.5 - -1; + let c = true ? b - a : a - b; + return label + (c + a * b).toString(); +} + +function intsApplied(extra) { + let s = (2 + (-3 * extra | 0) | 0) - 7 | 0; + let t = (s * -3 | 0) + (7 * 2 | 0) | 0; + return 10n + BigInt((s * t | 0) + -3 | 0); +} + +function count() { + let _n = 10; + let _acc = 0; + while (true) { + let acc = _acc; + let n = _n; + if (n <= 0) { + return acc; + } + _acc = acc + n | 0; + _n = n - 1 | 0; + continue; + }; +} + +Mocha.describe("Inline_constant_args_test", () => { + Mocha.test("float, bool and string constants", () => Test_utils.eq("File \"inline_constant_args_test.res\", line 42, characters 7-14", "value: 13.5", scaled(2))); + Mocha.test("int and bigint constants", () => Test_utils.eq("File \"inline_constant_args_test.res\", line 45, characters 7-14", -777n, intsApplied(3))); + Mocha.test("reassigned parameter", () => Test_utils.eq("File \"inline_constant_args_test.res\", line 48, characters 7-14", 55, count())); +}); + +export { + scaled, + intsApplied, + count, +} +/* Not a pure module */ diff --git a/tests/tests/src/inline_constant_args_test.res b/tests/tests/src/inline_constant_args_test.res new file mode 100644 index 00000000000..af29f2c6a01 --- /dev/null +++ b/tests/tests/src/inline_constant_args_test.res @@ -0,0 +1,50 @@ +open Mocha +open Test_utils + +// The partial applications inline the local functions in JS: constant +// arguments replace the parameters, except the string, which stays bound. +let scaled = { + let scale = (k, offset, flip, label, x, y) => { + let a = x *. k +. offset + let b = y *. k -. offset + let c = if flip { + b -. a + } else { + a -. b + } + label ++ Float.toString(c +. a *. b) + } + scale(2.5, -1., true, "value: ", 1., ...) +} + +let intsApplied = { + let ints = (a, b, big, x, y) => { + let s = x + a * y - b + let t = s * a + b * x + big + BigInt.fromInt(s * t + a) + } + ints(-3, 7, 10n, 2, ...) +} + +// [n] is reassigned by the loop, so the constant must stay bound. +let count = () => { + let rec loop = (n, acc) => + if n > 0 { + loop(n - 1, acc + n) + } else { + acc + } + loop(10, 0) +} + +describe(__MODULE__, () => { + test("float, bool and string constants", () => { + eq(__LOC__, "value: 13.5", scaled(2.)) + }) + test("int and bigint constants", () => { + eq(__LOC__, -777n, intsApplied(3)) + }) + test("reassigned parameter", () => { + eq(__LOC__, 55, count()) + }) +}) diff --git a/tests/tests/src/loop_regression_test.mjs b/tests/tests/src/loop_regression_test.mjs index 1018a521889..f79849f5974 100644 --- a/tests/tests/src/loop_regression_test.mjs +++ b/tests/tests/src/loop_regression_test.mjs @@ -6,9 +6,8 @@ import * as Test_utils from "./test_utils.mjs"; function f() { let v = 0; let acc = 0; - let n = 10; while (true) { - if (v > n) { + if (v > 10) { return acc; } acc = acc + v | 0; diff --git a/tests/tests/src/test_demo.mjs b/tests/tests/src/test_demo.mjs index 6d11614500a..6b82fc3bd55 100644 --- a/tests/tests/src/test_demo.mjs +++ b/tests/tests/src/test_demo.mjs @@ -55,8 +55,7 @@ function g1(x, y) { let x = 14; function v(extra) { - let xx = 6; - return (xx + extra | 0) + 7 | 0; + return (6 + extra | 0) + 7 | 0; } let nil = "Nil"; From d4c9fc8e774d4fe5b52117f03cca9581ffdd4f2f Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Wed, 21 Oct 2026 11:33:46 +0000 Subject: [PATCH 2/2] Keep only the taken branch of a condition on an inlined constant Why: Substituting a boolean argument left conditions such as `true ? b - a : a - b` and `if (false)` in the inlined body. When a parameter replaced by a boolean or integer constant is the condition of a `?:` or an `if`, or is negated, the substitution now keeps only the branch taken, or yields the boolean, as `Js_stmt_make.if_` does for constant conditions during code generation. Other conditions and arithmetic are not folded. JS modified (condition on a constant argument): Example: let logged = { let log = (verbose, x) => { let out = [] if verbose { out->Array.push(Int.toString(x)) } if !verbose { out->Array.push("done") } out->Array.push(verbose ? "verbose" : "brief") out } log(false, ...) } Before (js): function logged(extra) { let out = []; if (false) { out.push(extra.toString()); } if (!false) { out.push("done"); } out.push(false ? "verbose" : "brief"); return out; } After (js): function logged(extra) { let out = []; out.push("done"); out.push("brief"); return out; } Review: Can a declaration in the kept branch clash once the branch is flattened into the enclosing block? No more than for the constant conditions `Js_stmt_make.if_` already flattens: names are allocated per function, so a declaration of the same name elsewhere is renamed. Review: Why not rebuild through `Js_exp_make.econd`? It treats any number condition as true, which is wrong for `0.0`, `NaN` and `0n`, so only boolean and integer constants decide a condition here. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Kr4cjoUi2f5VpMvE6wT9bo Signed-off-by: Christoph Knittel --- compiler/core/js_pass_tailcall_inline.ml | 37 ++++++++++++++++++- tests/tests/src/inline_constant_args_test.mjs | 23 ++++++++++-- tests/tests/src/inline_constant_args_test.res | 23 ++++++++++++ 3 files changed, 77 insertions(+), 6 deletions(-) diff --git a/compiler/core/js_pass_tailcall_inline.ml b/compiler/core/js_pass_tailcall_inline.ml index df0f2a5c072..ca54be32c3a 100644 --- a/compiler/core/js_pass_tailcall_inline.ml +++ b/compiler/core/js_pass_tailcall_inline.ml @@ -33,13 +33,29 @@ *) module S = Js_stmt_make -(* module E = Js_exp_make *) +module E = Js_exp_make let super = Js_record_map.super +(* The truth value of a condition that is a parameter replaced by a boolean or + integer constant, or its negation. Only such conditions are decided while + substituting: the substitution is what made them constant. *) +let rec substituted_truth (consts : J.expression Map_ident.t) (e : J.expression) + = + match e.expression_desc with + | Var (Id id) -> ( + match Map_ident.find_opt id consts with + | Some {expression_desc = Bool b} -> Some b + | Some {expression_desc = Number (Int {i})} -> Some (i <> 0l) + | _ -> None) + | Js_not e -> Option.map not (substituted_truth consts e) + | _ -> None + (* [vars] maps a parameter to the variable argument that replaces it, [consts] to the constant argument that replaces it. Both apply only to parameters - that are never assigned, so every occurrence of such a parameter is a read. *) + that are never assigned, so every occurrence of such a parameter is a read. + A conditional whose condition becomes a constant keeps only the branch it + takes, and the negation of such a condition becomes a boolean. *) let substitute_arguments (vars : Ident.t Map_ident.t) (consts : J.expression Map_ident.t) = { @@ -56,7 +72,24 @@ let substitute_arguments (vars : Ident.t Map_ident.t) match Map_ident.find_opt id consts with | Some c -> c | None -> super.expression self e) + | Cond (pred, ifso, ifnot) -> ( + match substituted_truth consts pred with + | Some truth -> self.expression self (if truth then ifso else ifnot) + | None -> super.expression self e) + | Js_not pred -> ( + match substituted_truth consts pred with + | Some truth -> E.bool (not truth) + | None -> super.expression self e) | _ -> super.expression self e); + statement = + (fun self st -> + match st.statement_desc with + | If (pred, ifso, ifnot) -> ( + match substituted_truth consts pred with + | Some truth -> + S.block (self.block self (if truth then ifso else ifnot)) + | None -> super.statement self st) + | _ -> super.statement self st); } (* Constants that are as cheap to repeat as the variable they replace: they diff --git a/tests/tests/src/inline_constant_args_test.mjs b/tests/tests/src/inline_constant_args_test.mjs index ea9c7d209c1..b67263f53d7 100644 --- a/tests/tests/src/inline_constant_args_test.mjs +++ b/tests/tests/src/inline_constant_args_test.mjs @@ -7,7 +7,7 @@ function scaled(extra) { let label = "value: "; let a = 1 * 2.5 + -1; let b = extra * 2.5 - -1; - let c = true ? b - a : a - b; + let c = b - a; return label + (c + a * b).toString(); } @@ -17,6 +17,15 @@ function intsApplied(extra) { return 10n + BigInt((s * t | 0) + -3 | 0); } +function logged(extra) { + let out = []; + let line = "quiet " + extra.toString(); + out.push(line); + out.push("done"); + out.push("brief"); + return out; +} + function count() { let _n = 10; let _acc = 0; @@ -33,14 +42,20 @@ function count() { } Mocha.describe("Inline_constant_args_test", () => { - Mocha.test("float, bool and string constants", () => Test_utils.eq("File \"inline_constant_args_test.res\", line 42, characters 7-14", "value: 13.5", scaled(2))); - Mocha.test("int and bigint constants", () => Test_utils.eq("File \"inline_constant_args_test.res\", line 45, characters 7-14", -777n, intsApplied(3))); - Mocha.test("reassigned parameter", () => Test_utils.eq("File \"inline_constant_args_test.res\", line 48, characters 7-14", 55, count())); + Mocha.test("float, bool and string constants", () => Test_utils.eq("File \"inline_constant_args_test.res\", line 62, characters 7-14", "value: 13.5", scaled(2))); + Mocha.test("int and bigint constants", () => Test_utils.eq("File \"inline_constant_args_test.res\", line 65, characters 7-14", -777n, intsApplied(3))); + Mocha.test("bool constant conditions", () => Test_utils.eq("File \"inline_constant_args_test.res\", line 68, characters 7-14", [ + "quiet 4", + "done", + "brief" + ], logged(4))); + Mocha.test("reassigned parameter", () => Test_utils.eq("File \"inline_constant_args_test.res\", line 71, characters 7-14", 55, count())); }); export { scaled, intsApplied, + logged, count, } /* Not a pure module */ diff --git a/tests/tests/src/inline_constant_args_test.res b/tests/tests/src/inline_constant_args_test.res index af29f2c6a01..cdb7ede6c8b 100644 --- a/tests/tests/src/inline_constant_args_test.res +++ b/tests/tests/src/inline_constant_args_test.res @@ -26,6 +26,26 @@ let intsApplied = { ints(-3, 7, 10n, 2, ...) } +// A condition on a constant argument keeps only the branch it takes. +let logged = { + let log = (verbose, prefix, x) => { + let out = [] + if verbose { + let line = prefix ++ Int.toString(x) + out->Array.push(line) + } else { + let line = "quiet " ++ Int.toString(x) + out->Array.push(line) + } + if !verbose { + out->Array.push("done") + } + out->Array.push(verbose ? "verbose" : "brief") + out + } + log(false, "x = ", ...) +} + // [n] is reassigned by the loop, so the constant must stay bound. let count = () => { let rec loop = (n, acc) => @@ -44,6 +64,9 @@ describe(__MODULE__, () => { test("int and bigint constants", () => { eq(__LOC__, -777n, intsApplied(3)) }) + test("bool constant conditions", () => { + eq(__LOC__, ["quiet 4", "done", "brief"], logged(4)) + }) test("reassigned parameter", () => { eq(__LOC__, 55, count()) })