Repository navigation
Conversation
…bals A function returning a tuple passed every component but the first through a global of the function on Lua too: a global write per component in the function, a global read per component after each call. Lua has multiple results, so the function now returns its scalar components together (return x, y) and a call takes them into locals of its own (a, b = f()). EliminateTuples makes the shape on Lua (LuaMultipleResults states and checks it): the function's return type is the flat tuple of its components, each return returns a tuple expression of them, and a call is a statement or the value of a results local, read only by component selections. To the optimizer a results local is an ordinary local which a call writes. The backend prints one Lua local per component, a multiple assignment for the call and a return of the values. The global of a component which no call read was garbage, value and all. The garbage removal now drops such a result from every function of its group (the implementations a call can dispatch between, and the functions whose calls share a results local), keeping what the dropped value did in its place.
let p = f() becomes p_x = t.0; p_y = t.1 after the call, and the copies stayed: copy propagation knew only a variable or a constant as a value. A component of a results local is one too, until another call writes the local, so the reads of p_x read t.0 and the copy is garbage.
Each emulated hashtable native was a call of a __wurst_ helper: a global
lookup and a call on every load and save. The backend now prints the table
accesses the helper consists of: a load is (h.T[p] or __wurst_htEmpty)[c]
with the default the helper answered, a save
(h.T[p] or __wurst_htNewChild(h.T, p))[c] = v, a remove tests the child
first, the flushes reset the subtables. An operation keeps the call of its
helper where an operand has an effect, since printed in place an operand is
read again or not at all; the helper is then defined, and only then.
LuaNativeLowering keeps the stubs in ImTranslator.luaHashtableStubs, so the
optimizer and the backend match them by identity (LuaHashtable). Loads and
HaveSaved tests count as reads (isLuaTableRead), so an unused one is dropped,
as the Jass natives already were. The helper bodies lose the tests for a
missing subtable, which InitHashtable always creates. The printer puts a ';'
before an assignment whose target starts with '(', which Lua would read as
the arguments of a call ending the previous line.
Table keys its hashtable by this castTo int. Inlined, that cast is an operand of the hashtable native, and the store operand classification counted every cast as one which may raise, so every inlined Table load, test and remove kept the call of its helper. On Lua the cast is (x or 0), and a cast from one class to another is x itself: neither raises nor changes anything, so such a cast is classified as its operand is.
The methods of an override family are grouped into dispatch slots by the key of their signature, which was the printed type. A type variable prints with an identity hash (ImPrinter), so the parameter T of State and the T of NoOpState had different keys in most runs and the same key in some, and the FSM's slot was named after State in one compilation and after NoOpState in the next (DeterministicChecks failed now and then). A type variable is now its owner, its position there and its name.
…lace HashList and the old HashMap key their hashtable by elem castTo int. On Lua that cast of a variable is ((x == 0) and zero) or (x or 0), which only reads, but the store operand classification counted it as an effect, so once the loads counted as pure and the inliner passed the cast in directly, every such load, test and store in zombie defense stayed a helper call (more than before this branch).
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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 61a93f6138
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
This branch has not been deployed
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.
Two of the medium items from the Lua output audit: tuple returns, and the emulated hashtable natives.
Tuple returns as Lua multiple results
A function returning a tuple passed every component but the first through a global of the function, on Lua as on Jass: a global write per component in the function and a global read per component after each call. Lua has multiple results, so the function now returns its components together and a call takes them into locals of its own:
EliminateTuplesmakes the shape on Lua.LuaMultipleResultsstates it and checks it after the elimination: a function returning two or more components has the flat tuple of them as its return type and returns tuple expressions of them; a call is a statement or the value of a results local, which only such calls write and only component selections read. To the optimizer a results local is an ordinary local which a call writes, so no pass needed to learn a new kind of assignment. The backend prints one Lua local per component, a multiple assignment for the call and a return of the values (two Lua AST nodes,LuaMultipleAssignmentandLuaReturnValues).LuaUnreadResults).let p = f()reads the results where they were received instead of copying them.Hashtable natives printed where they are called
LoadInteger,SaveIntegerand the rest were Lua helper functions defined by literal bodies, so every call (about 219 loads and 218 saves in castle fight) cost a global lookup and a call, and the IM inliner could not see into them. They are now backend intrinsics like the KeyedMap operations, printed as the table operations they stand for:this castTo int, howTablekeys its hashtable) counts as its operand.HaveSavedtests count as natives which only read, as they do on Jass (the__wurst_rename had hidden them from that list), so an unused load is dropped.__wurst_htEmpty(the shared child of an absent parent key, never written) and__wurst_htNewChildare main-chunk locals, declared when first used.(from the statement before it with;, which Lua otherwise joins to it as a call.Measurements
Castle fight and zombie defense built with
-lua -inline -localOptimizations, against master678bb436f:_return_)__wurst_LoadInteger((calls and definition)__wurst_SaveInteger(SaveStr/HaveSavedInteger/FlushChildHashtablehelper callsSETTABUP)GETTABUP)The static count grows because an operation printed in place is a few instructions more than a call; what runs is less. Stock Lua 5.3, loop overhead included: a load 73 → 47 ns, a save 78 → 30 ns, a call with a tuple of 2 results 68 → 41 ns, of 3 results 95 → 53 ns. Castle fight builds byte-identically twice and both scripts pass
luac -p. Not run in game.Checks
LuaMultipleResultsTests(13) andLuaHashtableTests(9): output shape and run-time behaviour on the bundled Lua (dispatch, closures, nested tuples, recursion, discarded calls, a dropped result with an effect, an override group, the locals-table spill, a compile-time hashtable, operand evaluation order of the call form). Each was seen failing with its change undone.DeterministicChecks.dispatchSignatureKeysDoNotDependOnIdentityHashes: fails on the old key.fsmSiblingsInSeparatePackagesBindRootSlotInAnyUnitOrderfailed now and then on this branch (1 of 5 class runs; the two type variables printed the same identity hashT412in the failing compilation); with the structural key the class passed 5 of 5 and the slot-binding classes (865 tests) pass.Also in here
LuaDispatchPreparation): methods of an override family are grouped into slots by the key of their signature, which was the printed type, and a type variable prints with an identity hash. The parameterTofStateand theTofNoOpStatetherefore had different keys in most compilations and the same key in some, and the slot was named after either class. A type variable is now its owner, its position and its name. This branch did not cause it, but shifted the identity hashes enough to make it show.this castTo int) and an old-generics value of a variable cast to int count as the reads they print as, soTable,HashListand the oldHashMapget the in-place forms.Known gaps
a, b = f(); return a, b) rather thanreturn f().StringHash(c), a call, so those saves keep the helper call.