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

fix clobbered or lateout registers overlapping with input registers #628

Merged
merged 1 commit into from
Feb 13, 2025

Conversation

folkertdev
Copy link
Contributor

Apparently, in GCC, input registers cannot also be clobbered.

this previously errored https://godbolt.org/

this fixes half of tests/ui/asm/x86_64/multiple-clobber-abi.rs, but it also runs into #451

@folkertdev folkertdev force-pushed the clobber-input-registers branch from d578ce9 to 8b2835c Compare February 12, 2025 22:04
Copy link
Contributor

@antoyo antoyo left a comment

Choose a reason for hiding this comment

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

Looks good to me.
There are possibly two things to change.

@antoyo
Copy link
Contributor

antoyo commented Feb 13, 2025

this fixes half of tests/ui/asm/x86_64/multiple-clobber-abi.rs, but it also runs into #451

I implemented the ABIs used in this test in #629, so rebasing should allow this test to pass.

@folkertdev folkertdev force-pushed the clobber-input-registers branch from 8b2835c to 7f85f6b Compare February 13, 2025 09:55
@folkertdev
Copy link
Contributor Author

folkertdev commented Feb 13, 2025

locally I can't quite get that to work, but maybe something else needs to be updated?

---- [ui] tests/ui/asm/x86_64/multiple-clobber-abi.rs stdout ----
Saved the actual stderr to "/home/folkertdev/rust/rustc_codegen_gcc/build/rust/build/x86_64-unknown-linux-gnu/test/ui/asm/x86_64/multiple-clobber-abi/multiple-clobber-abi.stderr"
normalized stderr:
gcc_jit_function_add_attribute: attribute should be a `gcc_jit_fn_attribute` enum value
gcc_jit_function_add_attribute: attribute should be a `gcc_jit_fn_attribute` enum value

I did remove the line to see whether CI maybe did update correctly already.

edit: it likely just needs the master feature?

@antoyo
Copy link
Contributor

antoyo commented Feb 13, 2025

Oh, I forgot to update this file with the latest commit.
Do you use the default config.toml which downloads libgccjit.so?

@folkertdev
Copy link
Contributor Author

I think so yes

@antoyo
Copy link
Contributor

antoyo commented Feb 13, 2025

Could you please update the file with the latest commit from the gcc repo and see if this works?
If so, please add this change to this PR.

@folkertdev folkertdev force-pushed the clobber-input-registers branch from 7f85f6b to 3ecc012 Compare February 13, 2025 13:59
@folkertdev
Copy link
Contributor Author

Could you please update the file with the latest commit from the gcc repo and see if this works? If so, please add this change to this PR.

yes that does work, I've added it now.

@antoyo
Copy link
Contributor

antoyo commented Feb 13, 2025

Is there any other change you would like to make or is this ready to merge?

@folkertdev
Copy link
Contributor Author

no I think we're good

@antoyo antoyo merged commit 1f98329 into rust-lang:master Feb 13, 2025
37 checks passed
@antoyo
Copy link
Contributor

antoyo commented Feb 13, 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.

3 participants