-
Notifications
You must be signed in to change notification settings - Fork 70
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
support reg_byte
registers
#630
Conversation
@@ -611,114 +611,126 @@ fn estimate_template_length( | |||
} | |||
|
|||
/// Converts a register class to a GCC constraint code. | |||
fn reg_to_gcc(reg: InlineAsmRegOrRegClass) -> ConstraintOrRegister { | |||
let constraint = match reg { | |||
// For vector registers LLVM wants the register name to match the type size. |
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 comment is lost right now, I'm not sure what it was supposed to say, and why what llvm does is relevant?
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.
Oh, that was copy-pasted from the LLVM codegen, but I forgot to change that to GCC.
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.
ok, but what does this comment mean? What type is talked about here? it just doesn't seem like the right place for this comment?
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.
It comes from here originally.
Maybe the type is the layout
parameter that we don't have in cg_gcc.
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.
yeah I think that's right, e.g.
match layout.size.bytes() {
64 => 'z',
32 => 'y',
_ => 'x',
this really does pick the register width based on a type. Do you think GCC will have to do the same? So far that does not seem necessary.
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.
I do not know. I would need to check if needed.
57fd394
to
53aa977
Compare
Is this ready to be merged? |
yes |
Thanks for your contribution! |
fixes #485
I believe (see the issue) that the
b
suffix can just be trimmed, without causing invalid behavior.I also refactored a bit to reduce the level of nesting, given that we probably need much more logic here to support more register classes that don't translate 1-to-1 between llvm and gcc. Hopefully the rustc assembly tests will guide us towards the edge cases.