Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wazevo: fuzz, fix regalloc ensure phi spills are consistent across siblings. #1845

Closed
wants to merge 6 commits into from

Conversation

evacchi
Copy link
Contributor

@evacchi evacchi commented Nov 23, 2023

Fuzz case. Consider:

graph TD
    blk1 --> blk3
    blk2 --> blk3
Loading

where blk1 spills a given vReg, say v183 like in this case, and blk2 does not. Then if blk3 allocates its registers starting from blk1, then it will expect v183 to be on the stack.

We need to ensure that blk2 correctly spills v183 even if locally it wouldn't be necessary, so that it can be unspilled by blk3 regardless if the flow originates in blk1 or blk2.

This is an attempt to fix this, and it works in the given fuzz case without breaking any other tests, but it could be horribly misguided.

Feel free to close the PR if the solution is horrifying. I suspect I am missing some bits.

Signed-off-by: Edoardo Vacchi <[email protected]>
Signed-off-by: Edoardo Vacchi <[email protected]>
Signed-off-by: Edoardo Vacchi <[email protected]>
Signed-off-by: Edoardo Vacchi <[email protected]>
Signed-off-by: Edoardo Vacchi <[email protected]>
@evacchi evacchi changed the title wazevo: fuzz, fix regalloc when phis are spilled in only one predecessor. wazevo: fuzz, fix regalloc ensure phi spills are consistent across siblings. Nov 23, 2023
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh I guess this was committed by mistake

@@ -719,7 +720,7 @@ func Test1825(t *testing.T) {
})
}

// Test1825 tests that lowerFcopysignImpl allocates correctly the temporary registers.
// Test1826 tests that lowerFcopysignImpl allocates correctly the temporary registers.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was incorrect

@@ -734,3 +735,20 @@ func Test1826(t *testing.T) {
require.Equal(t, uint64(0), m.Globals[0].ValHi)
})
}

// Test1845 tests regalloc ensures that when one predecessor Y1 for a block X
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Test1845 tests regalloc ensures that when one predecessor Y1 for a block X
// Test1845 tests regalloc and ensures that when one predecessor Y1 for a block X

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants