Match psdisp (WIP) - #3250
Draft
MarkMcCaskey wants to merge 53 commits into
Draft
Conversation
From the open PR doldecomp#3223: an operand-order temp in psDispSub's axis.y, the appsrt mmtx argument spelled in place, an explicit transformed_x carrier and prev_pos.y operand swap in psDispSubAPPSRTPoint, and the psDispParticles declaration set renamed and reordered. 98.6764% -> 98.7148%. Co-authored-by: Ford Lascari <ford.lascari@gmail.com>
psUpdateProjectionCache's non-perspective branch reads prj[1..4] at each use instead of through staged locals, which recovers the target's load order, and the perspective branch folds the x_offset product into the store expression. 98.7148% -> 98.7341%.
psDispSub's axis.y stages the subtrahend so the cross product emits fmsubs instead of fnmsubs, and psDispSubAPPSRTPoint's cur_pos evaluates its x, y and z column products in that order. Four real instruction-stream rows recovered; the nominal score moves -0.02% because both changes trade instruction rows for register/displacement rows.
Both prev_pos blocks declare dy before dx, which recovers the target's velocity load order in psDispSubAPPSRTPoint and one scheduling row in psDispSubAppSRT. Real instruction-stream divergence is now 23 rows (was 34); the nominal score falls to 98.6314% because the recovered rows trade for insert/delete alignment pairs that objdiff weights more heavily than the argument mismatches they replace.
Moving chan_mat_color after tlut_obj puts tlut_obj at the target's 0x788 with the same four-byte hole below it, recovering six frame rows. 827 rows, 456 of them stack displacements (was 833/462).
cur_pos.x takes its own statement and the comma expression carries y into z, which recovers the target's ssx/x74/x84 load order. Two more real instruction-stream rows; 46 remain.
The depth-3 cap left an extra by-value staging slot at the bottom of the frame (our band started at 0x1c, the target's at 0x18) and doubled the inline-site pool below gx_tlut_obj. Raising the cap recovers both; PAD_STACK grows to 0x3c to hold the frame at 0x9a0 while the inline-local objects those bytes stand for are still unreconstructed. Identical instruction stream and register allocation, 12 fewer frame-displacement rows: 815 rows, 444 of them stack.
Arrays allocate in reverse declaration order, so cbuf first puts it above vbuf as the target has it. 808 rows, 437 stack.
The allocator capture shows the frame is the homed-object list in reverse creation order, and that this helper's temps are created last in ours but early in the target. Splicing its body into psDispSub -- its single call site, since Melee's AppSRT path does not share it -- lowers those objects in psDispSub's own order and shrinks the unaccounted inline pool from 88 bytes to 40, so PAD_STACK drops to 0xC. 98.6389% -> 98.7612% (above master's 98.6764%), 808 -> 770 rows, and register allocation rows fall 325 -> 280.
With psDispSubMakePolygon spliced into psDispSub the expansion no longer nests past depth 3, so the pragma returns to its original value, and the trail buffers return to declaration order. Both are byte-identical now.
With psDispSubMakePolygon expanded the register pressure that previously punished this ordering is gone, so psDispSubAppSRT's cur_pos rows evaluate in the target's order and psDispSubAPPSRTPoint carries its z column instead of its x. Six real instruction-stream rows recovered (46 -> 40); 768 rows, 98.7802%.
The appsrt refresh was duplicated verbatim in both AppSRT display paths. They differ only in where frameNum is stamped: the point path stamps unconditionally under its own NULL guard, the polygon path only inside the changed-frame block. A constant int parameter folds away at both call sites, so one helper emits both shapes -- byte-identical output, 28 fewer lines.
The axis-triple colouring rotated once the MakePolygon expansion changed the register pressure: this swap regressed in earlier rounds and now recovers 25 register rows. 743 rows, 98.8272%.
A systematic sweep of every adjacent same-type declaration pair found three levers the hand-reasoned search had missed: both vertex-emission colour runs want r, b, g rather than b, g, r, and the projection cache wants y_scale ahead of w3. Together: 743 -> 737 rows, real instruction-stream divergence 40 -> 36, register rows 252 -> 246.
Both call sites pass true or false, and MWCC emits byte-identical output for the narrower semantic type.
Let both AppSRT callers own the refresh scratch matrix, and reuse the polygon path's draw matrix after refresh. This recovers the target's four-matrix ownership and dispatch order without duplicating refresh logic. The frame-neutral build remains at 98.842354% while exposing the remaining scalar-home spacing problem.
Collaborator
|
Regress away. |
psDispParticles builds a 0x998 frame where the DOL has 0x9a0: the retail compile spills two doubles in psDispSub's perspective block (0x888 and 0x890) and this source spills only one. A four-byte reserve rounds the frame back to 0x9a0, which realigns the constant pool and the whole callee-saved save area. Recovers 13 diff rows (693 -> 680); the reserve goes away once the second spill is induced for real.
Two independent statement-order recoveries found by sweeping every adjacent pair of data-independent assignments in the file: - both trail loops compute `alpha` before `converted_alpha`, which places four stack temps on their retail slots (8 rows); - psDispSub's non-billboard branch assigns each `up_*` before its `right_*`, matching the branch's register order (3 rows). 693 -> 669 rows with the frame reserve; no instruction-stream rows either way.
The two reused `right`/`up` locals collapse the helper into two webs. The retail code births six, and because this helper is lowered before psDispSub, its web count sets the virtual-register numbering for everything downstream: splitting them rotates psDispSub's nine callee-saved float registers onto their retail assignment. 669 -> 661 rows, 279 -> 271 register rows, no instruction-stream rows. The same eight rows are reachable by swapping the reused pair instead, but that loads `inv_view[i][1]` before `inv_view[i][0]` and costs two real rows.
Hill-climbing every adjacent swap in the function-scope declaration list moves six objects onto retail slots. This band really is numbered by declaration order, unlike the inlined helpers' locals, and no previous sweep had touched it. 661 -> 642 rows, 382 -> 363 stack rows, no instruction-stream rows. The `spNNNN` placeholders are named for offsets they no longer all sit on; they want renaming once the band stops moving.
Adjacent swaps had converged at 642. Sweeping full permutations of every sliding window of three declarations reaches orders pairwise swaps cannot, and hill-climbing that to convergence takes another fifteen rows off the frame: 642 -> 634 -> 629 -> 627 (stack 363 -> 348) No register or instruction-stream rows either way. As before, the `spNNNN` placeholders are named for offsets they no longer sit on.
The helper took fourteen arguments, three of which it never read — the body uses pp->vel.x/y/z directly. Passing three arguments nobody looks at is not something the original could plausibly have done; it is an artifact of reconstructing the call from the register-loaded arguments. Removing them is byte-identical (627 rows either way). Reading them instead of pp->vel is not: that costs 136 rows, which confirms the body really does go through the particle.
psDispSub's projected-position block gains the fourth accumulator its sibling in psDispSubAppSRT already has, and names the repeated pvmtx[1][3] load. psDispSubMakePolygon only rescales the trail axis when there is a primitive to draw, which keeps the axis in one web across both emission loops instead of splitting it at the multiply.
Row-neutral, but it takes the frame ordering nine objects closer to the DOL's (out-of-rank 31 -> 22, displacement 118 -> 90), and the surrounding previous-position scratch already lives at function scope despite only being used on the trail path.
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
2 participants
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.
This is going to be a long one. We'll need to regress a lot to get where we need to be