Stop the local-player barrier from refusing inlining on control taint - #1284
Merged
Conversation
Four measured costs in the emitted Lua for the standard library's spatial index: coercion wrappers on every typed array read, a local-player inlining barrier that refuses most of the call graph, vararg calls that allocate a table per call, and integer div/mod as helper chains. The spec gives each a root cause, the exact change, the assertions that encode the old behaviour, and acceptance criteria. AGENTS.md states the policy the spec implements: emitted constructs are consumed by Wurst code, so nothing defends against foreign writes, and Lua-native mechanisms replace emulation.
The analysis marks a function's return fact whenever the function is reachable from a client-local branch, transitively over the call graph, and the inliner treated that fact as a reason not to inline. In a program which links the standard library that is most of the call graph: on the spatial index probe 577 of 1678 call sites were refused, among them max, min and the pure index arithmetic in the query loops. Control taint is the wrong question for inlining. Substituting a body at a call site runs it under exactly the control the call already had, so nothing crosses a client-local boundary; the passes which do move code run after inlining and re-analyse the inlined program. What must stay a call is a function whose result is client-local by its own body: it calls a client-local native directly, or returns a value derived from one. That second half needs a data graph without two kinds of edge. Control edges are the ones above. Call-site argument-to-parameter edges have to go too: the analysis is context-insensitive, so one max(...) call anywhere with a client-local argument would taint max and everything computed from its result. Both edge kinds stay in the full graph every other consumer reads. Measured on the same probe: 10 refusals on 5 functions, each of which reaches a client-local native.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
This was referenced Sep 3, 2026
Merged
Frotty
added a commit
that referenced
this pull request
Sep 4, 2026
* Stop the local-player barrier from refusing inlining on control taint (#1284) * 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 * Give vararg calls a fixed-arity copy on Lua (#1286) * Inline small Lua helpers regardless of popularity (#1289) * Deduplicate Lua callback adapters (#1290) * Deduplicate Lua callback adapters * Preserve renamed Lua callback targets * Bound Lua inlining by register pressure (#1291)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
ImInlinerrefused to inline any function whose return factLocalPlayerContextAnalyzerhad marked. That fact fires for every function reachable from a client-local branch, transitively over the call graph. In a map that links the standard library this is most of the call graph.Measured with
-Dwurst.inliner.log=trueon a stdlib-linked probe (release flags):local_player_context_barrierBefore:
max,min,headSlot,groupSlot,cellAt,cellCoordX,unit_getX,__wurst_intDivand other pure leaves stayed as calls inside the spatial index's query loops. After: the five remaining refusals areinit_Player,PingMinimapForPlayer,GetPlayableMapRect,GetCurrentCameraBoundsMapRectBJandInitMapRects, each of which reaches a client-local native.Why control taint is the wrong question for inlining
Substituting a callee body at a call site runs it under exactly the control the call already had, so nothing crosses a client-local boundary. The passes that do move code (
BranchMerger,TempMerger,LocalMerger,ConstantAndCopyPropagation) run after inlining and re-analyse the inlined program, where that control is explicit. They keep reading the full graph and are unchanged.What must remain a call is a function whose result is client-local by its own body: it calls a client-local native directly, or returns a value derived from one.
How
LocalPlayerContextAnalyzerkeeps a second dependency map,dataDependents, that receives every ordinary edge but not:addEnclosingControlDependency), for the reason above;addCallArgumentDependency). The analysis is context-insensitive: a parameter fact merges the arguments of every call site, so onemax(...)call anywhere with a client-local argument would taintmaxand everything computed from its result.propagateDataFacts()walks that graph from the same sources and publishes only RETURN facts intolocalPlayerDataDependentReturns.functionInliningIsLocalPlayerSensitiveis nowisClientLocalValueSource || functionsDirectlyUsingLocalPlayer || localPlayerDataDependentReturns.Two simpler rules were tried and rejected, recorded in
LUA_HOT_PATH_SPEC.md: USE facts only (breakstestInlineAnnotation, because with the stdlib linkedprintreachesGetLocalPlayer), and data-only returns with the ordinary parameter edges (still barriersmax,min,headSlot).Tests
OptimizerTests.pureHelperReachableFromLocalPlayerBranchIsStillInlined(Jass_inl.j): a pure helper called once under aGetLocalPlayerbranch inlines at both sites; a wrapper that callsGetLocalPlayerstays a call.LuaBackendAuditTests.pureHelpersReachableFromLocalPlayerBranchInlineIntoLuaHotLoops(release Lua): index arithmetic inlines into a query loop; theGetLocalPlayerwrapper stays a call.functionUsingGetLocalPlayerMustNotBeInlined,testInlineAnnotation, alllocalPlayer*tests.OptimizerTests,LuaBackendAuditTests,LuaTranslationTests,InterpreterTests,LuaTypecastingTests): 327 tests, 0 failures.SmallCheckViaJUnitCoreTestNG, which passes when run alone. The fuzz programs all write the same fixedtest-output/CompilerFuzzTestsSC_assertCompilesForBothBackends_*.jfilename, and the reported error was a truncatedfunctionkeyword: a torn file from concurrent writers, pre-existing and unrelated.Also in this PR
LUA_HOT_PATH_SPEC.mdand an AGENTS.md §7 "Lua performance policy" section, committed separately. The spec covers this change (Task 2) and the remaining emission costs found in the same investigation: array-read coercion wrappers, varargtable.pack, div/mod helper chains, and the inliner rating formula refusing tiny popular helpers.