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

allow to spill to mmx #692

Merged
merged 2 commits into from
Jan 22, 2024
Merged

allow to spill to mmx #692

merged 2 commits into from
Jan 22, 2024

Conversation

bgregoir
Copy link
Contributor

The pull request may look more complex that what it should be but it is mostly to remove stupid cycle dependency.

@eponier
Copy link
Contributor

eponier commented Jan 19, 2024

Maybe you could have split the two changes to make this fact clear, but now that's done.

@vbgl
Copy link
Member

vbgl commented Jan 19, 2024

What happens when you target an architecture that does not have an “extra” register bank? Do you want to be able to write portable code (spill to mmx if the mmx set is not void, to usual registers otherwise)? or just crash (too late)?

@bgregoir
Copy link
Contributor Author

It crash, as if you declare a #mmx in arm, certainly too late, sometime generating an uncaught exception.
Do you think the fix should be done in the commit?

@bgregoir
Copy link
Contributor Author

I have added a last commit, that fix the problem of error with mmx for arm.
One other thing to discus is, there is 2 possible ways to indicate where the variable should be spill:
1/ At the declaration of the variable (the current implemenantion)
2/ When we spill, by having a spill_operator to mmx and on to stack.
The second possibility is more flexible but also more cumbersome to use for the user.
It is also a little bit more complex for the implementation.

@vbgl vbgl merged commit f071a81 into main Jan 22, 2024
1 check passed
@vbgl vbgl deleted the spilling_mmx branch January 22, 2024 16:05
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.

3 participants