-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Make the Tail call conv follow the system call conv for the return area ptr #9287
Conversation
5efc511
to
7459dfb
Compare
…ea ptr And for Winch make it follow the system call conv on all targets except aarch64 where the first regular argument register is used instead of the designated register.
7459dfb
to
439fa7c
Compare
Subscribe to Label Action
This issue or pull request has been labeled: "cranelift", "cranelift:area:aarch64", "cranelift:area:x64", "winch"
Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! LGTM, although I have some minor nitpicks. If you feel strongly that some of them are unnecessary, I can be convinced otherwise.
next_x_reg += 1; | ||
} | ||
Some(ABIArg::reg( | ||
x_reg(x_start).to_real_reg().unwrap(), | ||
I64, | ||
ir::ArgumentExtension::None, | ||
ir::ArgumentPurpose::Normal, | ||
)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feel similarly here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This really needs to be the first argument register. If next_x_reg - 1 is not x_start at this point, that is a bug.
47fcf3e
to
7e163b2
Compare
No description provided.