From 298c06a86cac922f797918faeed84794ddf17c04 Mon Sep 17 00:00:00 2001 From: ASDAlexander77 Date: Mon, 14 Sep 2026 14:33:32 +0100 Subject: [PATCH] Implement optimization for direct calls to hybrid functions with statically-null `this` --- ...hybrid-func-null-this-call-optimization.md | 137 ++++++++++++++++++ tslang/lib/TypeScript/TypeScriptOps.cpp | 17 +++ tslang/test/tester/CMakeLists.txt | 3 + .../tester/tests/00funcs_hybrid_null_this.ts | 32 ++++ 4 files changed, 189 insertions(+) create mode 100644 docs/hybrid-func-null-this-call-optimization.md create mode 100644 tslang/test/tester/tests/00funcs_hybrid_null_this.ts diff --git a/docs/hybrid-func-null-this-call-optimization.md b/docs/hybrid-func-null-this-call-optimization.md new file mode 100644 index 000000000..a4a0e9693 --- /dev/null +++ b/docs/hybrid-func-null-this-call-optimization.md @@ -0,0 +1,137 @@ +# Optimization: direct call when a hybrid function has a statically-null `this` + +Status: implemented as option D below (`SimplifyIndirectCallWithKnownCallee`, test `00funcs_hybrid_null_this.ts`). + +## Problem + +Casting a plain function value (e.g. an address from `GetProcAddress`) to a function type and +calling it produces a runtime `this` check whose outcome is known at compile time. + +Repro (`--emit=llvm --opt_level=0`): + +```ts +declare function GetProcAddress(library: Opaque, functionName: Opaque): Opaque; +const f = GetProcAddress(module, "wglCreateContext"); +let hdc: Opaque = 0; +const r = (<(p: Opaque) => Opaque>f)(hdc); +``` + +MLIR: + +```mlir +%162 = "ts.Cast"(%145) : (!ts.opaque) -> !ts.func +%163 = "ts.Cast"(%162) : (!ts.func<...>) -> !ts.hybrid_func +%165 = "ts.CallIndirect"(%163, %164) : (!ts.hybrid_func<...>, !ts.opaque) -> !ts.opaque +``` + +LLVM IR: + +```llvm +%39 = insertvalue { ptr, ptr, ptr } undef, ptr %24, 0 +%40 = insertvalue { ptr, ptr, ptr } %39, ptr null, 1 ; this = null (constant) +%41 = insertvalue { ptr, ptr, ptr } %40, ptr null, 2 +%42 = load ptr, ptr %1 +%43 = extractvalue { ptr, ptr, ptr } %41, 1 +%44 = ptrtoint ptr %43 to i64 +%45 = icmp ne i64 %44, 0 ; always false +br i1 %45, label %46, label %49 +46: %48 = call ptr %47(ptr %43, ptr %42) ; dead +49: %51 = call ptr %50(ptr %42) ; the only reachable call +52: %53 = phi ptr [ %51, %49 ], [ %48, %46 ] +``` + +The result is correct, but every such call gets a dead branch, a phi and an extra call site. +At `--opt` LLVM probably folds it. At `--opt_level=0` / `--di` it stays, which makes debug +builds bigger and stepping through the code confusing. + +## Where it comes from + +1. `MLIRGenCast.cpp` (~L1169, "opaque to hybrid func"): `Opaque -> FunctionType -> HybridFunctionType`. + A function type in a cast expression becomes `HybridFunctionType`. +2. `include/TypeScript/LowerToLLVM/CastLogicHelper.h` (~L273): `FunctionType -> HybridFunctionType` + is lowered to `CreateBoundFunctionOp(NullOp, func)`, so `this` is a literal null. + The same applies to `NullType -> HybridFunctionType` (~L313). +3. `lib/TypeScript/LowerToLLVM.cpp`: + - `CallInternalOpLowering` (~L1720) rewrites any call whose callee is `HybridFunctionType` + into `CallHybridInternalOp`. + - `CallHybridInternalOpLowering` (~L1751) always emits `GetThisOp` -> `Cast` to boolean -> + `conditionalBlocksLowering` with a with-this call and a no-this call. + - `InvokeOpLowering` (~L1865) and `InvokeHybridOpLowering` (~L1902) have the same shape for + calls inside `try`. + +## Proposed fix (pick one; A is the smallest) + +### A. Fold at call lowering when the callee was built from a plain function + +In `CallInternalOpLowering`, before rewriting to `CallHybridInternalOp`, look at the *original* +callee `op.getOperand(0)` (not `transformed`): + +- If it is defined by `mlir_ts::CastOp` whose input type is `mlir_ts::FunctionType`, the value + has no `this`. Emit a plain indirect `LLVM::CallOp` using the cast's input, converted with + `rewriter.getRemappedValue(castOp.getIn())` or a `DialectCastOp`, and skip the hybrid path. +- If it is defined by `mlir_ts::CreateBoundFunctionOp` whose `this` operand comes from + `mlir_ts::NullOp`, do the same with the function operand. + +Apply the same check in `InvokeOpLowering` before it creates `InvokeHybridOp`. + +Watch out: this is a dialect conversion, so the defining op may already be scheduled for +replacement. Read the original op's operands and map them through the rewriter, and don't rely +on the op still being there after the call. If the cast result has no other uses it becomes dead +and should be erased by DCE, or explicitly. + +### B. Only the no-this branch in `CallHybridInternalOpLowering` + +Keep the rewrite to `CallHybridInternalOp`, but in `CallHybridInternalOpLowering` (and +`InvokeHybridOpLowering`) check whether `this` is provably null, using the same defining-op test +as in A. If it is, emit only the body of the "no this" lambda (L1824-L1838) and skip +`conditionalBlocksLowering`. + +### C. At MLIR generation + +In `MLIRGenCast.cpp`, when the target of an `Opaque`/`FunctionType` cast is a function type that +has no `this`, produce `FunctionType` instead of `HybridFunctionType`. This fixes it earliest, +but it changes what type the expression has. Check assignment compatibility: a variable of that +function type may later receive a bound method, and that is exactly why `HybridFunctionType` +exists. Only do this if the cast result is consumed directly by a call, so A/B is safer. + +### D. Fold in the `CallIndirectOp` canonicalizer (implemented) + +`SimplifyIndirectCallWithKnownCallee` (`lib/TypeScript/TypeScriptOps.cpp`) already folds indirect +calls with a known callee. It now also handles a callee defined by `ts.Cast` from `FunctionType` to +`HybridFunctionType`: the call's callee operand is replaced by the cast's input, and the cast is +erased if it has no other uses. + +Why here rather than A/B/C: + +- `mlir::createCanonicalizerPass()` runs unconditionally (not gated by `--opt`), so the fold applies + at `--opt_level=0` / `--di`. +- It runs before `LowerToAffine` turns calls inside `try` into `ts.Invoke`, so one pattern covers + both `CallInternalOp` and `InvokeOp`; the four LLVM lowerings are untouched. +- It works on plain TS dialect ops, so there is no dialect-conversion replacement hazard. +- `CallIndirectOp` accepts a `FunctionType` callee, with the same inputs and results. + +Limits: only a cast consumed directly by the call is folded. A function-typed variable +(`const g = f; g()`) keeps the runtime check, which is correct since such a variable may hold a +bound method. The `null -> HybridFunctionType` case is only materialized during LLVM lowering and is +not folded (calling it crashes anyway). + +Before the fix, the dead branch even called the plain function with an extra `ptr null` argument +(`call double @add1(ptr null, double 4.1e+01)`), which was only harmless because it was unreachable. + +## Expected output after the fix + +```llvm +%42 = load ptr, ptr %1 +%51 = call ptr %24(ptr %42) +``` + +## Tests to add / run + +- The repro above: check with `--emit=llvm --opt_level=0` that there is no `icmp`/`br`/`phi` around the call. +- The same call inside `try { ... }` (goes through `InvokeOp` / `InvokeHybridOp`). +- Regression: a real bound method stored in a variable of function type, then called. It must + still pass `this`, e.g. + `class C { x = 1; m() { return this.x; } } const c = new C(); const g: () => number = c.m; g();` +- Regression: a `null` value cast to a function type, then called through the hybrid path + (CastLogicHelper.h ~L313). +- Run the existing test suite, `-mm=gc` and `-mm=rc` at least. diff --git a/tslang/lib/TypeScript/TypeScriptOps.cpp b/tslang/lib/TypeScript/TypeScriptOps.cpp index f01497741..6e9aecade 100644 --- a/tslang/lib/TypeScript/TypeScriptOps.cpp +++ b/tslang/lib/TypeScript/TypeScriptOps.cpp @@ -1263,6 +1263,23 @@ struct SimplifyIndirectCallWithKnownCallee : public OpRewritePattern()) + { + if (isa(castOp.getRes().getType()) && isa(castOp.getIn().getType())) + { + rewriter.modifyOpInPlace(indirectCall, [&] { indirectCall->setOperand(0, castOp.getIn()); }); + + if (castOp->use_empty()) + { + rewriter.eraseOp(castOp); + } + + return success(); + } + } + // supporting trumpoline if (auto getMethodOp = indirectCall.getCallee().getDefiningOp()) { diff --git a/tslang/test/tester/CMakeLists.txt b/tslang/test/tester/CMakeLists.txt index 43fec5ee6..c32144524 100644 --- a/tslang/test/tester/CMakeLists.txt +++ b/tslang/test/tester/CMakeLists.txt @@ -169,6 +169,7 @@ add_test(NAME test-compile-00-funcs-typed-generic-iterator COMMAND test-runner " add_test(NAME test-compile-00-funcs-nesting COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00funcs_nesting.ts") add_test(NAME test-compile-00-funcs-nesting-generic COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00funcs_nesting_generic.ts") add_test(NAME test-compile-00-funcs-nesting-capture COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00funcs_nesting_capture.ts") +add_test(NAME test-compile-00-funcs-hybrid-null-this COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00funcs_hybrid_null_this.ts") add_test(NAME test-compile-00-funcs-expression-generic COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00funcs_expression_generic.ts") add_test(NAME test-compile-00-funcs-expression-iterator COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00funcs_expression_iterator.ts") add_test(NAME test-compile-00-arrow-generic COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00arrow_generic.ts") @@ -583,6 +584,7 @@ add_test(NAME test-jit-00-funcs-typed-generic-iterator COMMAND test-runner -jit add_test(NAME test-jit-00-funcs-nesting COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00funcs_nesting.ts") add_test(NAME test-jit-00-funcs-nesting-generic COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00funcs_nesting_generic.ts") add_test(NAME test-jit-00-funcs-nesting-capture COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00funcs_nesting_capture.ts") +add_test(NAME test-jit-00-funcs-hybrid-null-this COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00funcs_hybrid_null_this.ts") add_test(NAME test-jit-00-funcs-expression-generic COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00funcs_expression_generic.ts") add_test(NAME test-jit-00-funcs-expression-iterator COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00funcs_expression_iterator.ts") add_test(NAME test-jit-00-arrow-generic COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00arrow_generic.ts") @@ -1416,6 +1418,7 @@ set(TSLANG_CORPUS 00funcs_generic_iterator.ts 00funcs_generic_with_typeof.ts 00funcs_generic.ts + 00funcs_hybrid_null_this.ts 00funcs_nesting_capture.ts 00funcs_nesting_generic.ts 00funcs_nesting.ts diff --git a/tslang/test/tester/tests/00funcs_hybrid_null_this.ts b/tslang/test/tester/tests/00funcs_hybrid_null_this.ts new file mode 100644 index 000000000..6126019d4 --- /dev/null +++ b/tslang/test/tester/tests/00funcs_hybrid_null_this.ts @@ -0,0 +1,32 @@ +// A plain function cast to a function type is a hybrid function with a null `this`; +// calling it directly must not pass a `this` (and is lowered to a direct call). +function add1(x: number) { + return x + 1; +} + +class C { + x: number = 10; + m(): number { + return this.x; + } +} + +function main() { + const p: Opaque = add1; + const r = (<(x: number) => number>p)(41); + assert(r == 42, "direct call"); + + let r2 = 0; + try { + r2 = (<(x: number) => number>p)(1); + } finally { + assert(r2 == 2, "direct call inside try"); + } + + // a real bound method in a function-typed variable still passes `this` + const c = new C(); + const g: () => number = c.m; + assert(g() == 10, "bound method"); + + print("done."); +}