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

support reg_byte registers #630

Merged
merged 1 commit into from
Feb 14, 2025
Merged

Conversation

folkertdev
Copy link
Contributor

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.

@@ -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.
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 comment is lost right now, I'm not sure what it was supposed to say, and why what llvm does is relevant?

Copy link
Contributor

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.

Copy link
Contributor Author

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?

Copy link
Contributor

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.

Copy link
Contributor Author

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.

Copy link
Contributor

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.

@antoyo
Copy link
Contributor

antoyo commented Feb 14, 2025

Is this ready to be merged?

@folkertdev
Copy link
Contributor Author

yes

@antoyo antoyo merged commit fcac229 into rust-lang:master Feb 14, 2025
37 checks passed
@antoyo
Copy link
Contributor

antoyo commented Feb 14, 2025

Thanks for your contribution!

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.

x86_64 asm: Build error when using r[8-15]b
2 participants