From 48b308444302f35b8a518f364411902262b191cb Mon Sep 17 00:00:00 2001 From: Frotty Date: Thu, 3 Sep 2026 20:57:45 +0200 Subject: [PATCH 1/6] Stop the local-player barrier from refusing inlining on control taint (#1284) --- AGENTS.md | 16 + LUA_HOT_PATH_SPEC.md | 448 ++++++++++++++++++ .../optimizer/LocalPlayerContextAnalyzer.java | 73 ++- .../tests/LuaBackendAuditTests.java | 44 ++ .../wurstscript/tests/OptimizerTests.java | 38 ++ 5 files changed, 610 insertions(+), 9 deletions(-) create mode 100644 LUA_HOT_PATH_SPEC.md diff --git a/AGENTS.md b/AGENTS.md index 16786cf9b..a15665195 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -226,6 +226,22 @@ Recent fixes established additional rules for backend work. Follow these for all requirement: common optimized paths must not retain avoidable compiler-introduced allocation, dispatch, copying, or bookkeeping overhead. +### Lua performance policy + +* **Wurst-emitted constructs are consumed by Wurst code.** Never add runtime coercion, nil guards, + normalisation wrappers or other defensive code to emitted Lua whose justification is that foreign + (non-Wurst) Lua might have mutated an emitted table, array or value. A user who bundles raw Lua that + writes into Wurst-emitted structures owns the result. Typed arrays already carry a metatable that + supplies the typed default; a read of a typed array is a raw table index and nothing else. +* **Leverage Lua-native mechanisms wherever semantics permit.** Prefer a metatable default over a + read-site helper, an operator over a helper call, a fixed-arity function over a `...` pack, and a + direct table over an emulated hashtable. Emulating Jass limitations on Lua needs evidence that the + limitation actually applies there. +* **A compiler-introduced call or allocation on an ordinary typed code path is a defect.** The + optimiser must be able to inline small pure helpers; an analysis barrier that refuses to inline a + function must be justified by what that function does, not by where else it happens to be called. +* The concrete open items and their acceptance criteria are in `LUA_HOT_PATH_SPEC.md`. + ### Jass/Lua feature parity * New language/compiler features must be validated for **both Jass and Lua** backends. diff --git a/LUA_HOT_PATH_SPEC.md b/LUA_HOT_PATH_SPEC.md new file mode 100644 index 000000000..a8b4592ef --- /dev/null +++ b/LUA_HOT_PATH_SPEC.md @@ -0,0 +1,448 @@ +# Lua hot-path emission: four compiler tasks + +Specification for removing compiler-introduced overhead from the Lua backend's emitted code. +Written from reading the release output (`-inline -localOptimizations`, no `-stacktraces`) of the +current compiler (`1.9.0.0-nightly-2-gd1bf06792`, which already contains #1280 and #1282) for the +standard library's `UnitSpatialIndex` and `SpatialPartition` packages, plus the inliner's own +decision log. Every claim below was read off that output, not inferred. + +Do the tasks in the order given. Task 2 is the cheapest and unblocks measuring the rest. + +## Policy this spec implements + +1. **Wurst-emitted constructs are consumed by Wurst code.** The compiler does not guard, coerce or + normalise emitted tables, arrays or values against foreign Lua that might mutate them. A user who + bundles raw Lua that writes into Wurst-emitted tables owns the consequences. Any wrapper, nil + check or coercion whose only justification is "external code could have written here" is a bug, + not a safety feature. This retires the "foreign writes" rationale introduced in #1280. +2. **Lua-native mechanisms over emulation.** Where Lua has a direct construct for what Wurst needs + (a metatable default, a table index, an integer operator, a fixed-arity function), emit that + construct. Do not emit a helper call, a vararg pack, or an IM-level shim as a workaround. +3. **A call in a hot loop is the most expensive thing the emitted code can do**, after a table + allocation. Compiler-introduced calls and allocations on ordinary typed code paths are defects. + +## Evidence + +Inner loop of `spatialIndexBeginQuery` as emitted today, per visited entry: + +```lua +next4 = __wurst_ensureInt(UnitSpatialIndex_nextInCell[idx9]) +temp34 = __wurst_ensureReal(UnitSpatialIndex_lastX[idx9]) +dy1 = (__wurst_ensureReal(UnitSpatialIndex_lastY[idx9]) - center_y) +``` + +`__wurst_ensureInt` is itself two nested calls (`__wurst_rawToNumberInt` then `__wurst_rawToInteger`), +`__wurst_ensureReal` one. That is seven Lua calls per visited entry before any work happens, on +arrays declared with a metatable whose `__index` already returns the typed default. + +Inliner decision log for the same build (`-Dwurst.inliner.log=true`), 1678 call sites: + +| decision / reason | count | +|---|---| +| keep: `local_player_context_barrier` | 577 | +| keep: `native` | 556 | +| inline | 326 | +| keep: `rating_too_high` | 146 | +| keep: `not_in_inlinable_set` | 54 | +| keep: `lua_callback_funcref_barrier` | 17 | + +105 of 546 distinct callees are refused by the local-player barrier. They include `max`, `min`, +`headSlot`, `groupSlot`, `coarseSlot`, `cellAt`, `cellCoordX`, `blockOfCell`, `currentMaxDisplacement`, +`unit_getX`, `rect_getMinX`. None of them touches a client-local native. Section "Task 2" explains why. + +Vararg lowering, `max(vararg int)` as emitted: + +```lua +function max(...) + local __args1 = table.pack(...) +``` + +One table allocation per call. `cellCoordX` calls `max` and `min` once each, so every relink in the +sweep allocates four tables, and `ArrayList.add(vararg T)` allocates one per element added. + +## Ground rules for whoever implements this + +- Follow `AGENTS.md`: failing test first, minimal patch, deterministic iteration order, both + backends validated. Run the focused suites named in each task, then the full suite once at the end. +- **Do not** solve any task with a name-based exclusion (no lists of stdlib function names, no + package-name checks, no `startsWith("__wurst")` tests beyond those that already exist). +- **Do not** keep the removed behaviour behind a flag, run arg, or annotation "just in case". Delete it. +- **Do not** narrow a task to the stdlib functions named here. The fixes are structural and apply to + every program. +- **Evidence over reasoning.** For inliner questions, run with `-Dwurst.inliner.log=true` and read the + `[INLINER]` lines. For emitted-shape questions, read the Lua. The test helper + `LuaBackendAuditTests.compileOptimizedLua` compiles with release flags and returns the source. +- **One task, one branch, one PR**, in the order given. Do not start Task 3 on a branch that still carries Task 1. Commit and PR conventions from `LOOP.md` apply (no AI or co-author references anywhere). +- **Identity, not names.** Where a task says to recognise a compiler-synthesised function (the raw div/mod natives in Task 4), compare against the `ImFunction` instance that `LuaNativeLowering` created and recorded, not its name string. AGENTS.md §7 forbids name comparison for semantic identity; keep a reference on `ImTranslator` the way `ensureIntFunc` is kept. +- When a listed test's assertion encodes the behaviour being removed, invert or delete that + assertion and say so in the commit message. Do not weaken unrelated assertions in the same test. + +--- + +## Task 1: Typed primitive array reads are raw table indexes + +### Current behaviour + +Two places wrap primitive array reads in `__wurst_ensureInt` / `__wurst_ensureReal` / +`__wurst_ensureStr`, or `(x == true)` for booleans: + +1. `LuaNativeLowering.lowerPrimitiveArrayBoundaryEnsure` (`translation/imtranslation/LuaNativeLowering.java`). + Its `visit(ImVarArrayAccess)` wraps **every** rvalue primitive array read in the program. Its + `visit(ImFunctionCall)` additionally wraps array reads passed to natives. The Javadoc says + "Arrays can be visible to foreign Lua/Jass code, so a present value can be malformed". That is the + rationale this spec retires. +2. `ExprTranslation.wrapLuaAtExternalBoundary` (`translation/imtranslation/ExprTranslation.java`) + wraps `ImVarArrayAccess` arguments at native call sites during AST-to-IM translation. + +#1280 exempted class field storage (`X_field_storage[this]`) from this. Package-level and local +arrays still pay. In the probe build, 86 `__wurst_ensureInt` and 6 `__wurst_ensureReal` call sites +remain, every one of them on a typed array read. + +### Why the wrapper is dead weight + +`LuaTranslator.getOrCreatePrimitiveArrayMetatable` gives every primitive-typed array a metatable +whose `__index` returns the typed default (`0`, `0.`, `false`, `""`). `defaultValue` installs it for +globals, locals, and nested arrays (`newDefaultArray`). A read of an unwritten key therefore already +yields the correct default with no call. A written key holds whatever typed Wurst code wrote, which +the type checker guarantees is a value of the declared type. There is no third case. + +`ensureInt` also applies `math.tointeger`. No Wurst integer expression produces a Lua float: +`div` lowers through `//` on integers, `R2I` uses `math.floor`/`math.ceil` which return the integer +subtype for representable values, and bit natives return integers. So removing the coercion changes +no observable value. + +### Required change + +- Delete `lowerPrimitiveArrayBoundaryEnsure` and its call from `LuaNativeLowering.transform`, plus + the helpers that exist only for it (`replaceWithEnsure`, `isExternalBoundary`, `isAlreadyNormalized`, + `isAlreadyNormalizedAccess`, `ensureFunctionFor`, and `callWithStacktrace`/`stacktraceParamIndex` + if nothing else uses them). +- In `ExprTranslation.wrapLuaAtExternalBoundary`, remove the `ImVarArrayAccess` branch. The method + should then be a no-op; delete it and its call sites if so. +- Fix the now-wrong Javadoc on `lua.translation.ExprTranslation.translate(ImVarArrayAccess)`, which + claims reads arrive pre-wrapped. +- **Keep** `ExprTranslation.wrapLua` and the `WurstTypeBoundTypeParam` normalisation. Erased generic + storage genuinely can hold `nil` for a primitive; that path is out of scope and must not regress. + The `ensure*` helper functions stay for it. +- Do not add any replacement flag, annotation, or opt-in. + +### Tests + +Existing assertions that encode the removed behaviour, all in `LuaBackendAuditTests`: + +- `erasedGenericPrimitiveDefaultsPropagateThroughCompositeContexts`: the final assertion with + message "global primitive array reads must remain safe for foreign writes" asserts + `__wurst_ensureInt(Test_values[0])` is present. Invert it: assert the compiled source contains + `return Test_values[0]` and does **not** contain `ensureInt(Test_values`. Keep the count assertion + on `Box_Box_get` unchanged (that is the erased-generic path). +- `seededTypeAssuranceBoundaryFuzz`: `readNormalization` and `arrayArgument` currently assert the + wrapped form for `read()` and for the native call argument. Change both to assert the **raw** + access (`TypeAssuranceFuzz_values[N]`) and assert `__wurst_ensure` is absent from `read`'s body + and from the array argument. Keep the `genericArgument` assertions (erased-generic path) as they + are. Keep the "ordinary typed values must not be normalized" assertion. +- Grep the whole test tree for `ensureInt(`, `ensureReal(`, `ensureStr(`, `ensureBool(` and + `== true)` assertions on array reads and update any others the same way. + +New tests, in `LuaBackendAuditTests`: + +- **Emitted shape.** Compile with `compileOptimizedLua` a package with `int array`, `real array`, + `bool array`, `string array` globals, a local `int array[8]`, and a function that reads each in a + `while` loop and passes one read to a `native`. Assert no `__wurst_ensure` and no `== true)` + appears anywhere in the output except inside functions whose name starts with `__wurst_`. +- **Runtime defaults still hold.** A `test().testLua(true).executeProg()` program that reads never- + written slots of all four primitive array types (including index `0`, a large index, and a slot of + a local sized array) and asserts `0`, `0.`, `false`, `""`; then writes `0`/`false`/`""` explicitly + and reads them back. This pins that the metatable, not the wrapper, was carrying the default. +- **Stacktrace mode.** Compile the same shape with `new RunArgs().with("-lua", "-stacktraces")` and + assert the same absence. The wrapper used to receive a stacktrace argument; make sure nothing + else did. + +Focused suites: `LuaBackendAuditTests`, `LuaTranslationTests`, `LuaTypecastingTests`, +`LuaNativesTests`, `LuaRunnerTests`, `FastHashMapTests`, `StdLibOwnTests`. + +--- + +## Task 2: The local-player inlining barrier must not fire on control taint + +### Root cause, precisely + +`LocalPlayerContextAnalyzer` (`intermediatelang/optimizer/LocalPlayerContextAnalyzer.java`) is a +whole-program, flow-insensitive fact propagation. Three edges combine into the over-approximation: + +1. `indexFunctionCall`: `addEnclosingControlDependency(controlContext, entryControlFact(called))`. + A callee's entry-control fact depends on the control context of **every** call site. A function + body's top-level control context is its own entry-control fact, so this is transitive over the + call graph. +2. `indexElementAfterChildren`, `ImReturn` case: + `addEnclosingControlDependency(controlContext, returnFact(owner))`. A function's RETURN fact + fires whenever its entry control is tainted, regardless of what it returns. +3. `functionInliningIsLocalPlayerSensitive` returns true when `localPlayerDependentReturns.contains(f)`. + +Consequence: any function reachable, through any chain of calls, from inside any +`if GetLocalPlayer() == ...` block anywhere in the program has a tainted RETURN fact and is refused +by the inliner at **every** call site, including ones nowhere near client-local code. In a program +that links the standard library that is most of the call graph. `headSlot(cell, groupId)` returns +`cell * 8 + groupId` and is refused. + +Edges 1 and 2 are correct for the passes that need control facts (`BranchMerger`, +`ConstantAndCopyPropagation`, `LocalMerger`, `TempMerger`, all via `isLocalPlayerDependent`). They +are irrelevant to inlining: substituting a callee body at a call site executes that body under +exactly the control context the call already had. Nothing moves across a client-local boundary. +Those passes run after inlining and re-analyse the inlined program, where the control context is +explicit, so they lose nothing. + +### Required change (implemented on `fix/inliner-local-player-barrier`) + +The barrier answers: does this function call a client-local native directly, or is its **return +value derived from one by the data flow of its own body**, independent of what callers pass in. + +Two simpler rules were tried first and are wrong; do not go back to them: + +- *USE facts only* ("transitively calls a client-local native") breaks `OptimizerTests.testInlineAnnotation`: + with the stdlib linked, `print` reaches `GetLocalPlayer`, so every function that prints stops + inlining. Calling something that uses a client-local value is not the same as producing one. +- *Data-only return facts with the ordinary argument-to-parameter edges* still barriers `max`, `min`, + `headSlot` and `cellCoordX` (75 functions in the probe). The analysis is context-insensitive: a + parameter fact merges the arguments of **every** call site, so one `max(...)` call anywhere with a + client-local argument taints `max`, then everything computed from its result. + +The implemented shape in `LocalPlayerContextAnalyzer`: + +- A second dependency map, `dataDependents`, receives every edge added through `addDependency`. + Control edges (`addEnclosingControlDependency`) and call-site argument-to-parameter edges + (`addCallArgumentDependency`, used in `indexFunctionCall` and `indexMethodCall`) go into the full + graph only. +- `propagateDataFacts()` runs after `propagateFacts()`, walks `dataDependents` from the same sources, + and publishes only RETURN facts into `localPlayerDataDependentReturns`. +- `functionInliningIsLocalPlayerSensitive(f)` is `isClientLocalValueSource(f) || functionsDirectlyUsingLocalPlayer.contains(f) || localPlayerDataDependentReturns.contains(f)`. + +Every other consumer of the analysis (`isLocalPlayerDependent`, `functionUsesLocalPlayer`) is +unchanged and still reads the full graph. + +Measured on the stdlib probe with `-Dwurst.inliner.log=true`: `local_player_context_barrier` went +from 577 call sites on 105 functions to 10 call sites on 5 functions (`init_Player`, +`PingMinimapForPlayer`, `GetPlayableMapRect`, `GetCurrentCameraBoundsMapRectBJ`, `InitMapRects`), each +of which really reaches a client-local native. + +### Tests + +- New, in `OptimizerTests`, Jass output (matches the style of `functionUsingGetLocalPlayerMustNotBeInlined`): + a pure `@inline function slot(integer a, integer b) returns integer` returning `a * 8 + b`, called + once inside `if GetLocalPlayer() == Player(0)` and once in plain code. Assert the `_inl.j` output + contains **no** `call slot(` and no `slot(` at all: both sites inlined. Add a second helper that + wraps `GetLocalPlayer()` and assert it remains a call at both sites. +- New, in `LuaBackendAuditTests` with `compileOptimizedLua`: an `int array` with two `@inline` index + helpers, a query loop, and one unrelated function containing a `GetLocalPlayer()` branch that calls + one of the helpers. Assert the loop body contains the arithmetic inline and no call to either + helper, and assert no `function (` definition survives (garbage removal drops it). +- Existing tests that must keep passing unchanged: `OptimizerTests.functionUsingGetLocalPlayerMustNotBeInlined`, + `localPlayerControlMustPropagateThroughCalledFunctions`, `localPlayerControlMustPropagateIntoFunctionReturns`, + `statementsAfterLocalEarlyReturnMustRemainLocallyControlled`, `branchMergerMustNotHoistAcrossClientLocalConditions`, + every `LocalPlayer*` test in `LuaBackendAuditTests`. + +Focused suites: `OptimizerTests`, `LuaBackendAuditTests`, `LuaTranslationTests`, `InterpreterTests`. + +### Expected effect, to verify with the log + +Re-run a stdlib-linked compile with `-Dwurst.inliner.log=true`. `local_player_context_barrier` must +drop from hundreds to the handful of functions that really call a client-local native. `headSlot`, +`groupSlot`, `cellAt`, `max`, `min` (after Task 3), `__wurst_intDiv`, `__wurst_safe_GetUnitX` and +`unit_getX` must show `decision=inline`. + +--- + +## Task 3: Vararg calls with a static argument count are fixed-arity on Lua + +### Current behaviour + +On Jass, `VarargEliminator` (`translation/imtranslation/VarargEliminator.java`) runs in +`WurstCompilerJassImpl.transformProgToJass` after `StackTraceInjector2` and before inlining. It +generates one copy of each vararg function per distinct call arity, unrolls the `ImVarargLoop`, and +redirects the calls. Varargs never reach the backend. + +On Lua, `transformProgToLua` never runs it. `LuaTranslator` renames the last parameter to `...` and +`lua.translation.StmtTranslation.translate(ImVarargLoop, ...)` emits `table.pack(...)` plus a +`while` loop. `ImInliner.isInlineCandidate` refuses vararg functions. So `max(a, b)` allocates a +table, loops over it, and can never be inlined. + +### Required change (implemented on `lua-fixed-arity-varargs`, #1286) + +`VarargEliminator` gained a target flag, `new VarargEliminator(prog, true)`, and `transformProgToLua` +runs it after `StackTraceInjector2` and before `LuaNativeLowering`, the same relative position Jass +uses. On Lua it differs from the Jass run in three ways: + +- **Method calls are handled too.** On Lua classes still exist when this runs, so `list.add(x)` is an + `ImMethodCall`, not an `ImFunctionCall`. The Lua backend already turns a method call with exactly one + possible implementation (`!isAbstract`, implementation present, no sub-methods) into a direct call of + that implementation, so the eliminator does the same for vararg methods: it generates the copy from the + implementation with the receiver as first argument and replaces the `ImMethodCall` with an + `ImFunctionCall` to the copy. Without this, `ArrayList.add` would never have been specialised. +- **No Jass parameter cap; a Lua arity bound instead.** `LUA_MAX_SPECIALISED_VARARG_ARITY = 64`. A call + with more vararg arguments than that keeps the original `...` function, which is always still present + on this target. +- **Originals are kept.** `prog.getFunctions().removeIf(IS_VARARG)` runs on Jass only. On Lua a vararg + function may still be reached through a polymorphic `ImMethodCall`, an `ImFuncRef`, or a call above + the bound; unreferenced originals are removed by `RemoveGarbage`. + +Measured on the stdlib probe (release flags): `table.pack` occurrences went from 5 to 0 (one per emitted vararg function; the call sites that fed them numbered in the hundreds, `max` and `min` alone 87). `max` and +`min` are emitted as `max_2`/`min_2`. `ArrayList.add` no longer exists as a function at all: the +one-element copy inlines at every call site to a capacity check, one store and one increment. + +The remaining `(receiver, ...)` signatures in the output are the `dispatch_*` stubs for polymorphic +methods; they forward `...` without packing and are not vararg functions in the Wurst sense. + +Two language facts learned while writing the tests: a vararg function may have only the one parameter, +and a vararg parameter cannot be forwarded to another vararg call (`sum(rest)` is a type error). The +eliminator's forwarding branch is therefore reachable only from its own generated copies. + +### Tests + +Existing assertions to update, in `LuaBackendAuditTests`: + +- `optimizedTupleVarargLoopUsesAttachedScalarLocals` asserts `table.pack(...)` is present. Invert: + assert it is **absent**, and assert the specialised `add` has two scalar element parameters and no + loop. Keep `assertFalse(compiled.contains("tupleCopy"))`. +- The `ImVarargLoop` visitor in `compileLuaWithRunArgs` stays; it simply finds no loops. + +Existing tests that must pass unchanged: `VarargTests` (all, including +`varargAllowsMoreThan31ArgumentsInLua` and `tupleVarargPreservesElementGroupingInLua`), +`LuaBackendAuditTests.varargLoopWithBareReturn` (runtime + shape), +`localPlayerTaintFlowsThroughVarargLoopValues`, `ClassesTests.constructor_chaining_vararg`, +`LuaTranslationTests.luaFunctionRefWrapperForwardsVarargs` (that one is about `xpcall` wrappers, not +Wurst varargs, and must be untouched). + +New tests, in `LuaBackendAuditTests`: + +- **Shape.** With `compileOptimizedLua`, a `function biggest(vararg int xs) returns int` called as + `biggest(a, b)` and `biggest(a, b, c)` from a loop. Assert no `table.pack` in the output and no + `function biggest(` definition with `...`. Assert the two-argument call site was inlined to + comparisons (no `biggest` call remains) or, if the inliner rating refuses it, that a + `biggest_2(` and `biggest_3(` pair exists with fixed parameters. +- **Runtime parity.** `test().testLua(true).executeProg()` covering: zero varargs, one, several, + tuple varargs, a vararg function forwarding its varargs to another vararg function, an early + `return` inside the loop, and a vararg class method called directly on a concrete class. Each + asserts the same results the interpreter gives (`testSuccess()`). +- **Fallback.** A call with more arguments than the chosen Lua bound compiles, runs, and still + contains `table.pack` for that function only. +- **Virtual dispatch leftover.** An interface with a vararg method and two implementations, called + through the interface. Compiles and runs; the implementations keep `...`. + +Focused suites: `VarargTests`, `LuaBackendAuditTests`, `LuaTranslationTests`, `ClassesTests`, +`GenericsTests`, `StdLibOwnTests`, `OptimizerTests`. + +--- + +## Task 4: Integer `div`/`mod` lower to operators, not helper chains + +### Current behaviour + +`LuaNativeLowering.lowerDivMod` rewrites `DIV_INT` to `__wurst_intDiv(a, b)`, an IM function whose +body calls `__wurst_rawFloorDivInt(a, b)`, a "native" whose Lua body is `return a // b` +(`lua.translation.LuaNatives`). `MOD_INT` becomes `__wurst_modInt` calling `__wurst_rawFmodInt` +which is `return math.fmod(a, b)`. In the probe build `__wurst_intDiv` was never inlined (local-player +barrier, Task 2), so one `div` was three Lua calls. + +### Required change + +- The three raw natives (`__wurst_rawFloorDivInt`, `__wurst_rawFmodInt`, `__wurst_rawFmodReal`) are + intrinsics at the Lua backend, not functions. In `lua.translation.ExprTranslation.translate(ImFunctionCall, ...)` + (the same place that already pattern-matches the `I2S(1 div 0)` abort trap), translate a call to + `__wurst_rawFloorDivInt` to the binary `//` expression and calls to the two fmod natives to a direct + `math.fmod(a, b)` call expression. Do not emit the function definitions when they are only used as + intrinsics. +- After Task 2, `__wurst_intDiv` and `__wurst_modInt` (size under 20) inline at every call site. + Verify with the log; if `ImInliner` still refuses them for a reason other than the barrier, fix + that reason, do not special-case the names. +- Do **not** touch `WurstOperator.moduloInteger`, the interpreter, or constant folding. AGENTS.md §7 + requires all div/mod semantics to stay centralised; this task changes only how the raw primitive is + spelled in the emitted Lua. +- Leave the `I2S(1 div 0)` abort-trap recognition exactly as it is + (`i2sDivByZeroAbortTrapSurvivesDivModLowering` pins it). + +### Tests + +Existing tests that must pass unchanged: `LuaBackendAuditTests.integerDivModMatchJassSemanticsInLua`, +`integerDivModReferenceSemanticsInInterpreter`, `i2sDivByZeroAbortTrapSurvivesDivModLowering`, +`nonConstantDivModCallsUseSharedHelper`. `divModHelpersAreOmittedWhenUnused` asserts the raw native +"always survives somewhere"; update it to assert the `//` operator or `math.fmod` appears instead, +and that no `__wurst_rawFloorDivInt` function definition is emitted. + +New test, `LuaBackendAuditTests` with `compileOptimizedLua`: a loop doing `x div 8` and `x mod 8` on +runtime values. Assert the loop body contains `// 8` (or the inlined `intDiv` body using `//`) and +`math.fmod`, and no call to `__wurst_raw`. + +Negative-operand semantics are the whole risk here; the runtime parity test +`integerDivModMatchJassSemanticsInLua` already covers `-7 div 2`, `7 mod -2` and friends. Run it under +`testLua(true)`. + +--- + + +--- + +## Task 5 (follow-up, measured after Task 2): the rating formula refuses tiny popular helpers + +With the barrier fixed, the inliner log on the stdlib probe shows the next reason small leaves stay +as calls in hot loops is `rating_too_high`: + +| callee | body | typical decision | +|---|---|---| +| `real_floor` | `toInt` plus a sign correction | `rating_too_high(875.0>=50.0)` | +| `unit_getX` | `return __wurst_safe_GetUnitX(this)` | `rating_too_high(118.0>=50.0)` | +| `unit_getAbilityLevel` | one nil-safe native wrapper | `rating_too_high(252.0>=50.0)` | +| `__wurst_intDiv` | floor-div plus one correction | `rating_too_high(992.0>=100.0)` | +| `__wurst_ensureInt` | two nested coercions | `rating_too_high(1088.0>=50.0)` (gone after Task 1) | + +`ImInliner.getRating` is `size * (callCount - 1)` against a threshold of 50 (100 when an argument is a +constant), with an early "always inline" only when `estimateSize(f) < 20`. `estimateSize` counts every +IM node, so a one-line wrapper around a nil-safe native is already past 20, and any such wrapper with a +handful of callers is refused everywhere. That is backwards for Lua: the cost the formula guards +against is emitted-script size, and duplicating a twenty-node body at each of ten call sites is cheaper +at runtime than ten calls in a loop and negligible in size. + +### Required change + +- Raise the unconditional small-body threshold for the Lua target so that a body consisting of a + single return of one call or one arithmetic expression, with or without a nil guard, always inlines + regardless of call count. Derive the number from `estimateSize` of exactly those shapes (measure + `unit_getX`, `__wurst_safe_GetUnitX`, `real_floor`, `__wurst_intDiv` in the log with a temporary + print, then set the threshold just above the largest), and record the measured sizes in the test. +- Keep the Jass behaviour unchanged unless the same measurement shows the same win there; the map + script size limit is a real constraint on Jass and is not on Lua. +- Do not special-case names. Do not make `@inline` the answer: stdlib authors should not have to + annotate every one-line accessor, and user code will not. + +### Tests + +- `LuaBackendAuditTests`, `compileOptimizedLua`: a one-line nil-safe native wrapper and a one-line + arithmetic helper each called from eight distinct functions. Assert neither helper is called from + any of the eight bodies. +- `OptimizerTests.testInlineAnnotation` and every existing inliner test unchanged. +- Re-run the stdlib probe log: `real_floor`, `unit_getX`, `unit_getAbilityLevel`, `__wurst_intDiv` + must show `decision=inline`. + +## Acceptance for the whole spec + +Compile the standard library's `UnitSpatialIndex` on Lua with release flags (a `withStdLib()` test +that imports `SpatialIndexForUnits` and calls `unitsInRange` is enough) and read +`spatialIndexBeginQuery`. The inner `while` body over a cell chain must be, modulo local names: + +```lua +next4 = UnitSpatialIndex_nextInCell[idx9] +dx1 = (UnitSpatialIndex_lastX[idx9] - center_x) +dy1 = (UnitSpatialIndex_lastY[idx9] - center_y) +cachedDistSq = ((dx1 * dx1) + (dy1 * dy1)) +``` + +with no `__wurst_ensure`, no `table.pack`, no call to any `@inline` leaf, and `cellCoordX` reduced to +arithmetic plus at most one `R2I`/`math.floor` call. The `SpatialPartition` query must read +`SpatialPartition_cellHead[((rowBase + cx) * 8) + groupId5]` directly. + +Then run the full suite once. + +## Measurement note for stdlib authors + +`wurst_run.args` in a generated project defaults to `-stacktraces` and no `-inline`. Every emitted +function then pays `wurst_stack` bookkeeping on entry and exit, and no leaf is inlined. Benchmarks of +emitted code that are meant to inform stdlib design must use `-inline -localOptimizations` without +`-stacktraces`, or they measure the debug configuration. diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalPlayerContextAnalyzer.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalPlayerContextAnalyzer.java index 28e3b120d..da3eba241 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalPlayerContextAnalyzer.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalPlayerContextAnalyzer.java @@ -75,11 +75,16 @@ public final class LocalPlayerContextAnalyzer { Collections.newSetFromMap(new IdentityHashMap<>()); private final Set functionsDirectlyUsingLocalPlayer = Collections.newSetFromMap(new IdentityHashMap<>()); + /** Functions whose return value is derived from a client-local value by data flow alone. */ + private final Set localPlayerDataDependentReturns = + Collections.newSetFromMap(new IdentityHashMap<>()); private final Set indexedElements = Collections.newSetFromMap(new IdentityHashMap<>()); private final Set activeFacts = Collections.newSetFromMap(new IdentityHashMap<>()); private final Map> dependents = new IdentityHashMap<>(); + /** The subset of {@link #dependents} reached without any control edge. */ + private final Map> dataDependents = new IdentityHashMap<>(); private final Map variableFacts = new IdentityHashMap<>(); private final Map returnFacts = new IdentityHashMap<>(); private final Map useFacts = new IdentityHashMap<>(); @@ -143,11 +148,20 @@ public boolean functionUsesLocalPlayer(ImFunction function) { && (isClientLocalValueSource(function) || functionsUsingLocalPlayer.contains(function)); } + /** + * Whether inlining this function must be refused: it is a client-local native, calls one + * directly, or returns a value derived from one by data flow. Control taint is deliberately not + * consulted. A function reachable from a client-local branch has a tainted return fact, but + * inlining substitutes its body at the call site, where it runs under exactly the control the + * call already had, so nothing crosses a boundary. The passes which do move code + * ({@link BranchMerger}, {@link TempMerger}, ...) run after inlining and re-analyse the inlined + * program, where that control is explicit. + */ public boolean functionInliningIsLocalPlayerSensitive(ImFunction function) { return function != null && (isClientLocalValueSource(function) || functionsDirectlyUsingLocalPlayer.contains(function) - || localPlayerDependentReturns.contains(function)); + || localPlayerDataDependentReturns.contains(function)); } public boolean isLocalPlayerDependent(ImVar variable) { @@ -166,6 +180,7 @@ private void analyze(ImProg prog) { analyzeFunctions(classes.get(i).getFunctions()); } propagateFacts(); + propagateDataFacts(); } private void analyzeFunctions(List functions) { @@ -410,13 +425,13 @@ private void indexFunctionCall(ImFunctionCall call, ImFunction owner, Object con int argumentCount = arguments.size(); int positionalCount = Math.min(argumentCount, fixedParameterCount); for (int i = 0; i < positionalCount; i++) { - addDependency(arguments.get(i), + addCallArgumentDependency(arguments.get(i), variableFact(calledParameters.get(i))); } ImVar varargParameter = varargParameter(called); if (varargParameter != null) { for (int i = fixedParameterCount; i < argumentCount; i++) { - addDependency(arguments.get(i), + addCallArgumentDependency(arguments.get(i), variableFact(varargParameter)); } } @@ -450,9 +465,9 @@ private void indexMethodCall(ImMethodCall call, ImFunction owner, Object control List parameters = implementation.getParameters(); for (int i = 0; i < parameters.size(); i++) { ImVar parameter = parameters.get(i); - addDependency(receiver, variableFact(parameter)); + addCallArgumentDependency(receiver, variableFact(parameter)); for (int j = 0; j < arguments.size(); j++) { - addDependency(arguments.get(j), variableFact(parameter)); + addCallArgumentDependency(arguments.get(j), variableFact(parameter)); } } } @@ -460,7 +475,8 @@ private void indexMethodCall(ImMethodCall call, ImFunction owner, Object control private void addEnclosingControlDependency(Object controlContext, Object dependent) { if (controlContext != null) { - addDependency(controlContext, dependent); + // A control edge: present in the full graph only, never in the data graph. + dependents.computeIfAbsent(controlContext, ignored -> new ArrayList<>()).add(dependent); } } @@ -550,9 +566,18 @@ private void addLocalPlayerSource(ImFunction function) { } private void addDependency(Object dependency, Object dependent) { - dependents.computeIfAbsent(dependency, - ignored -> new ArrayList<>()) - .add(dependent); + dependents.computeIfAbsent(dependency, ignored -> new ArrayList<>()).add(dependent); + dataDependents.computeIfAbsent(dependency, ignored -> new ArrayList<>()).add(dependent); + } + + /** + * A call-site argument flowing into a callee parameter. Present in the full graph only: the data + * graph answers what a function computes from its own body, so it must not merge the arguments + * of every caller into the parameter. With that merge, one client-local argument to a shared + * helper such as {@code max} would taint the helper and everything computed from its result. + */ + private void addCallArgumentDependency(Object argument, Object parameterFact) { + dependents.computeIfAbsent(argument, ignored -> new ArrayList<>()).add(parameterFact); } private void propagateFacts() { @@ -571,6 +596,36 @@ private void propagateFacts() { } } + /** + * Second pass over the data-only graph. Publishes just the RETURN facts, which is what the + * inlining barrier needs: whether a return value is derived from a client-local value regardless + * of where the function happens to be called from. + */ + private void propagateDataFacts() { + Set reached = Collections.newSetFromMap(new IdentityHashMap<>()); + Deque worklist = new ArrayDeque<>(); + for (Object source : sourceFacts) { + if (reached.add(source)) { + worklist.addLast(source); + } + } + while (!worklist.isEmpty()) { + Object fact = worklist.removeFirst(); + if (fact instanceof Fact typedFact && typedFact.kind == FactKind.RETURN) { + localPlayerDataDependentReturns.add((ImFunction) typedFact.subject); + } + List factDependents = dataDependents.get(fact); + if (factDependents != null) { + for (int i = 0; i < factDependents.size(); i++) { + Object dependent = factDependents.get(i); + if (reached.add(dependent)) { + worklist.addLast(dependent); + } + } + } + } + } + private void activateFact(Object fact, Deque worklist) { if (activeFacts.add(fact)) { publishFact(fact); diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java index 00fbd7f68..ac17f6b75 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java @@ -1988,6 +1988,50 @@ public void localPlayerEffectfulBooleanOperandSurvivesOptimization() { compiled.indexOf("localProbe()", definitionOrCall + 1) >= 0); } + /** + * The inliner used to refuse every function whose return fact the local-player analysis had + * marked, and that fact fires for anything reachable from a client-local branch anywhere in the + * program. In a stdlib-linked map that is most of the call graph, so pure index arithmetic in + * hot loops stayed as calls. Only functions which transitively invoke a client-local native are + * an inlining barrier. + */ + @Test + public void pureHelpersReachableFromLocalPlayerBranchInlineIntoLuaHotLoops() { + String compiled = compileOptimizedLua( + "pureHelpersReachableFromLocalPlayerBranchInlineIntoLuaHotLoops", + "type player extends handle", + "package Test", + "@extern native GetLocalPlayer() returns player", + "@extern native Player(integer i) returns player", + "native consume(int i)", + "native consumePlayer(player p)", + "int array cells", + "int offset = 0", + "@inline function slotOf(int cell, int group) returns int", + " return cell * 8 + group", + "@inline function chainHead(int cell, int group) returns int", + " return cells[slotOf(cell, group)]", + "@inline function localWrapper() returns player", + " return GetLocalPlayer()", + "@noinline function query(int group)", + " var cell = 0", + " while cell < 16", + " consume(chainHead(cell, group))", + " cell++", + "init", + " if GetLocalPlayer() == Player(0)", + " consume(slotOf(offset, 1))", + " consumePlayer(localWrapper())", + " query(2)" + ); + + assertFunctionBodyContains(compiled, "query", "slotOf", false); + assertFunctionBodyContains(compiled, "query", "chainHead", false); + assertFunctionBodyContains(compiled, "query", "* 8", true); + assertTrue("a wrapper which itself calls GetLocalPlayer must stay an explicit call", + compiled.contains("consumePlayer(localWrapper())")); + } + @Test public void localPlayerTaintFlowsThroughVarargLoopValues() { String compiled = compileOptimizedLua( diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java index f37b83c95..fc6bb5ade 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java @@ -2285,6 +2285,44 @@ public void functionUsingGetLocalPlayerMustNotBeInlined() throws Exception { "transitive GetLocalPlayer wrappers must remain explicit calls"); } + /** + * The local-player analysis marks a function's return fact whenever the function is reachable + * from a client-local control region, transitively over the call graph. That fact is right for + * the passes which move code across control boundaries and wrong as an inlining barrier: + * substituting a body at a call site runs it under exactly the control the call already had. + * A pure helper called once under a GetLocalPlayer branch must still inline everywhere, while a + * wrapper which itself calls GetLocalPlayer must stay an explicit call. + */ + @Test + public void pureHelperReachableFromLocalPlayerBranchIsStillInlined() throws Exception { + test().lines( + "type player extends handle", + "package test", + "@extern native GetLocalPlayer() returns player", + "@extern native Player(integer i) returns player", + "native consume(integer i)", + "native consumePlayer(player p)", + "integer offset = 0", + "@inline function slot(integer a, integer b) returns integer", + " return a * 8 + b", + "@inline function currentPlayer() returns player", + " return GetLocalPlayer()", + "init", + " if GetLocalPlayer() == Player(0)", + " consume(slot(offset, 1))", + " consume(slot(offset, 2))", + " consumePlayer(currentPlayer())" + ); + + String inlined = Files.toString( + new File("test-output/OptimizerTests_pureHelperReachableFromLocalPlayerBranchIsStillInlined_inl.j"), + Charsets.UTF_8); + assertFalse(inlined.contains("slot("), + "a pure helper must inline at every call site, including the one under the local-player branch"); + assertTrue(inlined.contains("call consumePlayer(currentPlayer())"), + "a wrapper which calls GetLocalPlayer itself must remain an explicit call"); + } + @Test public void branchMergerMustNotHoistAcrossClientLocalConditions() throws Exception { test().lines( From be57b4ec94626d6bbaa70772e72a447a921ca0ec Mon Sep 17 00:00:00 2001 From: Frotty Date: Thu, 3 Sep 2026 22:43:20 +0200 Subject: [PATCH 2/6] Optimize Lua array reads and div/mod emission (#1288) * Optimize Lua array reads and div-mod intrinsics Remove typed primitive array normalization, invert legacy assertions to require raw reads, and keep erased-generic normalization intact. Emit raw div/mod primitives directly as Lua operators without helper definitions. * Track Lua numeric intrinsics by IM identity --- .../imtranslation/ExprTranslation.java | 52 +---- .../imtranslation/ImTranslator.java | 4 + .../imtranslation/LuaNativeLowering.java | 124 ++---------- .../lua/translation/ExprTranslation.java | 31 +-- .../lua/translation/LuaAssertions.java | 6 +- .../lua/translation/LuaNatives.java | 18 -- .../lua/translation/LuaTranslator.java | 3 + .../tests/LuaBackendAuditTests.java | 187 ++++++++++++++---- .../tests/LuaTranslationTests.java | 8 +- 9 files changed, 200 insertions(+), 233 deletions(-) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ExprTranslation.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ExprTranslation.java index 8cb2a75bc..66b7390db 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ExprTranslation.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ExprTranslation.java @@ -731,9 +731,8 @@ && isCalledOnDynamicRef(e) } ImExpr receiver = leftExpr == null ? null : leftExpr.imTranslateExpr(t, f); - boolean normalizeAtBoundary = directFunc != null && isLuaExternalBoundary(directFunc); FunctionSignature selectedSignature = t.isLuaTarget() ? e.attrFunctionSignature() : null; - ImExprs imArgs = translateExprs(arguments, t, f, normalizeAtBoundary, selectedSignature); + ImExprs imArgs = translateExprs(arguments, t, f, selectedSignature); if (calledFunc instanceof TupleDef) { // creating a new tuple... @@ -857,16 +856,11 @@ private static boolean isCalledOnDynamicRef(FunctionCall e) { } private static ImExprs translateExprs(List arguments, ImTranslator t, ImFunction f) { - return translateExprs(arguments, t, f, false); + return translateExprs(arguments, t, f, null); } private static ImExprs translateExprs(List arguments, ImTranslator t, ImFunction f, - boolean externalBoundary) { - return translateExprs(arguments, t, f, externalBoundary, null); - } - - private static ImExprs translateExprs(List arguments, ImTranslator t, ImFunction f, - boolean externalBoundary, @Nullable FunctionSignature selectedSignature) { + @Nullable FunctionSignature selectedSignature) { ImExprs result = ImExprs(); for (int i = 0; i < arguments.size(); i++) { Expr e = arguments.get(i); @@ -876,9 +870,6 @@ private static ImExprs translateExprs(List arguments, ImTranslator t, ImFu ImExpr translated = expectedType != null && isCompositeExpectedTypeExpression(e) ? translateWithExpectedType(e, t, f, expectedType) : e.imTranslateExpr(t, f); - if (externalBoundary) { - translated = wrapLuaAtExternalBoundary(e, t, translated); - } result.add(translated); } return result; @@ -888,41 +879,6 @@ static boolean isCompositeExpectedTypeExpression(Expr e) { return e instanceof ExprIfElse || e instanceof ExprUnary || e instanceof ExprStatementsBlock; } - private static boolean isLuaExternalBoundary(ImFunction function) { - return function.isNative() || function.isBj() || function.isExtern(); - } - - private static ImExpr wrapLuaAtExternalBoundary(Expr source, ImTranslator t, ImExpr translated) { - WurstType actualType = source.attrTypRaw(); - // Ordinary Wurst locals and literals already have their normal Lua - // representation. Only values which can lose their primitive default - // in Lua need normalization: raw array reads crossing into untyped - // code. Erased generic values are normalized by wrapTranslation when - // a concrete primitive context consumes them. - if (!(translated instanceof ImVarArrayAccess)) { - return translated; - } - WurstType normalized = actualType.normalize(); - ImFunction ensureType = null; - if (normalized instanceof WurstTypeInt) { - ensureType = t.ensureIntFunc; - } else if (normalized instanceof WurstTypeBool) { - ensureType = t.ensureBoolFunc; - } else if (normalized instanceof WurstTypeReal) { - ensureType = t.ensureRealFunc; - } else if (normalized instanceof WurstTypeString) { - ensureType = t.ensureStrFunc; - } - if (ensureType == null) { - return translated; - } - if (ensureType == t.ensureBoolFunc) { - return ImOperatorCall(WurstOperator.EQ, ImExprs( - translated, ImBoolVal(true))); - } - return ImFunctionCall(source, ensureType, ImTypeArguments(), ImExprs(translated), false, CallType.NORMAL); - } - private static boolean isPrimitiveType(WurstType type) { WurstType normalized = type.normalize(); return normalized instanceof WurstTypeInt @@ -944,7 +900,7 @@ public static ImExpr translateIntern(ExprNewObject e, ImTranslator t, ImFunction ImTypeArguments typeArgs = getFunctionCallTypeArguments(t, sig, e, imClass.getTypeVariables()); FunctionSignature selectedSignature = t.isLuaTarget() ? sig : null; return ImFunctionCall(e, constructorImFunc, typeArgs, - translateExprs(e.getArgs(), t, f, false, selectedSignature), false, CallType.NORMAL); + translateExprs(e.getArgs(), t, f, selectedSignature), false, CallType.NORMAL); } public static ImExprOpt translate(NoExpr e, ImTranslator translator, ImFunction f) { diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ImTranslator.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ImTranslator.java index 0e38df8e8..c0f330026 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ImTranslator.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ImTranslator.java @@ -191,6 +191,10 @@ public T canonical(T copy) { @Nullable public ImFunction ensureRealFunc = null; @Nullable public ImFunction ensureStrFunc = null; @Nullable public ImFunction stringConcatFunc = null; + // Exact synthetic nodes owned by LuaNativeLowering; backend intrinsic recognition must use identity. + @Nullable public ImFunction luaRawFloorDivIntFunc = null; + @Nullable public ImFunction luaRawFmodIntFunc = null; + @Nullable public ImFunction luaRawFmodRealFunc = null; private final Map varsForTupleVar = new Object2ObjectLinkedOpenHashMap<>(); diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaNativeLowering.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaNativeLowering.java index c4df7f309..7a733f9eb 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaNativeLowering.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaNativeLowering.java @@ -126,8 +126,7 @@ public static void transform(ImProg prog, ImTranslator translator) { } lowerStringConcatenation(prog, translator); - lowerDivMod(prog); - lowerPrimitiveArrayBoundaryEnsure(prog, translator); + lowerDivMod(prog, translator); // Maps original BJ function → replacement (IS_NATIVE stub or nil-safety wrapper). // Populated lazily during the traversal. @@ -268,8 +267,8 @@ public void visit(ImOperatorCall call) { * handler's "was this an intentional abort" check. Leave that one * expression untouched so the existing recognition still fires. */ - private static void lowerDivMod(ImProg prog) { - DivModFunctions funcs = new DivModFunctions(); + private static void lowerDivMod(ImProg prog, ImTranslator translator) { + DivModFunctions funcs = new DivModFunctions(translator); prog.accept(new Element.DefaultVisitor() { @Override public void visit(ImOperatorCall call) { @@ -352,119 +351,13 @@ private static int stacktraceParamIndex(ImFunction f) { return -1; } - /** - * Normalizes primitive array reads which can cross the Lua/Wurst boundary. - * Arrays can be visible to foreign Lua/Jass code, so a present value can - * be malformed even though the array metatable supplies defaults for - * missing keys. Lvalue writes remain raw; only rvalue reads are wrapped. - */ - private static void lowerPrimitiveArrayBoundaryEnsure(ImProg prog, ImTranslator translator) { - prog.accept(new Element.DefaultVisitor() { - @Override - public void visit(ImVarArrayAccess access) { - super.visit(access); - if (access.isUsedAsLValue() || isAlreadyNormalized(access, translator) - || isAlreadyNormalizedAccess(access, translator)) { - return; - } - replaceWithEnsure(access, access.attrTrace(), translator); - } - - @Override - public void visit(ImFunctionCall call) { - super.visit(call); - ImFunction function = call.getFunc(); - if (!isExternalBoundary(function)) { - return; - } - for (ImExpr argument : new ArrayList<>(call.getArguments())) { - if (!(argument instanceof ImVarArrayAccess) - || isAlreadyNormalized(argument, translator)) { - continue; - } - replaceWithEnsure((ImVarArrayAccess) argument, call.attrTrace(), translator); - } - } - }); - } - - private static void replaceWithEnsure(ImVarArrayAccess access, de.peeeq.wurstscript.ast.Element trace, - ImTranslator translator) { - ImFunction ensure = ensureFunctionFor(access.attrTyp(), translator); - if (ensure == null) { - return; - } - ImExpr normalized; - if (ensure == translator.ensureBoolFunc) { - normalized = JassIm.ImOperatorCall(WurstOperator.EQ, - JassIm.ImExprs(access.copy(), JassIm.ImBoolVal(true))); - } else { - normalized = callWithStacktrace(trace, ensure, JassIm.ImExprs(access.copy())); - } - access.replaceBy(normalized); - } - - private static boolean isExternalBoundary(ImFunction function) { - return !function.getName().startsWith("__wurst_") - && (function.isNative() || function.isBj() || function.isExtern()); - } - - private static boolean isAlreadyNormalized(ImExpr argument, ImTranslator translator) { - if (argument instanceof ImFunctionCall - && (((ImFunctionCall) argument).getFunc() == translator.ensureIntFunc - || ((ImFunctionCall) argument).getFunc() == translator.ensureBoolFunc - || ((ImFunctionCall) argument).getFunc() == translator.ensureRealFunc - || ((ImFunctionCall) argument).getFunc() == translator.ensureStrFunc)) { - return true; - } - if (argument instanceof ImOperatorCall) { - ImOperatorCall operator = (ImOperatorCall) argument; - return operator.getOp() == WurstOperator.EQ - && operator.getArguments().size() == 2 - && operator.getArguments().get(1) instanceof ImBoolVal - && ((ImBoolVal) operator.getArguments().get(1)).getValB(); - } - return false; - } - - private static boolean isAlreadyNormalizedAccess(ImVarArrayAccess access, ImTranslator translator) { - Element parent = access.getParent(); - Element owner = parent == null ? null : parent.getParent(); - if (owner instanceof ImFunctionCall) { - ImFunction function = ((ImFunctionCall) owner).getFunc(); - return function == translator.ensureIntFunc || function == translator.ensureBoolFunc - || function == translator.ensureRealFunc || function == translator.ensureStrFunc; - } - if (!(owner instanceof ImOperatorCall)) { - return false; - } - ImOperatorCall operator = (ImOperatorCall) owner; - return operator.getOp() == WurstOperator.EQ - && operator.getArguments().size() == 2 - && operator.getArguments().get(0) == access - && operator.getArguments().get(1) instanceof ImBoolVal - && ((ImBoolVal) operator.getArguments().get(1)).getValB(); - } - - private static ImFunction ensureFunctionFor(ImType type, ImTranslator translator) { - if (TypesHelper.isIntType(type)) { - return translator.ensureIntFunc; - } else if (TypesHelper.isBoolType(type)) { - return translator.ensureBoolFunc; - } else if (TypesHelper.isRealType(type)) { - return translator.ensureRealFunc; - } else if (TypesHelper.isStringType(type)) { - return translator.ensureStrFunc; - } - return null; - } - /** * Lazily builds (and memoizes) the div/mod helper functions and the tiny * raw-Lua-primitive natives they delegate to (Wurst's IM has no * floor-division/fmod operator of its own). */ private static final class DivModFunctions { + private final ImTranslator translator; private final List created = new ArrayList<>(); private ImFunction rawFloorDivInt; private ImFunction rawFmodInt; @@ -473,6 +366,10 @@ private static final class DivModFunctions { private ImFunction modInt; private ImFunction modReal; + private DivModFunctions(ImTranslator translator) { + this.translator = translator; + } + List createdFunctions() { return created; } @@ -508,6 +405,7 @@ ImFunction jassModInt() { private ImFunction rawFloorDivInt() { if (rawFloorDivInt == null) { rawFloorDivInt = rawNative("__wurst_rawFloorDivInt", TypesHelper.imInt()); + translator.luaRawFloorDivIntFunc = rawFloorDivInt; created.add(rawFloorDivInt); } return rawFloorDivInt; @@ -516,6 +414,7 @@ private ImFunction rawFloorDivInt() { private ImFunction rawFmodInt() { if (rawFmodInt == null) { rawFmodInt = rawNative("__wurst_rawFmodInt", TypesHelper.imInt()); + translator.luaRawFmodIntFunc = rawFmodInt; created.add(rawFmodInt); } return rawFmodInt; @@ -524,12 +423,13 @@ private ImFunction rawFmodInt() { private ImFunction rawFmodReal() { if (rawFmodReal == null) { rawFmodReal = rawNative("__wurst_rawFmodReal", TypesHelper.imReal()); + translator.luaRawFmodRealFunc = rawFmodReal; created.add(rawFmodReal); } return rawFmodReal; } - /** A native leaf with two params and a return, all of the same primitive type. Body supplied by LuaNatives. */ + /** A native leaf with two params and a return, translated as a Lua backend intrinsic. */ private static ImFunction rawNative(String name, ImType numType) { ImVar a = JassIm.ImVar(SYNTHETIC_TRACE, numType.copy(), "a", false); ImVar b = JassIm.ImVar(SYNTHETIC_TRACE, numType.copy(), "b", false); diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/ExprTranslation.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/ExprTranslation.java index 796d6ad49..50f10fa97 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/ExprTranslation.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/ExprTranslation.java @@ -136,12 +136,24 @@ public static LuaExpr translate(ImFunctionCall e, LuaTranslator tr) { } } - LuaFunction f = tr.luaFunc.getFor(e.getFunc()); // Use the immutable ImFunction name rather than f.getName(), because f is a cached // LuaFunction object shared across all call sites of this native. The setName() calls // below mutate it, so f.getName() changes after the first translation and can no longer // be relied upon for sentinel checks. String imFuncName = e.getFunc().getName(); + if (isRawNumericIntrinsic(e.getFunc(), tr)) { + if (e.getArguments().size() != 2) { + throw new CompileError(e.attrTrace().attrSource(), + imFuncName + " expects exactly two arguments"); + } + LuaExpr left = e.getArguments().get(0).translateToLua(tr); + LuaExpr right = e.getArguments().get(1).translateToLua(tr); + if (e.getFunc() == tr.imTr.luaRawFloorDivIntFunc) { + return LuaAst.LuaExprBinary(left, LuaAst.LuaOpFloorDiv(), right); + } + return LuaAst.LuaExprFunctionCallByName("math.fmod", LuaAst.LuaExprlist(left, right)); + } + LuaFunction f = tr.luaFunc.getFor(e.getFunc()); if ("I2S".equals(imFuncName) && isIntentionalThreadAbortCall(e)) { return LuaAst.LuaExprFunctionCallByName("error", LuaAst.LuaExprlist( LuaAst.LuaExprStringVal(WURST_ABORT_THREAD_SENTINEL), @@ -156,6 +168,12 @@ public static LuaExpr translate(ImFunctionCall e, LuaTranslator tr) { return LuaAst.LuaExprFunctionCall(f, tr.translateExprList(e.getArguments())); } + static boolean isRawNumericIntrinsic(ImFunction function, LuaTranslator tr) { + return function == tr.imTr.luaRawFloorDivIntFunc + || function == tr.imTr.luaRawFmodIntFunc + || function == tr.imTr.luaRawFmodRealFunc; + } + private static boolean isIntentionalThreadAbortCall(ImFunctionCall e) { if (e.getArguments().size() != 1) { return false; @@ -445,16 +463,7 @@ public static LuaExpr translate(ImVarAccess e, LuaTranslator tr) { return LuaAst.LuaExprVarAccess(tr.luaVar.getFor(e.getVar())); } - /** - * Primitive-typed array reads are wrapped in a type-normalizing helper - * call at the IM level, before the optimizer runs (see - * LuaNativeLowering#lowerPrimitiveArrayEnsure), by rewriting the read into - * a call against ImTranslator#ensureIntFunc and friends - so by the time - * an ImVarArrayAccess reaches this method, it is already either a - * genuine lvalue/raw access or an access whose type never needed - * wrapping (e.g. class/handle-typed arrays, which default to nil the - * same way an untouched Lua table key already does). - */ + /** Primitive-typed arrays carry their Wurst defaults through metatables, so every read is raw. */ public static LuaExpr translate(ImVarArrayAccess e, LuaTranslator tr) { return translateArrayAccessRaw(e, tr); } diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaAssertions.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaAssertions.java index 4c1ed5298..ada9f0957 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaAssertions.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaAssertions.java @@ -76,7 +76,11 @@ public void visit(LuaTableNamedField f) { @Override public void visit(LuaExprFunctionCallByName call) { super.visit(call); - check("call to", call.getFuncName()); + // Backend-owned qualified standard-library calls are valid Lua expressions, + // though they are deliberately not valid single identifiers. + if (!"math.fmod".equals(call.getFuncName())) { + check("call to", call.getFuncName()); + } } }); if (!invalid.isEmpty()) { diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaNatives.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaNatives.java index f4e9ef9c1..3e3797a04 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaNatives.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaNatives.java @@ -138,24 +138,6 @@ public class LuaNatives { f.getBody().add(LuaAst.LuaLiteral("return math.ceil(x)")); }); - addNative("__wurst_rawFloorDivInt", f -> { - f.getParams().add(LuaAst.LuaVariable("a", LuaAst.LuaNoExpr())); - f.getParams().add(LuaAst.LuaVariable("b", LuaAst.LuaNoExpr())); - f.getBody().add(LuaAst.LuaLiteral("return a // b")); - }); - - addNative("__wurst_rawFmodInt", f -> { - f.getParams().add(LuaAst.LuaVariable("a", LuaAst.LuaNoExpr())); - f.getParams().add(LuaAst.LuaVariable("b", LuaAst.LuaNoExpr())); - f.getBody().add(LuaAst.LuaLiteral("return math.fmod(a, b)")); - }); - - addNative("__wurst_rawFmodReal", f -> { - f.getParams().add(LuaAst.LuaVariable("a", LuaAst.LuaNoExpr())); - f.getParams().add(LuaAst.LuaVariable("b", LuaAst.LuaNoExpr())); - f.getBody().add(LuaAst.LuaLiteral("return math.fmod(a, b)")); - }); - addNative(Arrays.asList("__wurst_rawToNumberInt", "__wurst_rawToNumberReal"), f -> { f.getParams().add(LuaAst.LuaVariable("x", LuaAst.LuaNoExpr())); f.getBody().add(LuaAst.LuaLiteral("return tonumber(x)")); diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaTranslator.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaTranslator.java index ebe88d940..a7180712e 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaTranslator.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaTranslator.java @@ -725,6 +725,9 @@ private void translateFunc(ImFunction f) { // do not translate blizzard functions return; } + if (f.isNative() && ExprTranslation.isRawNumericIntrinsic(f, this)) { + return; + } LuaFunction lf = luaFunc.getFor(f); if (f.isNative()) { LuaNatives.get(lf); diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java index ac17f6b75..b95c40e6f 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java @@ -2234,7 +2234,7 @@ public void integerDivModReferenceSemanticsInInterpreter() { * specifically to stay exempt from that rewrite. */ @Test - public void ensureStrAndStringConcatNilChecksSurviveEliminateLocalTypes() throws IOException { + public void stringConcatNilCheckSurvivesEliminateLocalTypes() throws IOException { test().testLua(true).executeProg().lines( "package Test", "native testSuccess()", @@ -2247,8 +2247,7 @@ public void ensureStrAndStringConcatNilChecksSurviveEliminateLocalTypes() throws " print(names[5])", " testSuccess()" ); - String compiled = compiledLua("ensureStrAndStringConcatNilChecksSurviveEliminateLocalTypes"); - assertNilCheckNotCorruptedToEmptyStringCheck(compiled, "__wurst_ensureStr("); + String compiled = compiledLua("stringConcatNilCheckSurvivesEliminateLocalTypes"); assertNilCheckNotCorruptedToEmptyStringCheck(compiled, "__wurst_stringConcat("); } @@ -2276,8 +2275,8 @@ public void genericNormalizationIsKeptAtNativeBoundaryOnly() { compiled.contains("consumeBool((forward(false) == true))")); assertFalse("boolean normalization must not call the ensure helper", compiled.contains("__wurst_ensureBool(forward(false))")); - assertTrue("primitive array reads crossing a native boundary must be normalized:\n" + compiled, - compiled.contains("__wurst_ensureStr(Test_values[1])")); + assertTrue("typed primitive arrays must cross native boundaries as raw reads:\n" + compiled, + compiled.contains("print(Test_values[1])")); } @Test @@ -2462,8 +2461,10 @@ public void erasedGenericPrimitiveDefaultsPropagateThroughCompositeContexts() th String compiled = compiledLua("erasedGenericPrimitiveDefaultsPropagateThroughCompositeContexts"); assertEquals("each concrete integer consumer must normalize its erased generic input", 12, countOccurrences(compiled, "__wurst_ensureInt(Box_Box_get(")); - assertTrue("global primitive array reads must remain safe for foreign writes", - compiled.contains("__wurst_ensureInt(Test_values[0])")); + assertTrue("global primitive array reads must be raw table indexes", + compiled.contains("return Test_values[0]")); + assertFalse("typed array reads must not use erased-generic normalization", + compiled.contains("ensureInt(Test_values")); } @Test @@ -2518,9 +2519,9 @@ public void erasedGenericDefaultsUseResolvedAssignmentAndDelegationTargets() thr * Seeded boundary corpus for the type-assurance change. Each case varies * the primitive type, literal value, and array slot while checking the two * unsafe paths independently: erased generic propagation and a raw array - * read. The intermediate generic functions must stay free of assurance - * calls, while global array reads and native call sites must have the - * appropriate normalization. This is intentionally compile-only: the + * read. The intermediate generic functions and typed array reads must stay + * free of assurance calls, while erased generic values keep normalization + * at concrete uses. This is intentionally compile-only: the * generated native sinks have no Warcraft runtime implementation. */ @Test @@ -2562,25 +2563,98 @@ public void seededTypeAssuranceBoundaryFuzz() { ); assertFunctionBodyContains(compiled, "forward", "__wurst_ensure", false); - String readNormalization = type.equals("bool") - ? "(TypeAssuranceFuzz_values[" + arrayIndex + "] == true)" - : "__wurst_ensure" + suffix + "(TypeAssuranceFuzz_values[" + arrayIndex + "])"; - assertFunctionBodyContains(compiled, "read", readNormalization, true); + String rawArrayRead = "TypeAssuranceFuzz_values[" + arrayIndex + "]"; + assertFunctionBodyContains(compiled, "read", rawArrayRead, true); + assertFunctionBodyContains(compiled, "read", "__wurst_ensure", false); + assertFunctionBodyContains(compiled, "read", "== true", false); String genericArgument = type.equals("bool") ? "(forward(" + literal + ") == true)" : "__wurst_ensure" + suffix + "(forward(" + literal + "))"; assertTrue("generic boundary case " + caseIndex + " was not normalized:\n" + compiled, compiled.contains(sink + "(" + genericArgument + ")")); - String arrayArgument = type.equals("bool") - ? "(TypeAssuranceFuzz_values[" + arrayIndex + "] == true)" - : "__wurst_ensure" + suffix + "(TypeAssuranceFuzz_values[" + arrayIndex + "])"; - assertTrue("array boundary case " + caseIndex + " was not normalized:\n" + compiled, - compiled.contains(sink + "(" + arrayArgument + ")")); + assertTrue("array boundary case " + caseIndex + " was not emitted raw:\n" + compiled, + compiled.contains(sink + "(" + rawArrayRead + ")")); assertTrue("ordinary typed values must not be normalized at the boundary:\n" + compiled, compiled.contains(sink + "(" + literal + ")")); } } + @Test + public void typedPrimitiveArrayReadsAreRawInOptimizedLua() { + String compiled = compileOptimizedLua( + "LuaBackendAuditTests_typedPrimitiveArrayReadsAreRawInOptimizedLua", + primitiveArrayReadShapeLines() + ); + assertFalse("typed array reads must not call assurance helpers:\n" + compiled, + compiled.contains("__wurst_ensure")); + assertFalse("boolean array reads must not be normalized with a true comparison:\n" + compiled, + compiled.contains("== true)")); + } + + @Test + public void typedPrimitiveArrayDefaultsComeFromMetatables() throws IOException { + test().testLua(true).executeProg().lines( + "package Test", + "native testSuccess()", + "int array ints", + "real array reals", + "bool array bools", + "string array strings", + "function localDefault() returns int", + " int array[8] localInts", + " return localInts[7]", + "init", + " if ints[0] == 0 and ints[1000000] == 0", + " and reals[0] == 0. and reals[1000000] == 0.", + " and not bools[0] and not bools[1000000]", + " and strings[0] == \"\" and strings[1000000] == \"\"", + " and localDefault() == 0", + " ints[3] = 0", + " reals[3] = 0.", + " bools[3] = false", + " strings[3] = \"\"", + " if ints[3] == 0 and reals[3] == 0. and not bools[3] and strings[3] == \"\"", + " testSuccess()" + ); + } + + @Test + public void typedPrimitiveArrayReadsAreRawWithStacktraces() { + String compiled = compileLuaWithRunArgs( + "LuaBackendAuditTests_typedPrimitiveArrayReadsAreRawWithStacktraces", + new RunArgs().with("-lua", "-stacktraces"), + primitiveArrayReadShapeLines() + ); + assertFalse("stacktrace mode must not restore typed-array assurance calls:\n" + compiled, + compiled.contains("__wurst_ensure")); + assertFalse("stacktrace mode must not normalize boolean reads with a true comparison:\n" + compiled, + compiled.contains("== true)")); + } + + private static String[] primitiveArrayReadShapeLines() { + return new String[]{ + "package Test", + "native consumeInt(int value)", + "int array ints", + "real array reals", + "bool array bools", + "string array strings", + "function scan(int limit) returns real", + " int array[8] localInts", + " int i = 0", + " real sum = 0.", + " while i < limit", + " sum += ints[i] + reals[i] + localInts[i]", + " if bools[i] and strings[i] == \"\"", + " sum += 1", + " consumeInt(ints[i])", + " i++", + " return sum", + "init", + " scan(8)" + }; + } + private static void assertFunctionBodyContains(String compiled, String functionName, String text, boolean expected) { int start = compiled.indexOf("function " + functionName + "("); @@ -2608,22 +2682,20 @@ private void assertNilCheckNotCorruptedToEmptyStringCheck(String compiled, Strin * backend and the interpreter for negative operands * (e.g. -7 div 2 was -4 instead of -3, and 7 mod -2 was -1 instead of 1). * - * Div/mod are now lowered to portable IM functions before the optimizer - * runs (see LuaNativeLowering#lowerDivMod), so calls with constant - * arguments - like the ones below - may get inlined away entirely rather - * than showing up as a helper call in the output. The floor-div/fmod - * *native* they delegate to (Wurst has no such IM operator) always - * survives somewhere in the output, inlined or not, so checking for it - * is robust regardless of the inliner's decision. + * Div/mod are lowered to portable IM functions before the optimizer runs + * (see LuaNativeLowering#lowerDivMod). Their raw primitive calls are Lua + * backend intrinsics, so emitted code uses // and math.fmod directly. */ @Test public void integerDivModMatchJassSemanticsInLua() throws IOException { test().testLua(true).executeProg().lines(DIV_MOD_PROG); String compiled = compiledLua("integerDivModMatchJassSemanticsInLua"); - assertTrue("div must go through the truncating floor-div correction", - compiled.contains("__wurst_rawFloorDivInt(")); - assertTrue("mod must go through the ModuloInteger-compatible fmod correction", - compiled.contains("__wurst_rawFmodInt(")); + assertTrue("div must use Lua floor division inside the truncating correction", + compiled.contains(" // ")); + assertTrue("mod must use math.fmod inside the ModuloInteger-compatible correction", + compiled.contains("math.fmod(")); + assertFalse("raw numeric primitive calls must be intrinsic", + compiled.contains("__wurst_rawF")); assertFalse("mod/div must not use math.floor directly", compiled.contains("math.floor")); } @@ -2713,6 +2785,47 @@ public void nonConstantDivModCallsUseSharedHelper() throws IOException { 1, countOccurrences(compiled, "function __wurst_modInt(")); } + @Test + public void optimizedIntegerDivModUsesLuaPrimitivesInLoop() { + String compiled = compileOptimizedLua( + "LuaBackendAuditTests_optimizedIntegerDivModUsesLuaPrimitivesInLoop", + "package Test", + "native consumeInt(int value)", + "function run(int limit)", + " int x = limit", + " while x > 0", + " consumeInt(x div 8)", + " consumeInt(x mod 8)", + " x--", + "init", + " run(32)" + ); + assertTrue("optimized div must contain Lua floor division:\n" + compiled, + compiled.contains(" // 8")); + assertTrue("optimized mod must contain math.fmod:\n" + compiled, + compiled.contains("math.fmod(")); + assertFalse("optimized loop must not call raw numeric helpers:\n" + compiled, + compiled.contains("__wurst_raw")); + } + + @Test + public void numericIntrinsicRecognitionUsesFunctionIdentity() throws IOException { + test().testLua(true).executeProg().lines( + "package Test", + "native testSuccess()", + "function __wurst_rawFmodInt(int a, int b) returns int", + " return 123", + "init", + " if __wurst_rawFmodInt(7, 2) == 123", + " testSuccess()" + ); + String compiled = compiledLua("numericIntrinsicRecognitionUsesFunctionIdentity"); + assertTrue("an ordinary same-named function must keep its definition:\n" + compiled, + compiled.contains("function __wurst_rawFmodInt(")); + assertFalse("an ordinary same-named call must not lower to fmod:\n" + compiled, + compiled.contains("math.fmod(")); + } + /** * String concatenation is lowered to a synthetic stringConcat IM function. * The polyfill and its call sites used to be linked only by both happening @@ -2819,22 +2932,16 @@ public void optimizedMovedImHelpersHaveNoDanglingReferences() { ); String[] helperNames = { - "__wurst_ensureInt", "__wurst_ensureBool", "__wurst_ensureReal", "__wurst_ensureStr", "__wurst_stringConcat", "__wurst_intDiv", "__wurst_modInt", "__wurst_modReal", - "__wurst_rawToNumberInt", "__wurst_rawToInteger", "__wurst_rawToNumberReal", - "__wurst_rawToString", "__wurst_rawConcat", "__wurst_rawFloorDivInt", - "__wurst_rawFmodInt", "__wurst_rawFmodReal" + "__wurst_rawConcat" }; for (String helperName : helperNames) { assertHelperDefinedWhenCalled(compiled, helperName); } - assertTrue("repro must exercise integer ensure lowering", compiled.contains("__wurst_rawToNumberInt")); - assertTrue("repro must exercise real ensure lowering", compiled.contains("__wurst_rawToNumberReal")); - assertTrue("repro must exercise string ensure lowering", compiled.contains("__wurst_rawToString")); assertTrue("repro must exercise string concat lowering", compiled.contains("__wurst_rawConcat")); - assertTrue("repro must exercise integer div lowering", compiled.contains("__wurst_rawFloorDivInt")); - assertTrue("repro must exercise integer mod lowering", compiled.contains("__wurst_rawFmodInt")); - assertTrue("repro must exercise real mod lowering", compiled.contains("__wurst_rawFmodReal")); + assertTrue("repro must exercise integer div lowering", compiled.contains(" // ")); + assertTrue("repro must exercise integer and real mod lowering", compiled.contains("math.fmod(")); + assertFalse("raw numeric primitive calls must not survive Lua emission", compiled.contains("__wurst_rawF")); } /** diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java index ccd7b311f..1405c74ba 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java @@ -503,7 +503,7 @@ public void lazyGenericClosureDispatchWorksInLua() throws IOException { } @Test - public void stringArrayReadIsEnsuredAtNativeBoundary() throws IOException { + public void stringArrayReadIsRawAtNativeBoundary() throws IOException { test().testLua(true).withStdLib().lines( "package Test", "string array playerName", @@ -511,8 +511,10 @@ public void stringArrayReadIsEnsuredAtNativeBoundary() throws IOException { " let i = 0", " SetPlayerName(Player(i), playerName[i])" ); - String compiled = Files.toString(new File("test-output/lua/LuaTranslationTests_stringArrayReadIsEnsuredAtNativeBoundary.lua"), Charsets.UTF_8); - assertTrue("native boundary must normalize an array read", + String compiled = Files.toString(new File("test-output/lua/LuaTranslationTests_stringArrayReadIsRawAtNativeBoundary.lua"), Charsets.UTF_8); + assertTrue("native boundary must receive a raw typed array read", + compiled.contains(", Test_playerName[")); + assertFalse("typed array reads must not use erased-generic normalization", compiled.contains("__wurst_ensureStr(Test_playerName[")); } From 8f2cb5e059ebbb20c739b8de6e99ebd0fb12e3ef Mon Sep 17 00:00:00 2001 From: Frotty Date: Thu, 3 Sep 2026 23:22:05 +0200 Subject: [PATCH 3/6] Give vararg calls a fixed-arity copy on Lua (#1286) --- .../peeeq/wurstio/WurstCompilerJassImpl.java | 8 + .../imtranslation/VarargEliminator.java | 225 ++++++++++++- .../tests/LuaBackendAuditTests.java | 304 +++++++++++++++++- 3 files changed, 519 insertions(+), 18 deletions(-) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/WurstCompilerJassImpl.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/WurstCompilerJassImpl.java index 105d4520f..c37625a02 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/WurstCompilerJassImpl.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/WurstCompilerJassImpl.java @@ -897,6 +897,14 @@ public LuaCompilationUnit transformProgToLua() { timeTaker.endPhase(); } } + // Same position as on Jass: after stack traces, before lowering and inlining. Calls with a + // static argument count go to fixed-arity copies, so the emitted Lua packs no table and the + // copies can inline; originals stay for dispatch, function references and calls above the bound. + beginPhase(4, "eliminate varargs"); + new VarargEliminator(imProg, true).run(); + imTranslator.assertProperties(); + timeTaker.endPhase(); + ImTranslator imTranslator2 = getImTranslator(); ImOptimizer optimizer = new ImOptimizer(timeTaker, imTranslator2); diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/VarargEliminator.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/VarargEliminator.java index 36b3fcffd..e522281d0 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/VarargEliminator.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/VarargEliminator.java @@ -13,6 +13,7 @@ import java.util.stream.Collectors; import static de.peeeq.wurstscript.translation.imtranslation.FunctionFlagEnum.IS_VARARG; +import static de.peeeq.wurstscript.translation.imtranslation.FunctionFlagEnum.PRESERVE_NAME; /** * Takes a program and eliminates vararg functions, replacing them with @@ -21,30 +22,178 @@ public class VarargEliminator { private static final int JASS_MAX_PARAMETERS = ImHelper.JASS_MAX_PARAMETERS; + /** + * Largest number of emitted parameters a fixed-arity copy may have on Lua, counted after tuple + * flattening: one four-field tuple argument is four parameters, so an arity that looks modest in + * source can exceed what the target accepts. Lua caps a function at 200 locals including its + * parameters, and the locals-table fallback cannot spill parameters, so this leaves room for the + * body's own locals. A call above it keeps the original `...` function, which is always still + * present on that target. + */ + public static final int LUA_MAX_SPECIALISED_VARARG_PARAMETERS = 64; private final ImProg prog; + /** + * On Lua classes are still present when this runs, so a vararg function can also be reached + * through a method dispatch or a function reference. Originals are therefore kept, only direct + * calls are redirected, and unreferenced originals are left to garbage removal. + */ + private final boolean luaTarget; // original + number of args --> new function private final Table varargFuncs = HashBasedTable.create(); public VarargEliminator(ImProg prog) { + this(prog, false); + } + + public VarargEliminator(ImProg prog, boolean luaTarget) { this.prog = prog; + this.luaTarget = luaTarget; } public void run() { - // create new vararg functions - for (ImFunctionCall c : collectVarargCalls()) { - if (c.getFunc().hasFlag(IS_VARARG)) { - generateVarargFunc(c); + // Create new vararg functions. Repeated to a fixpoint: a generated copy can contain a call + // to a vararg function at an arity nothing has needed yet, which is what a recursive vararg + // function calling itself with a different argument count produces. + boolean generated = true; + while (generated) { + generated = false; + for (ImFunctionCall c : collectVarargCalls()) { + if (c.getFunc().hasFlag(IS_VARARG) && shouldSpecialise(c) && !forwardsAVarargParameter(c.getArguments()) + && !varargFuncs.contains(c.getFunc(), c.getArguments().size())) { + generateVarargFunc(c); + generated = true; + } + } + if (luaTarget) { + // The Lua backend already turns a method call with exactly one possible + // implementation into a direct call of that implementation. Doing the same here for + // vararg methods is what lets ArrayList.add and friends get a fixed-arity copy at + // all: on this target the call is still an ImMethodCall when varargs are eliminated. + for (ImMethodCall c : collectMonomorphicVarargMethodCalls()) { + ImFunction implementation = c.getMethod().getImplementation(); + List arguments = receiverAndArguments(c); + if (shouldSpecialise(arguments) && !forwardsAVarargParameter(arguments) + && !varargFuncs.contains(implementation, arguments.size())) { + generateVarargFunc(implementation, arguments, c); + generated = true; + } + } } } - // remove original vararg functions: - prog.getFunctions().removeIf(f -> f.hasFlag(IS_VARARG)); + if (!luaTarget) { + // remove original vararg functions: + prog.getFunctions().removeIf(f -> f.hasFlag(IS_VARARG)); + } // rewrite calls to use new functions: // (need to collect vararg calls again, because first phase can create copies of calls) for (ImFunctionCall call : collectVarargCalls()) { - redirectCall(call, varargFuncs.get(call.getFunc(), call.getArguments().size())); + ImFunction newFunc = varargFuncs.get(call.getFunc(), call.getArguments().size()); + if (newFunc != null && !forwardsAVarargParameter(call.getArguments())) { + redirectCall(call, newFunc); + } + } + if (luaTarget) { + for (ImMethodCall call : collectMonomorphicVarargMethodCalls()) { + ImFunction implementation = call.getMethod().getImplementation(); + ImFunction newFunc = varargFuncs.get(implementation, 1 + call.getArguments().size()); + if (newFunc != null && !forwardsAVarargParameter(receiverAndArguments(call))) { + redirectMethodCall(call, newFunc); + } + } + } + } + + + /** + * Whether a call passes a vararg placeholder straight through, which is what the generated + * `new_C` wrapper of a vararg constructor does with its own parameter. The placeholder is a + * single node standing for however many arguments the caller actually passed, so the call's node + * count is not an arity: specialising by it would produce a fixed-arity callee and drop every + * argument after the first. + * + *

Only reachable on Lua. The forwarding call lives in the body of a vararg original, and a + * copy has its placeholder expanded into real parameters before anything looks at it again, so + * this matches only originals - which Jass removes and Lua retains. + * + *

Both the generation and the rewrite loop consult this. Skipping generation alone would not + * be enough: another call could have produced a copy at the same node count, and the rewrite + * would then redirect the forwarding call to it. + */ + private static boolean forwardsAVarargParameter(List arguments) { + for (ImExpr argument : arguments) { + if (argument instanceof ImVarAccess access && isVarargPlaceholder(access.getVar())) { + return true; + } + } + return false; + } + + /** The trailing parameter of a function still marked vararg, as opposed to a local or a copy's. */ + private static boolean isVarargPlaceholder(ImVar variable) { + if (variable.getParent() == null + || !(variable.getParent().getParent() instanceof ImFunction function) + || !function.hasFlag(IS_VARARG)) { + return false; + } + List parameters = function.getParameters(); + return !parameters.isEmpty() && parameters.get(parameters.size() - 1) == variable; + } + /** A method call which can only ever reach one implementation, and that implementation is vararg. */ + private Collection collectMonomorphicVarargMethodCalls() { + final Collection calls = new ArrayList<>(); + prog.accept(new ImProg.DefaultVisitor() { + @Override + public void visit(ImMethodCall c) { + super.visit(c); + ImMethod method = c.getMethod(); + if (method != null && !method.getIsAbstract() && method.getImplementation() != null + && method.getSubMethods().isEmpty() && method.getImplementation().hasFlag(IS_VARARG)) { + calls.add(c); + } + } + }); + return calls; + } + + /** The implementation's argument list: the receiver is its first parameter. */ + private static List receiverAndArguments(ImMethodCall call) { + List arguments = new ArrayList<>(1 + call.getArguments().size()); + arguments.add(call.getReceiver()); + arguments.addAll(call.getArguments()); + return arguments; + } + + private void redirectMethodCall(ImMethodCall call, ImFunction newFunc) { + ImExprs args = JassIm.ImExprs(call.getReceiver().copy()); + args.addAll(call.getArguments().removeAll()); + call.replaceBy(JassIm.ImFunctionCall(call.getTrace(), newFunc, + JassIm.ImTypeArguments(call.getTypeArguments().removeAll()), args, + call.getTuplesEliminated(), CallType.NORMAL)); + } + + /** Whether a call gets a fixed-arity copy. Always on Jass; on Lua only within the parameter bound. */ + private boolean shouldSpecialise(ImFunctionCall call) { + return shouldSpecialise(call.getArguments()); + } + + /** + * Counted after tuple flattening, because that is what the emitted parameter list costs: twenty + * four-field tuples are eighty parameters, not twenty. + */ + private boolean shouldSpecialise(List arguments) { + if (!luaTarget) { + return true; + } + int parameters = 0; + for (ImExpr argument : arguments) { + parameters += ImHelper.flattenedJassArity(argument.attrTyp()); + if (parameters > LUA_MAX_SPECIALISED_VARARG_PARAMETERS) { + return false; + } } + return true; } @NotNull @@ -70,13 +219,17 @@ public void visit(ImFunctionCall c) { * for the function call. */ private void generateVarargFunc(ImFunctionCall sourceCall) { - ImFunction func = sourceCall.getFunc(); - int numberOfParams = sourceCall.getArguments().size(); - int jassParameterCount = sourceCall.getArguments().stream() + generateVarargFunc(sourceCall.getFunc(), sourceCall.getArguments(), sourceCall); + } + + /** {@code arguments} are in the callee's parameter order, so for a method they start with the receiver. */ + private void generateVarargFunc(ImFunction func, List arguments, Element trace) { + int numberOfParams = arguments.size(); + int jassParameterCount = arguments.stream() .mapToInt(argument -> ImHelper.flattenedJassArity(argument.attrTyp())) .sum(); - if (jassParameterCount > JASS_MAX_PARAMETERS) { - throw new CompileError(sourceCall, "Vararg call would generate " + jassParameterCount + if (!luaTarget && jassParameterCount > JASS_MAX_PARAMETERS) { + throw new CompileError(trace, "Vararg call would generate " + jassParameterCount + " Jass parameters; the maximum is " + JASS_MAX_PARAMETERS + ". Use multiple calls (for example with the cascade operator) or pass a collection instead."); } @@ -91,6 +244,31 @@ private void generateVarargFunc(ImFunctionCall sourceCall) { // Create new function ImFunction newFunc = ReferenceRewritingCopy.copy(func); + // ReferenceRewritingCopy retargets the function's own references - both call and reference + // nodes - so inside the copy they now name the copy. That is wrong for either kind. A + // recursive call must go back to naming the vararg original, so the rewrite below maps it to + // a copy of its own arity like any other call; a self reference must name the original too, + // because it is invoked at an arity this pass never sees. + newFunc.accept(new Element.DefaultVisitor() { + @Override + public void visit(ImFunctionCall call) { + super.visit(call); + if (call.getFunc() == newFunc) { + call.setFunc(func); + } + } + + @Override + public void visit(ImFuncRef ref) { + super.visit(ref); + // Lua only: nothing redirects a reference afterwards, so it keeps naming whatever it + // is set to here, and only this target retains the original. On Jass the original is + // removed below and pointing at it would leave the reference dangling. + if (luaTarget && ref.getFunc() == newFunc) { + ref.setFunc(func); + } + } + }); newFunc.setName(func.getName() + "_" + argumentSize); // replace vararg with special parameters: ImVar varargParam = newFunc.getParameters().remove(newFunc.getParameters().size() - 1); @@ -131,16 +309,23 @@ public void visit(ImVarargLoop imLoop) { params.addAll(list); // generate function for this new call - generateVarargFunc(call); + if (shouldSpecialise(call)) { + generateVarargFunc(call); + } } - // Remove vararg flag + // Drop the vararg flag, and on Lua the name preservation with it. A preserved name is part + // of the map's Warcraft-facing API and belongs to the retained original, which is what + // external code calls at an arity this pass never sees. Since a copy shares the original's + // trace, and LuaTranslator.collectPredefinedNames() resets every preserved function to its + // trace's source name, an inherited flag would emit both under one name. List list = new ArrayList<>(); for (FunctionFlag flag : newFunc.getFlags()) { - if (flag != IS_VARARG) { - list.add(flag); + if (flag == IS_VARARG || (luaTarget && flag == PRESERVE_NAME)) { + continue; } + list.add(flag); } newFunc.setFlags(list); // Add new function to prog @@ -166,7 +351,13 @@ public void visit(ImVarAccess va) { private void redirectCall(ImFunctionCall call, ImFunction newFunc) { // Redirect call to new function - ImFunctionCall newCall = JassIm.ImFunctionCall(call.getTrace(), newFunc, JassIm.ImTypeArguments(), JassIm.ImExprs(call.getArguments().removeAll()), call.getTuplesEliminated(), call.getCallType()); + // Carry the type arguments over rather than assuming there are none. Jass erases generics + // long before this pass, so an empty list was always right there; on Lua the erasure happens + // elsewhere and this list is empty in practice too, but rebuilding the call should not be + // the step that decides that. + ImFunctionCall newCall = JassIm.ImFunctionCall(call.getTrace(), newFunc, + JassIm.ImTypeArguments(call.getTypeArguments().removeAll()), + JassIm.ImExprs(call.getArguments().removeAll()), call.getTuplesEliminated(), call.getCallType()); call.replaceBy(newCall); } diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java index b95c40e6f..339e49bd4 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java @@ -271,7 +271,7 @@ public void optimizedTupleVarargLoopUsesAttachedScalarLocals() { " let bag = new Bag()", " bag.add(handles(makeFrame(), makeFrame()))" ); - assertTrue(compiled.contains("table.pack(...)")); + assertFalse("a static-arity vararg call must not pack a table on Lua", compiled.contains("table.pack(...)")); assertFalse(compiled.contains("tupleCopy")); } @@ -1988,6 +1988,308 @@ public void localPlayerEffectfulBooleanOperandSurvivesOptimization() { compiled.indexOf("localProbe()", definitionOrCall + 1) >= 0); } + /** + * On Lua a vararg function used to keep its {@code ...} parameter and pack it into a table on + * every call, and the inliner refused it. With a static argument count at the call site the + * call is redirected to a fixed-arity copy, as Jass has always done, so the pack is gone and + * the copy inlines like any other small function. + */ + @Test + public void staticArityVarargCallsAreFixedArityOnLua() { + String compiled = compileOptimizedLua( + "staticArityVarargCallsAreFixedArityOnLua", + "package Test", + "native consume(int i)", + "int array values", + "function biggest(vararg int xs) returns int", + " var best = -2147483648", + " for x in xs", + " if x > best", + " best = x", + " return best", + "@noinline function query(int a, int b, int c)", + " var i = 0", + " while i < 16", + " consume(biggest(a, values[i]))", + " consume(biggest(a, b, c))", + " i++", + "init", + " query(1, 2, 3)" + ); + assertFalse("no vararg call site may pack a table:\n" + compiled, compiled.contains("table.pack")); + assertFalse("the vararg original must not survive with a ... parameter:\n" + compiled, + compiled.contains("function biggest(...)")); + assertFunctionBodyContains(compiled, "query", "biggest(", false); + } + + @Test + public void fixedArityVarargLoweringKeepsSemanticsOnLua() throws IOException { + test().testLua(true).executeProg().lines( + "package Test", + "native testSuccess()", + "tuple pair(int x, int y)", + "function sum(vararg int xs) returns int", + " var total = 0", + " for x in xs", + " total += x", + " return total", + "function count(vararg int xs) returns int", + " var n = 0", + " for x in xs", + " n++", + " return n", + "function firstOr(vararg int xs) returns int", + " for x in xs", + " return x", + " return -1", + "function pairs(vararg pair ps) returns int", + " var result = 0", + " for p in ps", + " result = result * 100 + p.x * 10 + p.y", + " return result", + "class Bag", + " int total = 0", + " function add(vararg int xs)", + " for x in xs", + " total += x", + "init", + " let bag = new Bag()", + " bag.add(1)", + " bag.add(2, 3)", + " bag.add()", + " if sum() == 0 and sum(5) == 5 and sum(1, 2, 3, 4) == 10", + " and count() == 0 and count(9, 9, 9) == 3", + " and firstOr() == -1 and firstOr(4, 5) == 4", + " and pairs(pair(1, 2), pair(3, 4)) == 1234", + " and bag.total == 6", + " testSuccess()" + ); + String compiled = compiledLua("fixedArityVarargLoweringKeepsSemanticsOnLua"); + assertFalse("every call above has a static arity, so nothing may pack:\n" + compiled, + compiled.contains("table.pack")); + } + + @Test + public void varargCallAboveTheLuaArityBoundKeepsThePackedPath() throws IOException { + StringBuilder args = new StringBuilder(); + int n = 150; + for (int i = 1; i <= n; i++) { + if (i > 1) { + args.append(", "); + } + args.append(i); + } + test().testLua(true).executeProg().lines( + "package Test", + "native testSuccess()", + "function sum(vararg int xs) returns int", + " var total = 0", + " for x in xs", + " total += x", + " return total", + "init", + " if sum(" + args + ") == " + (n * (n + 1) / 2) + " and sum(1, 2) == 3", + " testSuccess()" + ); + String compiled = compiledLua("varargCallAboveTheLuaArityBoundKeepsThePackedPath"); + assertTrue("a call above the bound keeps the vararg original:\n" + compiled, + compiled.contains("table.pack")); + } + + /** + * A vararg constructor is reached through a compiler-generated `new_C` wrapper which forwards its + * vararg placeholder to `construct_C`. When a call above the bound keeps that wrapper as the + * retained original, its body still holds the forwarding call, and the placeholder is one node + * standing for however many arguments the caller passed. Specialising by node count would rewrite + * it to a fixed-arity constructor and silently drop every argument after the first. + */ + @Test + public void varargConstructorAboveTheLuaArityBoundKeepsThePackedPath() { + StringBuilder args = new StringBuilder(); + int n = 70; + for (int i = 1; i <= n; i++) { + if (i > 1) { + args.append(", "); + } + args.append(i); + } + test().testLua(true).executeProg().lines( + "package Test", + "native testSuccess()", + "class Tally", + " int total = 0", + " construct(vararg int xs)", + " for x in xs", + " total += x", + "init", + " let big = new Tally(" + args + ")", + " let small = new Tally(1, 2)", + " if big.total == " + (n * (n + 1) / 2) + " and small.total == 3", + " testSuccess()" + ); + } + + /** + * Jass erases generics long before this pass, so `redirectCall` could build the replacement with + * an empty type-argument list. Lua only specialises concrete operations at that point and leaves + * generics live, so dropping them leaves the redirected call typed by an unresolved type variable. + * `LuaNativeLowering` decides string concatenation from each operand's type, so a generic vararg + * returning its type parameter silently became a numeric addition on strings. + */ + @Test + public void genericVarargCallKeepsItsTypeArgumentsOnLua() { + test().testLua(true).withStdLib().executeProg().lines( + "package Test", + "function lastOf(vararg T xs) returns T", + " T result = null", + " for x in xs", + " result = x", + " return result", + "init", + " let joined = \"a\" + lastOf(\"b\", \"c\")", + " if joined == \"ac\" and lastOf(1, 2) == 2", + " testSuccess()" + ); + } + + @Test + public void virtuallyDispatchedVarargMethodKeepsThePackedPath() throws IOException { + test().testLua(true).executeProg().lines( + "package Test", + "native testSuccess()", + "interface Summer", + " function sum(vararg int xs) returns int", + "class Plain implements Summer", + " override function sum(vararg int xs) returns int", + " var total = 0", + " for x in xs", + " total += x", + " return total", + "class Doubling implements Summer", + " override function sum(vararg int xs) returns int", + " var total = 0", + " for x in xs", + " total += 2 * x", + " return total", + "init", + " Summer a = new Plain()", + " Summer b = new Doubling()", + " if a.sum(1, 2, 3) == 6 and b.sum(1, 2, 3) == 12", + " testSuccess()" + ); + } + + /** + * A vararg function may call itself with a different static arity. Copying the function for one + * arity must not retarget that inner call to the copy, which has the wrong parameter count; the + * call has to be mapped to its own arity like every other call. + */ + @Test + public void recursiveVarargCallsAreMappedToTheirOwnArity() throws IOException { + test().testLua(true).executeProg().lines( + "package Test", + "native testSuccess()", + "function depth(vararg int xs) returns int", + " var n = 0", + " for x in xs", + " n++", + " if n == 3", + " return 100 + depth(1, 2)", + " if n == 2", + " return 10 + depth(1)", + " return n", + "init", + " if depth(1, 2, 3) == 111 and depth(5) == 1 and depth() == 0", + " testSuccess()" + ); + } + + /** + * The Lua arity bound is about emitted parameters, which tuple elimination multiplies: twenty + * four-field tuples are eighty formal parameters. Such a call keeps the packed path rather than + * emitting a function Lua refuses to load. + */ + @Test + public void wideTupleVarargCallCountsFlattenedParametersAgainstTheLuaBound() throws IOException { + StringBuilder args = new StringBuilder(); + int n = 20; + for (int i = 1; i <= n; i++) { + if (i > 1) { + args.append(", "); + } + args.append("quad(").append(i).append(", 0, 0, 1)"); + } + test().testLua(true).executeProg().lines( + "package Test", + "native testSuccess()", + "tuple quad(int a, int b, int c, int d)", + "@noinline function sumFirst(vararg quad qs) returns int", + " var total = 0", + " for q in qs", + " total += q.a + q.d", + " return total", + "init", + " if sumFirst(" + args + ") == " + (n * (n + 1) / 2 + n) + " and sumFirst(quad(1, 2, 3, 4)) == 5", + " testSuccess()" + ); + String compiled = compiledLua("wideTupleVarargCallCountsFlattenedParametersAgainstTheLuaBound"); + assertTrue("eighty flattened parameters must keep the packed original:\n" + compiled, + compiled.contains("table.pack")); + } + + /** + * A vararg parameter cannot be passed on, to a function or to a method. Both halves matter here: + * a vararg function may have only the one parameter, so a receiver cannot be a second one, and + * the argument itself does not type as the element type. The eliminator's forwarding branch is + * therefore reachable only from calls it generated itself, which are always ImFunctionCall. + */ + @Test + public void varargParameterCannotBeForwardedToAMethod() { + testAssertErrorsLines(false, "Found vararg integer", + "package Test", + "class Sink", + " static Sink instance = null", + " function consume(vararg int xs)", + " skip", + "function relay(vararg int xs)", + " Sink.instance.consume(xs)" + ); + } + + + /** + * `@preserveName` and `ExecuteFunc` mark a function's emitted name as part of the map's + * WC3-facing API, and `LuaTranslator.collectPredefinedNames()` resets every function carrying + * that flag to its trace's source name. A generated copy shares the original's trace, so an + * inherited flag would emit the original and every copy under one name and let the last + * definition win. The preserved name belongs to the retained original: that is the one external + * code calls, at an arity this pass never gets to see. + */ + @Test + public void preservedNameStaysOnTheVarargOriginalNotItsCopies() { + String compiled = compileOptimizedLua( + "preservedNameStaysOnTheVarargOriginalNotItsCopies", + "package Test", + "native consume(int i)", + "@preserveName @noinline public function tally(vararg int xs) returns int", + " var sum = 0", + " for x in xs", + " sum += x", + " return sum", + "init", + " consume(tally(1, 2))" + ); + assertTrue("the fixed-arity copy must keep its own suffixed name:\n" + compiled, + compiled.contains("function tally_2(")); + int definitions = 0; + for (int at = compiled.indexOf("function tally("); at >= 0; + at = compiled.indexOf("function tally(", at + 1)) { + definitions++; + } + assertEquals("the preserved name must name exactly one function:\n" + compiled, + 1, definitions); + } + /** * The inliner used to refuse every function whose return fact the local-player analysis had * marked, and that fact fires for anything reachable from a client-local branch anywhere in the From 5ebbbdc5ba12915c0dbc092b5a3a8f0fdc707fe9 Mon Sep 17 00:00:00 2001 From: Frotty Date: Fri, 4 Sep 2026 00:28:49 +0200 Subject: [PATCH 4/6] Inline small Lua helpers regardless of popularity (#1289) --- .../translation/imoptimizer/ImInliner.java | 9 +- .../tests/LuaBackendAuditTests.java | 157 ++++++++++++++++-- 2 files changed, 154 insertions(+), 12 deletions(-) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imoptimizer/ImInliner.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imoptimizer/ImInliner.java index 7d3617bc3..5cd539817 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imoptimizer/ImInliner.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imoptimizer/ImInliner.java @@ -21,6 +21,9 @@ public class ImInliner { private static final String NOINLINE = "@noinline"; private static final double THRESHOLD_MODIFIER_CONSTANT_ARG = 2; + private static final int DEFAULT_ALWAYS_INLINE_SIZE = 20; + /** Just above the largest measured ordinary Lua leaf: unit_getAbilityLevel at 63 IM nodes. */ + private static final int LUA_ALWAYS_INLINE_SIZE = 64; private static final Set dontInline = Sets.newLinkedHashSet(); private static final boolean LOG_INLINER = Boolean.getBoolean("wurst.inliner.log"); @@ -80,6 +83,7 @@ private ImFunction inlineFunctions(ImFunction f, Element parent, int parentI, El boolean canInline = f != called && shouldInline(f, call, called); if (LOG_INLINER) { String msg = "[INLINER] caller=" + f.getName() + " callee=" + called.getName() + " decision=" + (canInline ? "inline" : "keep") + + " size=" + getFuncSize(called) + " rating=" + getRating(called) + (canInline ? "" : " reason=" + skipReason(f, call, called)); WLogger.info(msg); System.out.println(msg); @@ -322,7 +326,10 @@ private double getRating(ImFunction f) { } double size = getFuncSize(f); - if (size < 20) { + int alwaysInlineSize = translator.isLuaTarget() + ? LUA_ALWAYS_INLINE_SIZE + : DEFAULT_ALWAYS_INLINE_SIZE; + if (size < alwaysInlineSize) { // always inline small functions return 1; } diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java index 339e49bd4..58fea9e67 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java @@ -45,14 +45,24 @@ private String compiledLua(String testName) throws IOException { private String compileOptimizedLua(String testName, String... lines) { RunArgs runArgs = new RunArgs().with("-lua", "-inline", "-localOptimizations"); - return compileLuaWithRunArgs(testName, runArgs, lines); + return compileLuaWithRunArgs(testName, runArgs, false, lines); + } + + private String compileOptimizedLuaWithStdLib(String testName, String... lines) { + RunArgs runArgs = new RunArgs().with("-lua", "-inline", "-localOptimizations", + "-runcompiletimefunctions", "-lib", StdLib.getLib()); + return compileLuaWithRunArgs(testName, runArgs, true, lines); } private String compileLuaWithRunArgs(String testName, RunArgs runArgs, String... lines) { + return compileLuaWithRunArgs(testName, runArgs, false, lines); + } + + private String compileLuaWithRunArgs(String testName, RunArgs runArgs, boolean withStdLib, String... lines) { WurstGuiCliImpl gui = new WurstGuiCliImpl(); WurstCompilerJassImpl compiler = new WurstCompilerJassImpl(null, gui, null, runArgs); WurstModel model = parseFiles(Collections.emptyList(), - Collections.singletonList(new CU(testName + ".wurst", String.join("\n", lines))), false, compiler); + Collections.singletonList(new CU(testName + ".wurst", String.join("\n", lines))), withStdLib, compiler); assertTrue("unexpected parse/type errors: " + gui.getErrorList(), gui.getErrorList().isEmpty()); compiler.checkProg(model); assertTrue("unexpected compile errors: " + gui.getErrorList(), gui.getErrorList().isEmpty()); @@ -1988,6 +1998,75 @@ public void localPlayerEffectfulBooleanOperandSurvivesOptimization() { compiled.indexOf("localProbe()", definitionOrCall + 1) >= 0); } + /** + * Lua function calls are expensive enough that tiny leaf helpers should inline regardless of + * how many distinct callers use them. The old size * (callerCount - 1) rating did the opposite: + * a commonly reused one-expression helper quickly became less likely to inline. + */ + @Test + public void tinyPopularLuaHelpersInlineWithoutAnnotations() { + String compiled = compileOptimizedLua( + "tinyPopularLuaHelpersInlineWithoutAnnotations", + tinyPopularLuaHelpersProgram() + ); + + assertFalse("tiny nil-safe wrapper must inline at every Lua call site:\n" + compiled, + compiled.contains("safeCoordinate(")); + assertFalse("tiny arithmetic helper must inline at every Lua call site:\n" + compiled, + compiled.contains("arithmetic(")); + } + + /** + * Measured after Lua native lowering: unit_getX = 59 IM nodes, + * unit_getAbilityLevel = 63, real_floor = 35, and __wurst_intDiv = 31. Each helper is called + * from eight retained functions so the normal popularity rating exceeds the inline threshold; + * Lua's unconditional small-body rule must still remove every call. + */ + @Test + public void stdlibHotLeafSizesDefineLuaAlwaysInlineCutoff() { + String compiled = compileOptimizedLuaWithStdLib( + "stdlibHotLeafSizesDefineLuaAlwaysInlineCutoff", + popularStdlibHelpersProgram() + ); + for (int i = 1; i <= 8; i++) { + String caller = "caller" + i; + assertFunctionBodyContains(compiled, caller, "unit_getX(", false); + assertFunctionBodyContains(compiled, caller, "unit_getAbilityLevel(", false); + assertFunctionBodyContains(compiled, caller, "real_floor(", false); + assertFunctionBodyContains(compiled, caller, "__wurst_intDiv(", false); + } + } + + @Test + public void optimizedUnitSpatialIndexInnerLoopUsesRawLuaOperations() { + String compiled = compileOptimizedLuaWithStdLib( + "optimizedUnitSpatialIndexInnerLoopUsesRawLuaOperations", + "package Test", + "import SpatialIndexForUnits", + "@noinline function query(vec2 center)", + " let result = unitsInRange(center, 512.)", + " destroy result", + "init", + " query(vec2(0., 0.))" + ); + + // The spatial-index helpers each have one call site, so the optimizer folds the whole + // query chain into our retained entry point. Inspect that surviving hot-loop owner. + String body = topLevelFunctionBodyWithPrefix(compiled, "query"); + assertTrue("query loop must read the next-link array directly:\n" + body, + body.contains("UnitSpatialIndex_nextInCell[")); + assertTrue("query loop must read cached X directly:\n" + body, + body.contains("UnitSpatialIndex_lastX[")); + assertTrue("query loop must read cached Y directly:\n" + body, + body.contains("UnitSpatialIndex_lastY[")); + assertFalse("typed array reads must not retain assurance calls:\n" + body, + body.contains("__wurst_ensure")); + assertFalse("static-arity helpers must not allocate vararg packs:\n" + body, + body.contains("table.pack")); + assertFalse("hot query arithmetic must not retain portable div helpers:\n" + body, + body.contains("__wurst_intDiv(")); + } + /** * On Lua a vararg function used to keep its {@code ...} parameter and pack it into a table on * every call, and the inliner refused it. With a static argument count at the call site the @@ -2957,15 +3036,73 @@ private static String[] primitiveArrayReadShapeLines() { }; } + private static String[] tinyPopularLuaHelpersProgram() { + List lines = new ArrayList<>(List.of( + "type unit extends handle", + "package Test", + "@extern native GetUnitX(unit u) returns real", + "native consumeReal(real value)", + "native consumeInt(int value)", + "function safeCoordinate(unit u) returns real", + " return u == null ? 0. : GetUnitX(u)", + "function arithmetic(int value) returns int", + " return (value * 31 + 17) div 4" + )); + for (int i = 1; i <= 8; i++) { + lines.add("@noinline function caller" + i + "(unit u, int value)"); + lines.add(" consumeReal(safeCoordinate(u))"); + lines.add(" consumeInt(arithmetic(value))"); + } + lines.add("init"); + for (int i = 1; i <= 8; i++) { + lines.add(" caller" + i + "(null, " + i + ")"); + } + return lines.toArray(String[]::new); + } + + private static String[] popularStdlibHelpersProgram() { + List lines = new ArrayList<>(List.of( + "package Test", + "native consumeReal(real value)", + "native consumeInt(int value)" + )); + for (int i = 1; i <= 8; i++) { + lines.add("@noinline function caller" + i + "(unit u, real value, int divisor)"); + lines.add(" consumeReal(u.getX())"); + lines.add(" consumeInt(u.getAbilityLevel('A000'))"); + lines.add(" consumeInt(value.floor())"); + lines.add(" consumeInt(17 div divisor)"); + } + lines.add("init"); + for (int i = 1; i <= 8; i++) { + lines.add(" caller" + i + "(null, -1.5, 2)"); + } + return lines.toArray(String[]::new); + } + private static void assertFunctionBodyContains(String compiled, String functionName, String text, boolean expected) { + boolean found = functionBody(compiled, functionName).contains(text); + assertEquals("unexpected occurrence of " + text + " in " + functionName, + expected, found); + } + + private static String functionBody(String compiled, String functionName) { int start = compiled.indexOf("function " + functionName + "("); assertTrue("expected function " + functionName, start >= 0); int end = compiled.indexOf("\nend", start); assertTrue("unterminated function " + functionName, end >= 0); - boolean found = compiled.substring(start, end).contains(text); - assertEquals("unexpected occurrence of " + text + " in " + functionName, - expected, found); + return compiled.substring(start, end); + } + + private static String topLevelFunctionBodyWithPrefix(String compiled, String functionNamePrefix) { + int start = compiled.indexOf("function " + functionNamePrefix); + assertTrue("expected function starting with " + functionNamePrefix, start >= 0); + int end = compiled.indexOf("\nfunction ", start + 1); + if (end < 0) { + end = compiled.length(); + } + return compiled.substring(start, end); } private void assertNilCheckNotCorruptedToEmptyStringCheck(String compiled, String functionNamePrefix) { @@ -3164,9 +3301,9 @@ public void userFunctionNamedStringConcatDoesNotBreakConcatenation() throws IOEx * before its first call was introduced. */ @Test - public void optimizedStringConcatKeepsItsHelperDefinition() { + public void optimizedStringConcatHasNoDanglingHelperCalls() { String compiled = compileOptimizedLua( - "LuaBackendAuditTests_optimizedStringConcatKeepsItsHelperDefinition", + "LuaBackendAuditTests_optimizedStringConcatHasNoDanglingHelperCalls", "package Test", "native print(string message)", "function join1(string a, string b) returns string", @@ -3189,10 +3326,8 @@ public void optimizedStringConcatKeepsItsHelperDefinition() { " print(join5(\"a\", \"b\"))", " print(join6(\"a\", \"b\"))" ); - assertTrue("optimized Lua must retain the helper called by lowered string concatenation", - compiled.contains("function __wurst_stringConcat(")); - assertTrue("repro must contain calls in addition to the helper definition", - countOccurrences(compiled, "__wurst_stringConcat(") > 1); + assertFalse("the tiny lowered concat helper should inline without leaving dangling calls", + compiled.contains("__wurst_stringConcat(")); } /** From ff7eadca61f60ededc192d779327e8c0a9a85a98 Mon Sep 17 00:00:00 2001 From: Frotty Date: Fri, 4 Sep 2026 03:23:54 +0200 Subject: [PATCH 5/6] Deduplicate Lua callback adapters (#1290) * Deduplicate Lua callback adapters * Preserve renamed Lua callback targets --- .../lua/translation/ExprTranslation.java | 37 +-------- .../lua/translation/LuaTranslator.java | 64 ++++++++++++++++ .../tests/LuaTranslationTests.java | 76 ++++++++++++++++++- 3 files changed, 140 insertions(+), 37 deletions(-) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/ExprTranslation.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/ExprTranslation.java index 50f10fa97..3c1c92871 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/ExprTranslation.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/ExprTranslation.java @@ -53,42 +53,7 @@ public static LuaExpr translate(ImDealloc e, LuaTranslator tr) { } public static LuaExpr translate(ImFuncRef e, LuaTranslator tr) { -// return LuaAst.LuaExprFuncRef(tr.luaFunc.getFor(e.getFunc())); -// alternative: use xpcall to get stacktraces (did not work) - boolean returnsValue = !(e.getFunc().getReturnType() instanceof ImVoid); - LuaVariable dots = LuaAst.LuaVariable("...", LuaAst.LuaNoExpr()); - LuaStatements callbackBody = LuaAst.LuaStatements(); - if (returnsValue) { - LuaVariable tempRes = LuaAst.LuaVariable("tempRes", LuaAst.LuaExprNull()); - callbackBody.add(tempRes); - callbackBody.add(LuaAst.LuaExprFunctionCallByName("xpcall", - LuaAst.LuaExprlist( - LuaAst.LuaExprFunctionAbstraction( - LuaAst.LuaParams(dots.copy()), - LuaAst.LuaStatements( - LuaAst.LuaAssignment(LuaAst.LuaExprVarAccess(tempRes), - LuaAst.LuaExprFunctionCall(tr.luaFunc.getFor(e.getFunc()), LuaAst.LuaExprlist(LuaAst.LuaExprVarAccess(dots.copy()))))) - ), - LuaAst.LuaLiteral("function(err) if err == \"" + WURST_ABORT_THREAD_SENTINEL + "\" then return end BJDebugMsg(\"lua callback error: \" .. tostring(err)) xpcall(function() " + callErrorFunc(tr, "tostring(err)", "in lua callback error handler") + " end, function(err2) if err2 == \"" + WURST_ABORT_THREAD_SENTINEL + "\" then return end BJDebugMsg(\"error reporting error: \" .. tostring(err2)) BJDebugMsg(\"while reporting: \" .. tostring(err)) end) end"), - LuaAst.LuaExprVarAccess(dots.copy()) - ) - )); - callbackBody.add(LuaAst.LuaReturn(LuaAst.LuaExprVarAccess(tempRes))); - } else { - callbackBody.add(LuaAst.LuaExprFunctionCallByName("xpcall", - LuaAst.LuaExprlist( - LuaAst.LuaExprFunctionAbstraction( - LuaAst.LuaParams(dots.copy()), - LuaAst.LuaStatements( - LuaAst.LuaExprFunctionCall(tr.luaFunc.getFor(e.getFunc()), LuaAst.LuaExprlist(LuaAst.LuaExprVarAccess(dots.copy()))) - ) - ), - LuaAst.LuaLiteral("function(err) if err == \"" + WURST_ABORT_THREAD_SENTINEL + "\" then return end BJDebugMsg(\"lua callback error: \" .. tostring(err)) xpcall(function() " + callErrorFunc(tr, "tostring(err)", "in lua callback error handler") + " end, function(err2) if err2 == \"" + WURST_ABORT_THREAD_SENTINEL + "\" then return end BJDebugMsg(\"error reporting error: \" .. tostring(err2)) BJDebugMsg(\"while reporting: \" .. tostring(err)) end) end"), - LuaAst.LuaExprVarAccess(dots.copy()) - ) - )); - } - return LuaAst.LuaExprFunctionAbstraction(LuaAst.LuaParams(dots), callbackBody); + return LuaAst.LuaExprFuncRef(tr.callbackAdapterFor(e.getFunc())); } static String callErrorFunc(LuaTranslator tr, String msg) { diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaTranslator.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaTranslator.java index a7180712e..29188d783 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaTranslator.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaTranslator.java @@ -137,6 +137,8 @@ private ImProg getProg() { List tupleEqualsFuncs = new ArrayList<>(); List tupleCopyFuncs = new ArrayList<>(); + private final Map callbackAdapters = new IdentityHashMap<>(); + private LuaFunction callbackErrorHandler; // Array-default infrastructure (metatables/helper functions) shared across // every array of a given entry type, instead of allocated per array @@ -385,6 +387,68 @@ public LuaCompilationUnit translate() { return luaModel; } + /** + * Function references need an xpcall boundary, but the boundary is a property of the referenced + * function rather than of each expression which names it. Emit one reusable adapter per target + * so evaluating a function reference performs no closure allocation. + */ + LuaFunction callbackAdapterFor(ImFunction target) { + LuaFunction existing = callbackAdapters.get(target); + if (existing != null) { + return existing; + } + + LuaFunction targetLua = luaFunc.getFor(target); + LuaVariable dots = LuaAst.LuaVariable("...", LuaAst.LuaNoExpr()); + LuaFunction adapter = LuaAst.LuaFunction( + uniqueName("__wurst_callback_" + targetLua.getName()), + LuaAst.LuaParams(dots), LuaAst.LuaStatements()); + callbackAdapters.put(target, adapter); + + LuaFunction errorHandler = callbackErrorHandler(); + LuaExprFunctionCallByName xpcall = LuaAst.LuaExprFunctionCallByName("xpcall", + LuaAst.LuaExprlist( + LuaAst.LuaExprFuncRef(targetLua), + LuaAst.LuaExprFuncRef(errorHandler), + LuaAst.LuaExprVarAccess(dots.copy()))); + if (target.getReturnType() instanceof ImVoid) { + adapter.getBody().add(xpcall); + } else { + // Keep exactly the first callback result. Returning select(2, xpcall(...)) directly + // could leak additional Lua return values into a surrounding argument list. + LuaVariable ignored = LuaAst.LuaVariable("_", LuaAst.LuaNoExpr()); + LuaVariable result = LuaAst.LuaVariable("result", LuaAst.LuaNoExpr()); + adapter.getBody().add(ignored); + adapter.getBody().add(result); + adapter.getBody().add(LuaAst.LuaAssignment( + LuaAst.LuaLiteral("_, result"), xpcall)); + adapter.getBody().add(LuaAst.LuaReturn(LuaAst.LuaExprVarAccess(result))); + } + luaModel.add(adapter); + return adapter; + } + + private LuaFunction callbackErrorHandler() { + if (callbackErrorHandler != null) { + return callbackErrorHandler; + } + LuaVariable err = LuaAst.LuaVariable("err", LuaAst.LuaNoExpr()); + callbackErrorHandler = LuaAst.LuaFunction(uniqueName("__wurst_callback_error"), + LuaAst.LuaParams(err), LuaAst.LuaStatements()); + callbackErrorHandler.getBody().add(LuaAst.LuaLiteral( + "if err == \"" + ExprTranslation.WURST_ABORT_THREAD_SENTINEL + "\" then return end")); + callbackErrorHandler.getBody().add(LuaAst.LuaLiteral( + "BJDebugMsg(\"lua callback error: \" .. tostring(err))")); + callbackErrorHandler.getBody().add(LuaAst.LuaLiteral( + "xpcall(function() " + ExprTranslation.callErrorFunc(this, "tostring(err)", + "in lua callback error handler") + + " end, function(err2) if err2 == \"" + ExprTranslation.WURST_ABORT_THREAD_SENTINEL + + "\" then return end BJDebugMsg(\"error reporting error: \" .. tostring(err2))" + + " BJDebugMsg(\"while reporting: \" .. tostring(err)) end)")); + luaModel.add(callbackErrorHandler); + return callbackErrorHandler; + } + /** * Rejects calls/references to functions that an earlier optimizer pass * detached from the IM program. Without this invariant the Lua printer diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java index 1405c74ba..211830688 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java @@ -128,6 +128,15 @@ private List uniqueMatches(String output, String regex, int group) { return result; } + private int countMatches(String output, String regex) { + Matcher matcher = Pattern.compile(regex).matcher(output); + int result = 0; + while (matcher.find()) { + result++; + } + return result; + } + private String singleMatch(String output, String regex, int group) { Matcher matcher = Pattern.compile(regex).matcher(output); assertTrue("Expected pattern to occur: " + regex, matcher.find()); @@ -173,6 +182,11 @@ private String compileLuaWithRunArgs(String testName, boolean withStdLib, String private String compileLuaWithCUs(String testName, boolean withStdLib, List extraCUs, String... lines) { RunArgs runArgs = new RunArgs().with("-lua", "-inline", "-localOptimizations", "-stacktraces"); + return compileLuaWithCUs(testName, withStdLib, extraCUs, runArgs, lines); + } + + private String compileLuaWithCUs(String testName, boolean withStdLib, List extraCUs, + RunArgs runArgs, String... lines) { WurstGui gui = new WurstGuiCliImpl(); WurstCompilerJassImpl compiler = new WurstCompilerJassImpl(null, gui, null, runArgs); List inputs = new ArrayList<>(); @@ -2475,12 +2489,72 @@ public void luaFunctionRefWrapperForwardsVarargs() throws IOException { " ForForce(f, () -> skip)" ); String compiled = Files.toString(new File("test-output/lua/LuaTranslationTests_luaFunctionRefWrapperForwardsVarargs.lua"), Charsets.UTF_8); - assertTrue(compiled.contains("xpcall(function (...)")); + assertContainsRegex(compiled, "function\\s+__wurst_callback_[A-Za-z0-9_]+\\(\\.\\.\\.\\)"); + assertFalse(compiled.contains("xpcall(function (...)")); + assertContainsRegex(compiled, + "xpcall\\([A-Za-z0-9_]+, __wurst_callback_error[A-Za-z0-9_]*, \\.\\.\\.\\)"); assertTrue(compiled.contains(", ...)")); assertFalse(compiled.contains("local temp = ...")); assertFalse(compiled.contains("ForForce(f, function (...) \n\t\t\tlocal tempRes")); } + @Test + public void luaFunctionRefsReuseOneAdapterAndPreserveSingleReturn() { + String compiled = compileLuaWithCUs( + "LuaTranslationTests_luaFunctionRefsReuseOneAdapterAndPreserveSingleReturn", + false, + Collections.emptyList(), + new RunArgs().with("-lua", "-inline", "-localOptimizations"), + "type boolexpr extends handle", + "package Test", + "@extern native Condition(code callback) returns boolexpr", + "function predicate() returns boolean", + " return true", + "init", + " let first = Condition(function predicate)", + " let second = Condition(function predicate)" + ); + + List adapters = uniqueMatches(compiled, + "function\\s+(__wurst_callback_predicate[A-Za-z0-9_]*)\\(\\.\\.\\.\\)", 1); + assertEquals("one adapter must serve every reference to the same function:\n" + compiled, + 1, adapters.size()); + String adapter = adapters.get(0); + assertEquals("both Condition calls must reference the cached adapter", 2, + countMatches(compiled, "Condition\\(" + Pattern.quote(adapter) + "\\)")); + String adapterBody = getFunctionBody(compiled, adapter); + assertTrue(adapterBody.contains("_, result = xpcall(predicate,")); + assertTrue(adapterBody.contains("return result")); + assertFalse("callback sites must not allocate anonymous wrappers", compiled.contains("Condition(function (")); + } + + @Test + public void luaFunctionRefAdapterTracksLateClassFunctionRename() { + String compiled = compileLuaWithCUs( + "LuaTranslationTests_luaFunctionRefAdapterTracksLateClassFunctionRename", + false, + Collections.emptyList(), + new RunArgs().with("-lua"), + "package Test", + "@extern native consume(code callback)", + "@extern native CallbackOwner_staticCallback()", + "class CallbackOwner", + " function start()", + " consume(function staticCallback)", + " private static function staticCallback()", + " consume(function staticCallback)", + "init", + " CallbackOwner_staticCallback()", + " new CallbackOwner().start()" + ); + + String callbackName = singleMatch(compiled, + "function\\s+(CallbackOwner_[A-Za-z0-9_]*staticCallback[A-Za-z0-9_]*)\\(\\)", 1); + assertTrue("adapter must track the class callback's final name:\n" + compiled, + compiled.contains("xpcall(" + callbackName + ",")); + assertFalse(compiled.contains("xpcall(staticCallback,")); + } + @Test public void luaFunctionRefStacktraceHandlerUsesWurstStackPosition() throws IOException { CU errorHandling = new CU("ErrorHandling.wurst", String.join("\n", From 7e8016d4ece4c2bc682ac695bd53a6865972c397 Mon Sep 17 00:00:00 2001 From: Frotty Date: Fri, 4 Sep 2026 12:40:13 +0200 Subject: [PATCH 6/6] Bound Lua inlining by register pressure (#1291) --- .../peeeq/wurstio/WurstCompilerJassImpl.java | 9 + .../optimizer/ControlFlowGraph.java | 1 - .../optimizer/LocalMerger.java | 150 +++++++- .../translation/imoptimizer/ImInliner.java | 334 ++++++++++++++++- .../translation/imoptimizer/ImOptimizer.java | 5 + .../imtranslation/ImTranslator.java | 3 + .../imtranslation/LuaNativeLowering.java | 3 + .../tests/LuaTranslationTests.java | 259 ++++++++++++++ .../wurstscript/tests/OptimizerTests.java | 337 ++++++++++++++++++ 9 files changed, 1081 insertions(+), 20 deletions(-) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/WurstCompilerJassImpl.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/WurstCompilerJassImpl.java index c37625a02..bf13da627 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/WurstCompilerJassImpl.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/WurstCompilerJassImpl.java @@ -946,6 +946,15 @@ public LuaCompilationUnit transformProgToLua() { timeTaker.endPhase(); } + if (runArgs.isInline() && runArgs.isLocalOptimizations()) { + beginPhase(10, "inline Lua arithmetic helpers within allocated local budget"); + int arithmeticHelpersInlined = optimizer.inlineLuaDivModHelpersWithinLocalBudget(); + if (arithmeticHelpersInlined > 0) { + optimizer.localOptimizations(); + } + timeTaker.endPhase(); + } + printDebugImProg("./test-output/lua/im " + stage++ + "_afterlocalopts.im"); boolean garbageChanged = optimizer.removeGarbage(); diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/ControlFlowGraph.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/ControlFlowGraph.java index 1fcbd54b1..4ae58a116 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/ControlFlowGraph.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/ControlFlowGraph.java @@ -163,7 +163,6 @@ private Node getNode(ImStmt s) { result.stmt = null; } else if (s instanceof ImVarargLoop) { result.setName("vararg loop"); - result.stmt = null; } } return result; diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalMerger.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalMerger.java index d53385112..ac48751a6 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalMerger.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalMerger.java @@ -6,7 +6,6 @@ import de.peeeq.wurstscript.translation.imtranslation.ImHelper; import de.peeeq.wurstscript.translation.imtranslation.ImTranslator; import de.peeeq.wurstscript.types.TypesHelper; -import io.vavr.collection.HashSet; import io.vavr.collection.Set; import it.unimi.dsi.fastutil.objects.Object2IntOpenHashMap; import it.unimi.dsi.fastutil.objects.ObjectOpenHashSet; @@ -43,9 +42,10 @@ private void optimizeFunctions(List functions) { public String getName() { return "Local variables merged"; } void optimizeFunc(ImFunction func) { - Map> livenessInfo = calculateLiveness(func); + LivenessAnalysis liveness = analyzeLiveness(func); + Map> livenessInfo = liveness.liveOut; eliminateDeadCode(livenessInfo); - mergeLocals(livenessInfo, func); + mergeLocals(livenessInfo, liveness.liveAtEntry, func); } void optimizeFunc(ImFunction func, LocalPlayerContextAnalyzer analyzer) { @@ -55,11 +55,23 @@ void optimizeFunc(ImFunction func, LocalPlayerContextAnalyzer analyzer) { private boolean canMerge(ImType a, ImType b) { return a.equalsType(b); } - private void mergeLocals(Map> livenessInfo, ImFunction func) { - Map> interference = calculateInferenceGraph(livenessInfo); + private void mergeLocals(Map> livenessInfo, Set liveAtEntry, + ImFunction func) { + Map> interference = + calculateInterferenceGraph(livenessInfo, liveAtEntry, func); + + Map declarationOrder = new IdentityHashMap<>(); + int nextOrder = 0; + for (ImVar parameter : func.getParameters()) { + declarationOrder.put(parameter, nextOrder++); + } + for (ImVar local : func.getLocals()) { + declarationOrder.put(local, nextOrder++); + } PriorityQueue queue = new PriorityQueue<>( - (x, y) -> interference.get(y).size() - interference.get(x).size() + Comparator.comparingInt(v -> interference.get(v).size()).reversed() + .thenComparingInt(declarationOrder::get) ); queue.addAll(interference.keySet()); @@ -81,8 +93,8 @@ private void mergeLocals(Map> livenessInfo, ImFunction func) continue; } if (localPlayerContextAnalyzer != null - && (localPlayerContextAnalyzer.isLocalPlayerDependent(v) - || localPlayerContextAnalyzer.isLocalPlayerDependent(color))) { + && localPlayerContextAnalyzer.isLocalPlayerDependent(v) + != localPlayerContextAnalyzer.isLocalPlayerDependent(color)) { continue; } @@ -158,17 +170,91 @@ private static int removeUnusedLocals(ImFunction f) { return before - kept.size(); } - private Map> calculateInferenceGraph(Map> livenessInfo) { - Map> g = new LinkedHashMap<>(); - for (Map.Entry> e : livenessInfo.entrySet()) { - Set live = e.getValue(); - for (ImVar v1 : live) { - Set set = g.getOrDefault(v1, HashSet.empty()); - set = set.addAll(live.filter(v2 -> canMerge(v1.getType(), v2.getType()))); - g.put(v1, set); + private Map> calculateInterferenceGraph( + Map> livenessInfo, Set liveAtEntry, ImFunction func) { + Map> graph = new LinkedHashMap<>(); + for (ImVar parameter : func.getParameters()) { + graph.put(parameter, new ObjectOpenHashSet<>()); + } + for (ImVar local : func.getLocals()) { + graph.put(local, new ObjectOpenHashSet<>()); + } + + // A definition interferes with every compatible value that remains live after it. + // Building only those edges is equivalent to cliquing every live set, while avoiding + // the old O(statements * liveValues^2) behavior on large inlined functions. + for (Map.Entry> entry : livenessInfo.entrySet()) { + List defined = definedLocals(entry.getKey()); + if (defined.isEmpty()) { + continue; + } + for (int i = 0; i < defined.size(); i++) { + ImVar definition = defined.get(i); + java.util.Set neighbors = graph.computeIfAbsent(definition, ignored -> new ObjectOpenHashSet<>()); + for (ImVar live : entry.getValue()) { + if (live == definition || !canMerge(definition.getType(), live.getType())) { + continue; + } + neighbors.add(live); + graph.computeIfAbsent(live, ignored -> new ObjectOpenHashSet<>()).add(definition); + } + // Vararg tuple components are assigned at the same loop boundary. They must + // occupy distinct slots even when neither component is live before the loop. + for (int j = i + 1; j < defined.size(); j++) { + ImVar other = defined.get(j); + if (canMerge(definition.getType(), other.getType())) { + neighbors.add(other); + graph.computeIfAbsent(other, ignored -> new ObjectOpenHashSet<>()).add(definition); + } + } + } + } + + // A local live at entry is read before every control-flow path has assigned it. Its + // target-default value must remain distinct from every incoming parameter and from the + // other entry-live locals, even if a later assignment eventually defines it. + List entryDefinitions = new ArrayList<>(func.getParameters()); + for (ImVar local : func.getLocals()) { + if (liveAtEntry.contains(local)) { + entryDefinitions.add(local); } } - return g; + for (int i = 0; i < entryDefinitions.size(); i++) { + ImVar definition = entryDefinitions.get(i); + java.util.Set neighbors = graph.get(definition); + for (int j = i + 1; j < entryDefinitions.size(); j++) { + ImVar other = entryDefinitions.get(j); + if (canMerge(definition.getType(), other.getType())) { + neighbors.add(other); + graph.get(other).add(definition); + } + } + } + return graph; + } + + private static List definedLocals(ImStmt stmt) { + if (stmt instanceof ImVarargLoop loop) { + List result = new ArrayList<>(loop.getLoopVars().size()); + for (ImVarargLoopVar loopVar : loop.getLoopVars()) { + result.add(loopVar.getVar()); + } + return result; + } + if (!(stmt instanceof ImSet set)) { + return Collections.emptyList(); + } + ImLExpr left = set.getLeft(); + if (left instanceof ImVarAccess access && !access.getVar().isGlobal()) { + return Collections.singletonList(access.getVar()); + } + if (left instanceof ImTupleSelection selection) { + ImVar var = TypesHelper.getSimpleAndPureTupleVar(selection); + if (var != null && !var.isGlobal()) { + return Collections.singletonList(var); + } + } + return Collections.emptyList(); } private void eliminateDeadCode(Map> livenessInfo) { @@ -250,6 +336,10 @@ private static boolean hasSideEffects(Element e) { * over the strongly connected components of the control flow graph. */ public Map> calculateLiveness(ImFunction func) { + return analyzeLiveness(func).liveOut; + } + + private LivenessAnalysis analyzeLiveness(ImFunction func) { // 1. Build Control Flow Graph ControlFlowGraph cfg = new ControlFlowGraph(func.getBody()); final List nodes = cfg.getNodes(); @@ -272,6 +362,17 @@ public Map> calculateLiveness(ImFunction func) { ImStmt stmt = node.getStmt(); if (stmt == null) continue; + if (stmt instanceof ImVarargLoop loop) { + for (ImVarargLoopVar loopVar : loop.getLoopVars()) { + if (!loopVar.getVar().isGlobal()) { + def[i].add(loopVar.getVar()); + } + } + // The loop body has its own CFG nodes. Visiting it here would incorrectly + // classify all body reads as uses at the loop header. + continue; + } + final int ii = i; stmt.accept(new ImStmt.DefaultVisitor() { @Override public void visit(ImVarAccess va) { @@ -376,6 +477,19 @@ protected Collection getIncidentNodes(Node t) { result.put(stmt, io.vavr.collection.HashSet.ofAll(out[i])); } } - return result; + Set liveAtEntry = N == 0 + ? io.vavr.collection.HashSet.empty() + : io.vavr.collection.HashSet.ofAll(in[0]); + return new LivenessAnalysis(result, liveAtEntry); + } + + private static final class LivenessAnalysis { + private final Map> liveOut; + private final Set liveAtEntry; + + private LivenessAnalysis(Map> liveOut, Set liveAtEntry) { + this.liveOut = liveOut; + this.liveAtEntry = liveAtEntry; + } } } diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imoptimizer/ImInliner.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imoptimizer/ImInliner.java index 5cd539817..8bb0c82d1 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imoptimizer/ImInliner.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imoptimizer/ImInliner.java @@ -5,8 +5,10 @@ import com.google.common.collect.Sets; import de.peeeq.wurstscript.WLogger; import de.peeeq.wurstscript.intermediatelang.optimizer.LocalPlayerContextAnalyzer; +import de.peeeq.wurstscript.intermediatelang.optimizer.LocalMerger; import de.peeeq.wurstscript.jassIm.*; import de.peeeq.wurstscript.translation.imtranslation.*; +import de.peeeq.wurstscript.translation.imtranslation.purity.Pure; import de.peeeq.wurstscript.types.TypesHelper; import java.util.*; @@ -24,6 +26,10 @@ public class ImInliner { private static final int DEFAULT_ALWAYS_INLINE_SIZE = 20; /** Just above the largest measured ordinary Lua leaf: unit_getAbilityLevel at 63 IM nodes. */ private static final int LUA_ALWAYS_INLINE_SIZE = 64; + /** Leave room below Lua's hard 200-local limit for backend-introduced locals. */ + private static final int LUA_INLINE_REGISTER_BUDGET = 190; + /** Rebuild CFG liveness after expansions large enough to invalidate the incremental estimate. */ + private static final int LUA_LIVENESS_REFRESH_INLINE_SIZE = 256; private static final Set dontInline = Sets.newLinkedHashSet(); private static final boolean LOG_INLINER = Boolean.getBoolean("wurst.inliner.log"); @@ -34,6 +40,8 @@ public class ImInliner { private final Map funcSizes = Maps.newLinkedHashMap(); private final Set done = Sets.newLinkedHashSet(); private final Map containsFuncRefCache = Maps.newLinkedHashMap(); + private final Map luaRegisterBudgets = Maps.newLinkedHashMap(); + private final Map luaRegisterPressure = Maps.newLinkedHashMap(); private final double inlineTreshold = 50; private LocalPlayerContextAnalyzer localPlayerContextAnalyzer; @@ -56,6 +64,43 @@ public void doInlining() { inlineFunctions(); } + /** + * Retry the tiny compiler-owned arithmetic wrappers after local allocation has reduced the + * caller. The late check rebuilds the locality analysis and uses the same allocation classes as + * the local merger, so it cannot push Lua over the hard local-variable limit. + */ + public int inlineLuaDivModHelpersWithinLocalBudget() { + if (!translator.isLuaTarget()) { + return 0; + } + prog.flatten(translator); + localPlayerContextAnalyzer = new LocalPlayerContextAnalyzer(prog); + int changed = 0; + for (ImFunction function : sortedFunctions(ImHelper.calculateFunctionsOfProg(prog))) { + LuaRegisterBudget budget = new LuaRegisterBudget(function); + changed += inlineLuaDivModHelpers(function, function, budget); + } + return changed; + } + + private int inlineLuaDivModHelpers(ImFunction function, Element element, LuaRegisterBudget budget) { + int changed = 0; + for (int i = 0; i < element.size(); i++) { + Element child = element.get(i); + if (child instanceof ImFunctionCall call && isLuaDivModHelper(call.getFunc())) { + ImFunction callee = call.getFunc(); + if (budget.fits(call, callee)) { + budget.recordInline(call, callee); + inlineCall(function, element, i, call); + changed++; + child = element.get(i); + } + } + changed += inlineLuaDivModHelpers(function, child, budget); + } + return changed; + } + private void inlineFunctions() { for (ImFunction f : sortedFunctions(ImHelper.calculateFunctionsOfProg(prog))) { inlineFunctions(f); @@ -84,13 +129,23 @@ private ImFunction inlineFunctions(ImFunction f, Element parent, int parentI, El if (LOG_INLINER) { String msg = "[INLINER] caller=" + f.getName() + " callee=" + called.getName() + " decision=" + (canInline ? "inline" : "keep") + " size=" + getFuncSize(called) + " rating=" + getRating(called) + + (translator.isLuaTarget() && inlinableFunctions.contains(called) + ? " projectedLuaRegisters=" + getLuaRegisterBudget(f).projectedPressure(call, called) + : "") + (canInline ? "" : " reason=" + skipReason(f, call, called)); WLogger.info(msg); System.out.println(msg); } if (canInline) { if (alreadyInlined.getOrDefault(called, 0) < 5) { // check maximum to ensure termination + if (translator.isLuaTarget()) { + getLuaRegisterBudget(f).recordInline(call, called); + } inlineCall(f, parent, parentI, call); + if (translator.isLuaTarget() + && getFuncSize(called) >= LUA_LIVENESS_REFRESH_INLINE_SIZE) { + getLuaRegisterBudget(f).refresh(); + } // translator.removeCallRelation(f, called); // XXX is it safe to remove this call relation? changed[0] = true; int newSize = estimateSize(f); @@ -147,6 +202,10 @@ private String skipReason(ImFunction caller, ImFunctionCall call, ImFunction f) if (rating >= threshold) { return "rating_too_high(" + rating + ">=" + threshold + ")"; } + if (translator.isLuaTarget() && !getLuaRegisterBudget(caller).fits(call, f)) { + return "lua_register_budget(" + getLuaRegisterBudget(caller).projectedPressure(call, f) + + ">" + LUA_INLINE_REGISTER_BUDGET + ")"; + } return "unknown"; } @@ -381,7 +440,280 @@ private boolean shouldInline(ImFunction caller, ImFunctionCall call, ImFunction // WLogger.info(" rating: " + getRating(f)); return inlinableFunctions.contains(f) && getRating(f) < threshold - && !isRecursive(f); + && !isRecursive(f) + && (!translator.isLuaTarget() + || getLuaRegisterBudget(caller).fits(call, f)); + } + + private boolean isLuaDivModHelper(ImFunction function) { + return function == translator.luaIntDivFunc + || function == translator.luaModIntFunc + || function == translator.luaModRealFunc; + } + + private static int backendGeneratedLuaLocals(ImFunction function) { + int[] result = {0}; + function.getBody().accept(new ImStmts.DefaultVisitor() { + @Override + public void visit(ImVarargLoop loop) { + result[0] += 2; // Lua translation introduces __args and __i for each retained loop. + super.visit(loop); + } + }); + return result[0]; + } + + private LuaRegisterBudget getLuaRegisterBudget(ImFunction function) { + return luaRegisterBudgets.computeIfAbsent(function, LuaRegisterBudget::new); + } + + private LuaPressure estimateLuaRegisterPressure(ImFunction function) { + LuaPressure cached = luaRegisterPressure.get(function); + if (cached != null) { + return cached; + } + Map> liveness = new LocalMerger().calculateLiveness(function); + LuaPressure pressure = estimateLuaRegisterPressure(function, liveness); + luaRegisterPressure.put(function, pressure); + return pressure; + } + + private LuaPressure estimateLuaRegisterPressure(ImFunction function, + Map> liveness) { + LuaPressure maximum = pressureOf(function.getParameters()); + for (Map.Entry> entry : liveness.entrySet()) { + java.util.Set active = Collections.newSetFromMap(new IdentityHashMap<>()); + active.addAll(entry.getValue().toJavaSet()); + collectReadLocals(entry.getKey(), active); + maximum.keepMaximums(pressureOf(active)); + } + return maximum; + } + + private static void collectReadLocals(ImStmt statement, java.util.Set result) { + if (statement instanceof ImVarargLoop) { + // The loop body has separate liveness entries. Counting all of its reads at the + // header would make sequential temporaries appear simultaneously live. + return; + } + statement.accept(new ImStmt.DefaultVisitor() { + @Override + public void visit(ImVarAccess access) { + super.visit(access); + if (!access.getVar().isGlobal()) { + result.add(access.getVar()); + } + } + }); + } + + private static int statementExpressionResultSlots(ImStmt statement) { + int[] result = {0}; + statement.accept(new ImStmt.DefaultVisitor() { + @Override + public void visit(ImStatementExpr expression) { + super.visit(expression); + ImType type = expression.getExpr().attrTyp(); + if (!(type instanceof ImVoid)) { + result[0] += ImHelper.flattenedJassArity(type); + } + } + }); + return result[0]; + } + + private static int argumentStagingSlots(ImFunctionCall call) { + int result = 0; + Element current = call; + while (current != null) { + if (current != call && current instanceof ImStmt) { + break; + } + Element parent = current.getParent(); + if (parent instanceof ImExprs expressions) { + int currentIndex = -1; + for (int i = 0; i < expressions.size(); i++) { + if (expressions.get(i) == current) { + currentIndex = i; + break; + } + } + for (int i = 0; i < currentIndex; i++) { + ImExpr earlier = expressions.get(i); + if (!(earlier.attrPurity() instanceof Pure)) { + result += ImHelper.flattenedJassArity(earlier.attrTyp()); + } + } + current = expressions.getParent(); + } else { + current = parent; + } + } + return result; + } + + private LuaPressure pressureOf(Iterable variables) { + LuaPressure result = new LuaPressure(); + for (ImVar variable : variables) { + boolean localPlayerDependent = localPlayerContextAnalyzer != null + && localPlayerContextAnalyzer.isLocalPlayerDependent(variable); + result.add(variable.getType() + "|local=" + localPlayerDependent, + ImHelper.flattenedJassArity(variable.getType())); + } + return result; + } + + private static final class LuaPressure { + private final Map slotsByTypeAndLocality = new LinkedHashMap<>(); + + private LuaPressure copy() { + LuaPressure result = new LuaPressure(); + result.slotsByTypeAndLocality.putAll(slotsByTypeAndLocality); + return result; + } + + private void add(String key, int slots) { + slotsByTypeAndLocality.merge(key, slots, Integer::sum); + } + + private void addConcurrent(LuaPressure other) { + for (Map.Entry entry : other.slotsByTypeAndLocality.entrySet()) { + add(entry.getKey(), entry.getValue()); + } + } + + private void keepMaximums(LuaPressure other) { + for (Map.Entry entry : other.slotsByTypeAndLocality.entrySet()) { + slotsByTypeAndLocality.merge(entry.getKey(), entry.getValue(), Math::max); + } + } + + private int total() { + int result = 0; + for (int slots : slotsByTypeAndLocality.values()) { + result += slots; + } + return result; + } + } + + private final class LuaRegisterBudget { + private final ImFunction function; + private Map> liveness; + private LuaPressure peakPressure; + private int backendLocals; + private int declarationsWithoutAllocation; + + private LuaRegisterBudget(ImFunction function) { + this.function = function; + liveness = new LocalMerger().calculateLiveness(function); + LuaPressure cachedPressure = luaRegisterPressure.get(function); + if (cachedPressure == null) { + cachedPressure = estimateLuaRegisterPressure(function, liveness); + luaRegisterPressure.put(function, cachedPressure); + } + peakPressure = cachedPressure.copy(); + backendLocals = backendGeneratedLuaLocals(function); + declarationsWithoutAllocation = flattenedDeclarationCount(function.getParameters()) + + flattenedDeclarationCount(function.getLocals()) + + backendLocals; + } + + private boolean fits(ImFunctionCall call, ImFunction callee) { + if (!translator.getRunArgs().isLocalOptimizations()) { + return declarationsWithoutAllocation + declarationsAddedByInline(callee) + + argumentStagingSlots(call) + <= LUA_INLINE_REGISTER_BUDGET; + } + return projectedPressure(call, callee) <= LUA_INLINE_REGISTER_BUDGET + - backendLocals - backendGeneratedLuaLocals(callee); + } + + private int declarationsAddedByInline(ImFunction callee) { + return flattenedDeclarationCount(callee.getParameters()) + + flattenedDeclarationCount(callee.getLocals()) + inlineControlLocals(callee) + + backendGeneratedLuaLocals(callee); + } + + private int inlineControlLocals(ImFunction callee) { + return maxOneReturn(callee) + ? 0 + : 1 + (callee.getReturnType() instanceof ImVoid + ? 0 + : ImHelper.flattenedJassArity(callee.getReturnType())); + } + + private int flattenedDeclarationCount(ImVars variables) { + int result = 0; + for (int i = 0; i < variables.size(); i++) { + result += ImHelper.flattenedJassArity(variables.get(i).getType()); + } + return result; + } + + private void recordInline(ImFunctionCall call, ImFunction callee) { + peakPressure.keepMaximums(pressureDuringInline(call, callee)); + backendLocals += backendGeneratedLuaLocals(callee); + declarationsWithoutAllocation += declarationsAddedByInline(callee); + declarationsWithoutAllocation += argumentStagingSlots(call); + // Callers are processed after their callees. Publish the expanded pressure so a + // later caller budgets the body it will actually copy, not the pre-inline callee. + luaRegisterPressure.put(function, peakPressure.copy()); + } + + private void refresh() { + liveness = new LocalMerger().calculateLiveness(function); + peakPressure = estimateLuaRegisterPressure(function, liveness); + backendLocals = backendGeneratedLuaLocals(function); + luaRegisterPressure.put(function, peakPressure.copy()); + } + + private int projectedPressure(ImFunctionCall call, ImFunction callee) { + LuaPressure projected = peakPressure.copy(); + projected.keepMaximums(pressureDuringInline(call, callee)); + return projected.total(); + } + + private LuaPressure pressureDuringInline(ImFunctionCall call, ImFunction callee) { + LuaPressure concurrent = pressureAt(call); + concurrent.addConcurrent(estimateLuaRegisterPressure(callee)); + int earlyReturnLocals = inlineControlLocals(callee); + if (earlyReturnLocals > 0) { + // These synthetic values cannot be classified by the source locality analysis. + concurrent.add("inline-control", earlyReturnLocals); + } + int stagedArguments = argumentStagingSlots(call); + if (stagedArguments > 0) { + concurrent.add("argument-staging", stagedArguments); + } + return concurrent; + } + + private LuaPressure pressureAt(Element element) { + Element current = element; + while (current != null) { + if (current instanceof ImStmt statement) { + io.vavr.collection.Set live = liveness.get(statement); + if (live != null) { + java.util.Set active = Collections.newSetFromMap(new IdentityHashMap<>()); + active.addAll(live.toJavaSet()); + collectReadLocals(statement, active); + LuaPressure pressure = pressureOf(active); + int stagedResults = statementExpressionResultSlots(statement); + if (stagedResults > 0) { + // Flattening stages each already-inlined sibling result until the + // surrounding expression consumes it. The pre-inline liveness map + // cannot contain those future backend temporaries yet. + pressure.add("statement-expression-results", stagedResults); + } + return pressure; + } + } + current = current.getParent(); + } + // Unknown synthetic shape: remain conservative rather than risking a whole-function spill. + return peakPressure.copy(); + } } private boolean isRecursive(ImFunction f) { diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imoptimizer/ImOptimizer.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imoptimizer/ImOptimizer.java index ee63a7e8d..01d4fc2ba 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imoptimizer/ImOptimizer.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imoptimizer/ImOptimizer.java @@ -67,10 +67,15 @@ public void doInlining() { removeGarbage(); } + public int inlineLuaDivModHelpersWithinLocalBudget() { + return new ImInliner(trans).inlineLuaDivModHelpersWithinLocalBudget(); + } + private int optCount = 1; public void localOptimizations() { totalCount.clear(); + optCount = 1; removeGarbage(); diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ImTranslator.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ImTranslator.java index c0f330026..34a6bd684 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ImTranslator.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ImTranslator.java @@ -195,6 +195,9 @@ public T canonical(T copy) { @Nullable public ImFunction luaRawFloorDivIntFunc = null; @Nullable public ImFunction luaRawFmodIntFunc = null; @Nullable public ImFunction luaRawFmodRealFunc = null; + @Nullable public ImFunction luaIntDivFunc = null; + @Nullable public ImFunction luaModIntFunc = null; + @Nullable public ImFunction luaModRealFunc = null; private final Map varsForTupleVar = new Object2ObjectLinkedOpenHashMap<>(); diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaNativeLowering.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaNativeLowering.java index 7a733f9eb..b75bded09 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaNativeLowering.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaNativeLowering.java @@ -377,6 +377,7 @@ List createdFunctions() { ImFunction intDiv() { if (intDiv == null) { intDiv = buildIntDiv(rawFloorDivInt()); + translator.luaIntDivFunc = intDiv; created.add(intDiv); } return intDiv; @@ -385,6 +386,7 @@ ImFunction intDiv() { ImFunction modInt() { if (modInt == null) { modInt = buildMod("__wurst_modInt", TypesHelper.imInt(), JassIm.ImIntVal(0), rawFmodInt()); + translator.luaModIntFunc = modInt; created.add(modInt); } return modInt; @@ -393,6 +395,7 @@ ImFunction modInt() { ImFunction modReal() { if (modReal == null) { modReal = buildMod("__wurst_modReal", TypesHelper.imReal(), JassIm.ImRealVal("0."), rawFmodReal()); + translator.luaModRealFunc = modReal; created.add(modReal); } return modReal; diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java index 211830688..3aeec8bb4 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java @@ -22,6 +22,8 @@ import java.util.List; import java.util.regex.Matcher; import java.util.regex.Pattern; +import java.util.stream.Collectors; +import java.util.stream.IntStream; import static org.testng.AssertJUnit.*; @@ -2163,6 +2165,263 @@ public void inlinerDoesNotForceSpillWhenCallerStaysBelowLimit() throws IOExcepti assertTrue("caller should keep direct call in this shape", callerBody.contains("small(1)")); } + @Test + public void luaInlinerKeepsCallWhenLiveValuesWouldExceedRegisterBudget() { + List lines = new ArrayList<>(); + lines.add("package Test"); + lines.add("native takesInt(int i)"); + lines.add("@inline function helper(int x) returns int"); + for (int i = 0; i < 16; i++) { + lines.add(" let h" + i + " = x + " + i); + } + lines.add(" return " + IntStream.range(0, 16) + .mapToObj(i -> "h" + i) + .collect(Collectors.joining(" + "))); + lines.add("@noinline function caller()"); + for (int i = 0; i < 180; i++) { + lines.add(" let v" + i + " = takesIntAndReturn(" + i + ")"); + } + lines.add(" var sum = helper(1)"); + for (int i = 0; i < 180; i++) { + lines.add(" sum += v" + i); + } + lines.add(" takesInt(sum)"); + lines.add("@noinline function takesIntAndReturn(int x) returns int"); + lines.add(" takesInt(x)"); + lines.add(" return x"); + lines.add("init"); + lines.add(" caller()"); + + String compiled = compileLuaWithCUs( + "LuaTranslationTests_luaInlinerKeepsCallWhenLiveValuesWouldExceedRegisterBudget", + false, Collections.emptyList(), + new RunArgs().with("-lua", "-inline"), + lines.toArray(new String[0])); + int callerStart = compiled.indexOf("function caller("); + assertTrue("caller function not found", callerStart >= 0); + int callerEnd = compiled.indexOf("\nend", callerStart); + assertTrue("caller function end not found", callerEnd > callerStart); + String callerBody = compiled.substring(callerStart, callerEnd); + assertTrue("@inline is a strong preference, but must not force Lua register spilling:\n" + callerBody, + callerBody.contains("helper(1)")); + assertFalse("budgeted caller must stay out of the heap locals fallback:\n" + callerBody, + callerBody.contains("__wurst_locals")); + } + + @Test + public void luaInliningWithoutLocalAllocationUsesDeclarationBudget() { + List lines = new ArrayList<>(); + lines.add("package Test"); + lines.add("native takesInt(int i)"); + lines.add("@noinline function caller(int value)"); + for (int i = 0; i < 70; i++) { + lines.add(" takesInt((value + " + i + ") mod 3)"); + } + lines.add("init"); + lines.add(" caller(7)"); + + String compiled = compileLuaWithCUs( + "LuaTranslationTests_luaInliningWithoutLocalAllocationUsesDeclarationBudget", + false, Collections.emptyList(), new RunArgs().with("-lua", "-inline"), + lines.toArray(new String[0])); + int callerStart = compiled.indexOf("function caller("); + assertTrue("caller function not found", callerStart >= 0); + int callerEnd = compiled.indexOf("\nend", callerStart); + assertTrue("caller function end not found", callerEnd > callerStart); + String body = compiled.substring(callerStart, callerEnd); + assertFalse("inlining without allocation must not cross into whole-function spill mode:\n" + body, + body.contains("__wurst_locals")); + assertTrue("the exact declaration budget must retain residual helper calls near the limit:\n" + body, + body.contains("__wurst_modInt(")); + } + + @Test + public void luaInliningWithoutLocalAllocationCountsFlattenedTuples() { + List lines = new ArrayList<>(); + lines.add("package Test"); + lines.add("tuple quad(int a, int b, int c, int d)"); + lines.add("native takesInt(int i)"); + lines.add("@inline function tupleHelper(quad a, quad b, quad c, quad d) returns int"); + lines.add(" return a.a + b.a + c.a + d.a"); + String parameters = IntStream.range(0, 47) + .mapToObj(i -> "quad p" + i) + .collect(Collectors.joining(", ")); + lines.add("@noinline function caller(" + parameters + ")"); + lines.add(" takesInt(tupleHelper(p0, p1, p2, p3))"); + lines.add("init"); + lines.add(" let value = quad(1, 2, 3, 4)"); + lines.add(" caller(" + IntStream.range(0, 47) + .mapToObj(i -> "value") + .collect(Collectors.joining(", ")) + ")"); + + String compiled = compileLuaWithCUs( + "LuaTranslationTests_luaInliningWithoutLocalAllocationCountsFlattenedTuples", + false, Collections.emptyList(), new RunArgs().with("-lua", "-inline"), + lines.toArray(new String[0])); + int callerStart = compiled.indexOf("function caller("); + assertTrue("caller function not found", callerStart >= 0); + int callerEnd = compiled.indexOf("\nend", callerStart); + assertTrue("caller function end not found", callerEnd > callerStart); + String body = compiled.substring(callerStart, callerEnd); + assertFalse("a caller below Lua's hard limit must stay register-backed:\n" + body, + body.contains("__wurst_locals")); + assertTrue("tuple components must count separately when deciding whether to inline:\n" + body, + body.contains("tupleHelper(")); + } + + @Test + public void luaInlinerReusesRegistersAcrossSequentialInlineSites() { + List lines = new ArrayList<>(); + lines.add("package Test"); + lines.add("native takesInt(int i)"); + lines.add("@inline function helper(int x) returns int"); + lines.add(" let a = x + 1"); + lines.add(" let b = a + 1"); + lines.add(" let c = b + 1"); + lines.add(" return c"); + lines.add("@noinline function caller()"); + lines.add(" var sum = 0"); + for (int i = 0; i < 80; i++) { + lines.add(" sum += helper(" + i + ")"); + } + lines.add(" takesInt(sum)"); + lines.add("init"); + lines.add(" caller()"); + + String compiled = compileLuaWithCUs( + "LuaTranslationTests_luaInlinerReusesRegistersAcrossSequentialInlineSites", + false, Collections.emptyList(), + new RunArgs().with("-lua", "-inline", "-localOptimizations"), + lines.toArray(new String[0])); + String callerBody = getFunctionBody(compiled, "caller"); + assertFalse("low-pressure sequential helper calls should still inline:\n" + callerBody, + callerBody.contains("helper(")); + assertFalse("sequential inline temporaries should reuse registers:\n" + callerBody, + callerBody.contains("__wurst_locals")); + } + + @Test + public void luaInlinerBudgetsTheExpandedNestedCallee() { + List lines = new ArrayList<>(); + lines.add("package Test"); + lines.add("native takesInt(int i)"); + lines.add("@noinline function takesIntAndReturn(int x) returns int"); + lines.add(" takesInt(x)"); + lines.add(" return x"); + lines.add("@inline function leaf(int x) returns int"); + for (int i = 0; i < 20; i++) { + lines.add(" let h" + i + " = x + " + i); + } + lines.add(" return " + IntStream.range(0, 20) + .mapToObj(i -> "h" + i) + .collect(Collectors.joining(" + "))); + lines.add("@inline function middle(int x) returns int"); + lines.add(" return leaf(x)"); + lines.add("@noinline function caller()"); + for (int i = 0; i < 175; i++) { + lines.add(" let v" + i + " = takesIntAndReturn(" + i + ")"); + } + lines.add(" var sum = middle(1)"); + for (int i = 0; i < 175; i++) { + lines.add(" sum += v" + i); + } + lines.add(" takesInt(sum)"); + lines.add("init"); + lines.add(" caller()"); + + String compiled = compileLuaWithCUs( + "LuaTranslationTests_luaInlinerBudgetsTheExpandedNestedCallee", + false, Collections.emptyList(), + new RunArgs().with("-lua", "-inline"), + lines.toArray(new String[0])); + int callerStart = compiled.indexOf("function caller("); + assertTrue("caller function not found", callerStart >= 0); + int callerEnd = compiled.indexOf("\nend", callerStart); + assertTrue("caller function end not found", callerEnd > callerStart); + String callerBody = compiled.substring(callerStart, callerEnd); + assertTrue("caller must budget the already-expanded middle body:\n" + callerBody, + callerBody.contains("middle(1)")); + assertFalse("nested inline accounting must prevent a whole-function spill:\n" + callerBody, + callerBody.contains("__wurst_locals")); + } + + @Test + public void luaLocalMergerReusesNonOverlappingLocalPlayerDependentSlots() { + String compiled = compileLuaWithCUs( + "LuaTranslationTests_luaLocalMergerReusesNonOverlappingLocalPlayerDependentSlots", + false, Collections.emptyList(), + new RunArgs().with("-lua", "-localOptimizations"), + "type player extends handle", + "package Test", + "@extern native GetLocalPlayer() returns player", + "@extern native takesPlayer(player p)", + "@noinline function caller()", + " let first = GetLocalPlayer()", + " takesPlayer(first)", + " let second = GetLocalPlayer()", + " takesPlayer(second)", + "init", + " caller()" + ); + String callerBody = getFunctionBody(compiled, "caller"); + assertEquals("same-locality values with disjoint live ranges should share one Lua register:\n" + callerBody, + 1, countMatches(callerBody, "local\\s+(?:first|second)\\b")); + } + + @Test + public void luaLocalMergerKeepsTupleVarargLoopBindingsDistinct() { + List lines = new ArrayList<>(); + lines.add("package Test"); + lines.add("tuple quad(int a, int b, int c, int d)"); + lines.add("@noinline function sumEdges(vararg quad values) returns int"); + lines.add(" var result = 0"); + lines.add(" for value in values"); + lines.add(" result += value.a + value.d"); + lines.add(" return result"); + lines.add("init"); + String arguments = IntStream.range(0, 33) + .mapToObj(i -> "quad(" + i + ", 0, 0, " + (100 + i) + ")") + .collect(Collectors.joining(", ")); + lines.add(" sumEdges(" + arguments + ")"); + + String compiled = compileLuaWithCUs( + "LuaTranslationTests_luaLocalMergerKeepsTupleVarargLoopBindingsDistinct", + false, Collections.emptyList(), + new RunArgs().with("-lua", "-localOptimizations"), + lines.toArray(new String[0])); + String body = getFunctionBody(compiled, "sumEdges"); + assertEquals("simultaneously assigned tuple components must use distinct Lua locals:\n" + body, + 4, countMatches(body, "local\\s+value_[abcd]\\b")); + } + + @Test + public void luaInlinerDoesNotTreatSequentialVarargLoopTempsAsConcurrent() { + List lines = new ArrayList<>(); + lines.add("package Test"); + lines.add("native takesInt(int i)"); + lines.add("@inline function small(int x) returns int"); + lines.add(" return x + 1"); + lines.add("@noinline function process(vararg int values)"); + lines.add(" for value in values"); + for (int i = 0; i < 191; i++) { + lines.add(" let temp" + i + " = value + " + i); + lines.add(" takesInt(temp" + i + ")"); + } + lines.add(" takesInt(small(value))"); + lines.add("init"); + lines.add(" process(" + IntStream.range(0, 33) + .mapToObj(Integer::toString) + .collect(Collectors.joining(", ")) + ")"); + + String compiled = compileLuaWithCUs( + "LuaTranslationTests_luaInlinerDoesNotTreatSequentialVarargLoopTempsAsConcurrent", + false, Collections.emptyList(), + new RunArgs().with("-lua", "-inline", "-localOptimizations"), + lines.toArray(new String[0])); + assertFalse("sequential loop temporaries must not consume concurrent register budget:\n" + compiled, + compiled.contains("small(")); + } + @Test public void spilledLocalsKeepNestedBlockInitializationsInLua() throws IOException { List lines = new ArrayList<>(); diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java index fc6bb5ade..e0bd59db7 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java @@ -2,6 +2,7 @@ import com.google.common.base.Charsets; import com.google.common.io.Files; +import de.peeeq.wurstio.TimeTaker; import de.peeeq.wurstio.UtilsIO; import de.peeeq.wurstscript.RunArgs; import de.peeeq.wurstscript.ast.Ast; @@ -12,6 +13,8 @@ import de.peeeq.wurstscript.intermediatelang.optimizer.LocalPlayerContextAnalyzer; import de.peeeq.wurstscript.intermediatelang.optimizer.SideEffectAnalyzer; import de.peeeq.wurstscript.jassIm.*; +import de.peeeq.wurstscript.translation.imoptimizer.ImInliner; +import de.peeeq.wurstscript.translation.imoptimizer.ImOptimizer; import de.peeeq.wurstscript.translation.imtranslation.ImTranslator; import de.peeeq.wurstscript.translation.imtranslation.FunctionFlagEnum; import de.peeeq.wurstscript.types.TypesHelper; @@ -1541,6 +1544,340 @@ public void localMergerLiveness() throws IOException { } } + @Test + public void localMergerKeepsImplicitEntryLocalSeparateFromParameter() { + WurstModel model = Ast.WurstModel(); + ImTranslator translator = new ImTranslator(model, false, new RunArgs()); + ImProg prog = translator.getImProg(); + ImVar sinkA = JassIm.ImVar(model, TypesHelper.imInt(), "a", false); + ImVar sinkB = JassIm.ImVar(model, TypesHelper.imInt(), "b", false); + ImFunction sink = JassIm.ImFunction(model, "sink", JassIm.ImTypeVars(), + JassIm.ImVars(sinkA, sinkB), JassIm.ImVoid(), JassIm.ImVars(), JassIm.ImStmts(), + Collections.emptyList()); + ImVar parameter = JassIm.ImVar(model, TypesHelper.imInt(), "parameter", false); + ImVar implicit = JassIm.ImVar(model, TypesHelper.imInt(), "implicit", false); + ImFunctionCall call = JassIm.ImFunctionCall(model, sink, JassIm.ImTypeArguments(), + JassIm.ImExprs(JassIm.ImVarAccess(parameter), JassIm.ImVarAccess(implicit)), false, + de.peeeq.wurstscript.translation.imtranslation.CallType.NORMAL); + ImSet laterDefinition = JassIm.ImSet(model, JassIm.ImVarAccess(implicit), JassIm.ImIntVal(1)); + ImFunction caller = JassIm.ImFunction(model, "caller", JassIm.ImTypeVars(), + JassIm.ImVars(parameter), JassIm.ImVoid(), JassIm.ImVars(implicit), + JassIm.ImStmts(call, laterDefinition), Collections.emptyList()); + prog.getFunctions().add(sink); + prog.getFunctions().add(caller); + + new LocalMerger().optimize(translator, new LocalPlayerContextAnalyzer(prog)); + + ImFunctionCall optimizedCall = (ImFunctionCall) caller.getBody().get(0); + ImVar first = ((ImVarAccess) optimizedCall.getArguments().get(0)).getVar(); + ImVar second = ((ImVarAccess) optimizedCall.getArguments().get(1)).getVar(); + assertNotSame(first, second, + "function-entry values must not be assigned the same allocation slot"); + } + + @Test + public void repeatedLocalOptimizationStartsANewIteration() { + WurstModel model = Ast.WurstModel(); + ImTranslator translator = new ImTranslator(model, false, new RunArgs()); + ImFunction main = JassIm.ImFunction(model, "main", JassIm.ImTypeVars(), JassIm.ImVars(), + JassIm.ImVoid(), JassIm.ImVars(), JassIm.ImStmts(), Collections.emptyList()); + ImFunction config = JassIm.ImFunction(model, "config", JassIm.ImTypeVars(), JassIm.ImVars(), + JassIm.ImVoid(), JassIm.ImVars(), JassIm.ImStmts(), Collections.emptyList()); + translator.getImProg().getFunctions().add(main); + translator.getImProg().getFunctions().add(config); + translator.setMainFunc(main); + translator.setConfigFunc(config); + ImOptimizer optimizer = new ImOptimizer(new TimeTaker.Default(), translator); + + optimizer.localOptimizations(); + main.getLocals().add(JassIm.ImVar(model, TypesHelper.imInt(), "lateUnused", false)); + optimizer.localOptimizations(); + + assertTrue(main.getLocals().isEmpty(), + "a second local-optimization invocation must execute its passes"); + } + + @Test + public void luaArithmeticHelperRetryRespectsFunctionLocalBudget() { + WurstModel model = Ast.WurstModel(); + ImTranslator translator = new ImTranslator(model, false, + new RunArgs().with("-lua", "-localOptimizations")); + ImProg prog = translator.getImProg(); + + ImVar helperA = JassIm.ImVar(model, TypesHelper.imInt(), "a", false); + ImVar helperB = JassIm.ImVar(model, TypesHelper.imInt(), "b", false); + ImFunction helper = JassIm.ImFunction(model, "__wurst_modInt", JassIm.ImTypeVars(), + JassIm.ImVars(helperA, helperB), TypesHelper.imInt(), JassIm.ImVars(), + JassIm.ImStmts(JassIm.ImReturn(model, JassIm.ImVarAccess(helperA))), + Collections.emptyList()); + translator.luaModIntFunc = helper; + + ImVars callerParameters = JassIm.ImVars(); + for (int i = 0; i < 177; i++) { + callerParameters.add(JassIm.ImVar(model, TypesHelper.imInt(), "p" + i, false)); + } + ImVar result = JassIm.ImVar(model, TypesHelper.imInt(), "result", false); + ImVars callerLocals = JassIm.ImVars(result); + ImStmts callerBody = JassIm.ImStmts(); + for (int i = 0; i < 6; i++) { + ImVar loopVar = JassIm.ImVar(model, TypesHelper.imInt(), "loop" + i, false); + callerLocals.add(loopVar); + callerBody.add(JassIm.ImVarargLoop(model, JassIm.ImStmts(), + JassIm.ImVarargLoopVars(JassIm.ImVarargLoopVar(loopVar)))); + } + ImFunctionCall call = JassIm.ImFunctionCall(model, helper, JassIm.ImTypeArguments(), + JassIm.ImExprs(JassIm.ImIntVal(7), JassIm.ImIntVal(3)), false, + de.peeeq.wurstscript.translation.imtranslation.CallType.NORMAL); + callerBody.add(JassIm.ImSet(model, JassIm.ImVarAccess(result), call)); + ImVars sinkParameters = JassIm.ImVars(); + ImExprs sinkArguments = JassIm.ImExprs(); + for (int i = 0; i < callerParameters.size(); i++) { + sinkParameters.add(JassIm.ImVar(model, TypesHelper.imInt(), "value" + i, false)); + sinkArguments.add(JassIm.ImVarAccess(callerParameters.get(i))); + } + ImFunction sink = JassIm.ImFunction(model, "sink", JassIm.ImTypeVars(), sinkParameters, + JassIm.ImVoid(), JassIm.ImVars(), JassIm.ImStmts(), Collections.emptyList()); + callerBody.add(JassIm.ImFunctionCall(model, sink, JassIm.ImTypeArguments(), sinkArguments, + false, de.peeeq.wurstscript.translation.imtranslation.CallType.NORMAL)); + ImFunction caller = JassIm.ImFunction(model, "caller", JassIm.ImTypeVars(), callerParameters, + JassIm.ImVoid(), callerLocals, callerBody, + Collections.emptyList()); + prog.getFunctions().add(helper); + prog.getFunctions().add(sink); + prog.getFunctions().add(caller); + + assertEquals(0, new ImInliner(translator).inlineLuaDivModHelpersWithinLocalBudget()); + ImSet assignment = (ImSet) caller.getBody().get(6); + assertTrue(assignment.getRight() instanceof ImFunctionCall, + "the late retry must retain the helper when declarations exceed the safe budget"); + } + + @Test + public void luaArithmeticHelperRetryReusesSequentialSlots() { + WurstModel model = Ast.WurstModel(); + ImTranslator translator = new ImTranslator(model, false, + new RunArgs().with("-lua", "-localOptimizations")); + ImProg prog = translator.getImProg(); + ImVar helperA = JassIm.ImVar(model, TypesHelper.imInt(), "a", false); + ImVar helperB = JassIm.ImVar(model, TypesHelper.imInt(), "b", false); + ImFunction helper = JassIm.ImFunction(model, "__wurst_modInt", JassIm.ImTypeVars(), + JassIm.ImVars(helperA, helperB), TypesHelper.imInt(), JassIm.ImVars(), + JassIm.ImStmts(JassIm.ImReturn(model, JassIm.ImVarAccess(helperA))), + Collections.emptyList()); + translator.luaModIntFunc = helper; + ImVars parameters = JassIm.ImVars(); + for (int i = 0; i < 187; i++) { + parameters.add(JassIm.ImVar(model, TypesHelper.imInt(), "p" + i, false)); + } + ImVar result = JassIm.ImVar(model, TypesHelper.imInt(), "result", false); + ImFunction caller = JassIm.ImFunction(model, "caller", JassIm.ImTypeVars(), parameters, + JassIm.ImVoid(), JassIm.ImVars(result), JassIm.ImStmts( + JassIm.ImSet(model, JassIm.ImVarAccess(result), JassIm.ImFunctionCall(model, helper, + JassIm.ImTypeArguments(), JassIm.ImExprs(JassIm.ImIntVal(7), JassIm.ImIntVal(3)), + false, de.peeeq.wurstscript.translation.imtranslation.CallType.NORMAL)), + JassIm.ImSet(model, JassIm.ImVarAccess(result), JassIm.ImFunctionCall(model, helper, + JassIm.ImTypeArguments(), JassIm.ImExprs(JassIm.ImIntVal(8), JassIm.ImIntVal(3)), + false, de.peeeq.wurstscript.translation.imtranslation.CallType.NORMAL))), + Collections.emptyList()); + prog.getFunctions().add(helper); + prog.getFunctions().add(caller); + + assertEquals(new ImInliner(translator).inlineLuaDivModHelpersWithinLocalBudget(), 2, + "sequential helper sites should share the same peak allocation slots"); + } + + @Test + public void luaArithmeticHelperRetryBudgetsOverlappingArgumentResults() { + WurstModel model = Ast.WurstModel(); + ImTranslator translator = new ImTranslator(model, false, + new RunArgs().with("-lua", "-localOptimizations")); + ImProg prog = translator.getImProg(); + ImVar helperA = JassIm.ImVar(model, TypesHelper.imInt(), "a", false); + ImVar helperB = JassIm.ImVar(model, TypesHelper.imInt(), "b", false); + ImFunction helper = JassIm.ImFunction(model, "__wurst_modInt", JassIm.ImTypeVars(), + JassIm.ImVars(helperA, helperB), TypesHelper.imInt(), JassIm.ImVars(), + JassIm.ImStmts(JassIm.ImReturn(model, JassIm.ImVarAccess(helperA))), + Collections.emptyList()); + translator.luaModIntFunc = helper; + + ImVars callerParameters = JassIm.ImVars(); + for (int i = 0; i < 187; i++) { + callerParameters.add(JassIm.ImVar(model, TypesHelper.imInt(), "p" + i, false)); + } + ImVars fiveParameters = JassIm.ImVars(); + ImExprs overlappingArguments = JassIm.ImExprs(); + for (int i = 0; i < 5; i++) { + fiveParameters.add(JassIm.ImVar(model, TypesHelper.imInt(), "arg" + i, false)); + overlappingArguments.add(JassIm.ImFunctionCall(model, helper, JassIm.ImTypeArguments(), + JassIm.ImExprs(JassIm.ImVarAccess(callerParameters.get(i)), JassIm.ImIntVal(3)), + false, de.peeeq.wurstscript.translation.imtranslation.CallType.NORMAL)); + } + ImFunction takesFive = JassIm.ImFunction(model, "takesFive", JassIm.ImTypeVars(), fiveParameters, + JassIm.ImVoid(), JassIm.ImVars(), JassIm.ImStmts(), Collections.emptyList()); + ImVars keepAliveParameters = JassIm.ImVars(); + ImExprs keepAliveArguments = JassIm.ImExprs(); + for (int i = 0; i < callerParameters.size(); i++) { + keepAliveParameters.add(JassIm.ImVar(model, TypesHelper.imInt(), "value" + i, false)); + keepAliveArguments.add(JassIm.ImVarAccess(callerParameters.get(i))); + } + ImFunction keepAlive = JassIm.ImFunction(model, "keepAlive", JassIm.ImTypeVars(), + keepAliveParameters, JassIm.ImVoid(), JassIm.ImVars(), JassIm.ImStmts(), + Collections.emptyList()); + ImFunction caller = JassIm.ImFunction(model, "caller", JassIm.ImTypeVars(), callerParameters, + JassIm.ImVoid(), JassIm.ImVars(), JassIm.ImStmts( + JassIm.ImFunctionCall(model, takesFive, JassIm.ImTypeArguments(), overlappingArguments, + false, de.peeeq.wurstscript.translation.imtranslation.CallType.NORMAL), + JassIm.ImFunctionCall(model, keepAlive, JassIm.ImTypeArguments(), keepAliveArguments, + false, de.peeeq.wurstscript.translation.imtranslation.CallType.NORMAL)), + Collections.emptyList()); + prog.getFunctions().add(helper); + prog.getFunctions().add(takesFive); + prog.getFunctions().add(keepAlive); + prog.getFunctions().add(caller); + + int changed = new ImInliner(translator).inlineLuaDivModHelpersWithinLocalBudget(); + assertTrue(changed < 5, + "overlapping argument results must stop helper inlining at the register budget"); + int[] remaining = {0}; + caller.getBody().accept(new ImStmts.DefaultVisitor() { + @Override + public void visit(ImFunctionCall call) { + super.visit(call); + if (call.getFunc() == helper) { + remaining[0]++; + } + } + }); + assertTrue(remaining[0] > 0, "some overlapping helper calls must remain after the budget is reached"); + } + + @Test + public void luaArithmeticHelperRetryBudgetsEarlierImpureArguments() { + WurstModel model = Ast.WurstModel(); + ImTranslator translator = new ImTranslator(model, false, + new RunArgs().with("-lua", "-localOptimizations")); + ImProg prog = translator.getImProg(); + ImVar helperA = JassIm.ImVar(model, TypesHelper.imInt(), "a", false); + ImVar helperB = JassIm.ImVar(model, TypesHelper.imInt(), "b", false); + ImFunction helper = JassIm.ImFunction(model, "__wurst_modInt", JassIm.ImTypeVars(), + JassIm.ImVars(helperA, helperB), TypesHelper.imInt(), JassIm.ImVars(), + JassIm.ImStmts(JassIm.ImReturn(model, JassIm.ImVarAccess(helperA))), + Collections.emptyList()); + translator.luaModIntFunc = helper; + ImVar impureParameter = JassIm.ImVar(model, TypesHelper.imInt(), "value", false); + ImFunction impure = JassIm.ImFunction(model, "impure", JassIm.ImTypeVars(), + JassIm.ImVars(impureParameter), TypesHelper.imInt(), JassIm.ImVars(), JassIm.ImStmts(), + Collections.singletonList(FunctionFlagEnum.IS_NATIVE)); + + ImVars callerParameters = JassIm.ImVars(); + for (int i = 0; i < 178; i++) { + callerParameters.add(JassIm.ImVar(model, TypesHelper.imInt(), "p" + i, false)); + } + ImVars outerParameters = JassIm.ImVars(); + ImExprs outerArguments = JassIm.ImExprs(); + for (int i = 0; i < 11; i++) { + outerParameters.add(JassIm.ImVar(model, TypesHelper.imInt(), "arg" + i, false)); + outerArguments.add(JassIm.ImFunctionCall(model, impure, JassIm.ImTypeArguments(), + JassIm.ImExprs(JassIm.ImVarAccess(callerParameters.get(i))), false, + de.peeeq.wurstscript.translation.imtranslation.CallType.NORMAL)); + } + outerParameters.add(JassIm.ImVar(model, TypesHelper.imInt(), "last", false)); + outerArguments.add(JassIm.ImFunctionCall(model, helper, JassIm.ImTypeArguments(), + JassIm.ImExprs(JassIm.ImVarAccess(callerParameters.get(11)), JassIm.ImIntVal(3)), false, + de.peeeq.wurstscript.translation.imtranslation.CallType.NORMAL)); + ImFunction outer = JassIm.ImFunction(model, "outer", JassIm.ImTypeVars(), outerParameters, + JassIm.ImVoid(), JassIm.ImVars(), JassIm.ImStmts(), Collections.emptyList()); + ImVars keepAliveParameters = JassIm.ImVars(); + ImExprs keepAliveArguments = JassIm.ImExprs(); + for (int i = 0; i < callerParameters.size(); i++) { + keepAliveParameters.add(JassIm.ImVar(model, TypesHelper.imInt(), "keep" + i, false)); + keepAliveArguments.add(JassIm.ImVarAccess(callerParameters.get(i))); + } + ImFunction keepAlive = JassIm.ImFunction(model, "keepAlive", JassIm.ImTypeVars(), + keepAliveParameters, JassIm.ImVoid(), JassIm.ImVars(), JassIm.ImStmts(), + Collections.emptyList()); + ImFunction caller = JassIm.ImFunction(model, "caller", JassIm.ImTypeVars(), callerParameters, + JassIm.ImVoid(), JassIm.ImVars(), JassIm.ImStmts( + JassIm.ImFunctionCall(model, outer, JassIm.ImTypeArguments(), outerArguments, false, + de.peeeq.wurstscript.translation.imtranslation.CallType.NORMAL), + JassIm.ImFunctionCall(model, keepAlive, JassIm.ImTypeArguments(), keepAliveArguments, false, + de.peeeq.wurstscript.translation.imtranslation.CallType.NORMAL)), + Collections.emptyList()); + prog.getFunctions().add(helper); + prog.getFunctions().add(impure); + prog.getFunctions().add(outer); + prog.getFunctions().add(keepAlive); + prog.getFunctions().add(caller); + + assertEquals(0, new ImInliner(translator).inlineLuaDivModHelpersWithinLocalBudget()); + } + + @Test + public void luaArithmeticHelperRetryPreservesLocalPlayerAllocationClasses() { + WurstModel model = Ast.WurstModel(); + ImTranslator translator = new ImTranslator(model, false, + new RunArgs().with("-lua", "-localOptimizations")); + ImProg prog = translator.getImProg(); + + ImVar helperA = JassIm.ImVar(model, TypesHelper.imInt(), "a", false); + ImVar helperB = JassIm.ImVar(model, TypesHelper.imInt(), "b", false); + ImFunction helper = JassIm.ImFunction(model, "__wurst_modInt", JassIm.ImTypeVars(), + JassIm.ImVars(helperA, helperB), TypesHelper.imInt(), JassIm.ImVars(), + JassIm.ImStmts(JassIm.ImReturn(model, JassIm.ImVarAccess(helperA))), + Collections.emptyList()); + translator.luaModIntFunc = helper; + ImFunction localValue = JassIm.ImFunction(model, "GetLocationZ", JassIm.ImTypeVars(), + JassIm.ImVars(), TypesHelper.imReal(), JassIm.ImVars(), JassIm.ImStmts(), + Collections.singletonList(FunctionFlagEnum.IS_NATIVE)); + + ImVars sinkParameters = JassIm.ImVars(); + for (int i = 0; i < 99; i++) { + sinkParameters.add(JassIm.ImVar(model, TypesHelper.imReal(), "value" + i, false)); + } + ImFunction sink = JassIm.ImFunction(model, "sink", JassIm.ImTypeVars(), sinkParameters, + JassIm.ImVoid(), JassIm.ImVars(), JassIm.ImStmts(), Collections.emptyList()); + ImVars callerLocals = JassIm.ImVars(); + ImStmts callerBody = JassIm.ImStmts(); + ImExprs localArguments = JassIm.ImExprs(); + ImExprs synchronizedArguments = JassIm.ImExprs(); + for (int i = 0; i < 99; i++) { + ImVar local = JassIm.ImVar(model, TypesHelper.imReal(), "local" + i, false); + ImVar synchronizedVar = JassIm.ImVar(model, TypesHelper.imReal(), "sync" + i, false); + callerLocals.add(local); + callerLocals.add(synchronizedVar); + callerBody.add(JassIm.ImSet(model, JassIm.ImVarAccess(local), + JassIm.ImFunctionCall(model, localValue, JassIm.ImTypeArguments(), JassIm.ImExprs(), + false, de.peeeq.wurstscript.translation.imtranslation.CallType.NORMAL))); + localArguments.add(JassIm.ImVarAccess(local)); + synchronizedArguments.add(JassIm.ImVarAccess(synchronizedVar)); + } + callerBody.add(JassIm.ImFunctionCall(model, sink, JassIm.ImTypeArguments(), localArguments, + false, de.peeeq.wurstscript.translation.imtranslation.CallType.NORMAL)); + for (int i = 0; i < 99; i++) { + ImVar synchronizedVar = callerLocals.get(i * 2 + 1); + callerBody.add(JassIm.ImSet(model, JassIm.ImVarAccess(synchronizedVar), JassIm.ImRealVal("1."))); + } + callerBody.add(JassIm.ImFunctionCall(model, sink, JassIm.ImTypeArguments(), synchronizedArguments, + false, de.peeeq.wurstscript.translation.imtranslation.CallType.NORMAL)); + ImVar result = JassIm.ImVar(model, TypesHelper.imInt(), "result", false); + callerLocals.add(result); + ImFunctionCall helperCall = JassIm.ImFunctionCall(model, helper, JassIm.ImTypeArguments(), + JassIm.ImExprs(JassIm.ImIntVal(7), JassIm.ImIntVal(3)), false, + de.peeeq.wurstscript.translation.imtranslation.CallType.NORMAL); + callerBody.add(JassIm.ImSet(model, JassIm.ImVarAccess(result), helperCall)); + ImFunction caller = JassIm.ImFunction(model, "caller", JassIm.ImTypeVars(), JassIm.ImVars(), + JassIm.ImVoid(), callerLocals, callerBody, Collections.emptyList()); + prog.getFunctions().add(localValue); + prog.getFunctions().add(helper); + prog.getFunctions().add(sink); + prog.getFunctions().add(caller); + + assertEquals(0, new ImInliner(translator).inlineLuaDivModHelpersWithinLocalBudget()); + assertTrue(((ImSet) caller.getBody().get(caller.getBody().size() - 1)).getRight() + instanceof ImFunctionCall, + "local-player-dependent and synchronized allocation classes must both count toward the budget"); + } + @Test public void testFunctionSplitter() { WurstModel model = Ast.WurstModel();