Repository navigation
Keep the stacks aligned on their top in PSStackToTree - #22417
Open
zerotohero3 wants to merge 1 commit into
Open
zerotohero3 wants to merge 1 commit into
zerotohero3 wants to merge 1 commit into
Conversation
calixteman
requested changes
Nov 2, 2026
calixteman
left a comment
calixteman
left a comment
Contributor
There was a problem hiding this comment.
Overall it LGTM, but your LLM missed a caller in src/core/internal_viewer_utils.js.
When the two paths of a stack-growing `if`, or the two branches of an `ifelse`, end with different stack depths, the shorter stack was padded with zeros on its top. The following operators then used these zeros instead of the actual values: for example the tint transform in issue5470.pdf is only correct when its guard fires, i.e. for a tint of 1, and any other tint is rendered in a darker red instead of red. Pad the bottom of the shorter stack instead. And when a function leaves more values on the stack than it has outputs, use the ones at the top, as the interpreter does, instead of the ones at the bottom.
zerotohero3
force-pushed
the
ps-type4-stack-alignment
branch
from
November 2, 2026 16:51
91e2135 to
0aaac44
Compare
Author
|
Good catch, thanks. |
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
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.
Fixes #22415
When the two paths of a stack-growing
if, or the two branches of anifelse, end with different stack depths, the shorter stack was padded with zeros on its top, so the following operators used these zeros instead of the actual values. The shorter stack is now padded at its bottom.And when a function leaves more values on the stack than it has outputs, the Wasm/JS compilers now use the ones at the top, as the interpreter does.
The first row of the new reference test uses the tint transform from
issue5470.pdf, which is currently only right for a tint of 1.