Skip to content

Check if clobber_abi is correctly supported #77

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

Open
antoyo opened this issue Sep 4, 2021 · 5 comments
Open

Check if clobber_abi is correctly supported #77

antoyo opened this issue Sep 4, 2021 · 5 comments
Labels
good first issue Good for newcomers

Comments

@antoyo
Copy link
Contributor

antoyo commented Sep 4, 2021

rust-lang/rust#87581

After #72 is merged.
Needs updating to a more recent nightly.

@antoyo antoyo added the good first issue Good for newcomers label Sep 4, 2021
@ghost
Copy link

ghost commented Sep 7, 2021

I'm not sure what kind of check you want here. This piece makes me think that the whole handling of clobber_abi is done by the rustc frontend and the GCC backend is not even aware of it - all the registers are simpley added to the outputs list as out("reg") _.

LLVM backend haven't been updated as well.

@antoyo
Copy link
Contributor Author

antoyo commented Sep 7, 2021

Yup, I know. But I still want to check that it works. I think there's a test in Rust that checks this, so we might not have much to do.

@ghost
Copy link

ghost commented Sep 9, 2021

The test is src/test/codegen/asm-clobber_abi.rs. I'll try to update the backend to the latest nightly tomorrow and see if it passes.

@antoyo
Copy link
Contributor Author

antoyo commented Sep 9, 2021

You'll have to enable those codegen tests as well.

@folkertdev
Copy link
Contributor

The clobbering logic is incorrect when input and clobbered registers overlap.

The clobber_abi feature is implemented by adding additional lateout("reg") operands for the registers that a specific abi clobbers. We can replicate the issue like so https://godbolt.org/z/bsTEfGT3Y

    let mut x = 8;
    unsafe {
        std::arch::asm!(
            "mov rax, rdi",
            in("rdi") x,
            lateout("rdi") _,
        );
    }

or equivalently

    let mut x = 8;
    unsafe {
        std::arch::asm!(
            "mov rax, rdi",
            in("rdi") x,
            clobber_abi("sysv64"),
        );
    }

libgccjit.so: error: : 'asm' specifier for variable 'input_register' conflicts with 'asm' clobber list

and it's right, in that the di register is clobbered because of the sysv64 calling convention

[src/asm.rs:298:9] &clobbers = [
    "cx",
    "dx",
    "si",
    "di",
    "r8",
    "r9",
    "r10",
    "r11",
    "xmm0",
    "xmm1",
    "xmm2",
    "xmm3",
    "xmm4",
    "xmm5",
    "xmm6",
    "xmm7",
    "xmm8",
    "xmm9",
    "xmm10",
    "xmm11",
    "xmm12",
    "xmm13",
    "xmm14",
    "xmm15",
]

This seems to be a GCC rule: input and clobber registers cannot overlap.

I believe the right idea here is to treat an in("reg") that is also clobbered (i.e. in the clobbers vec) as an inlateout("reg"), meaning that it is put both into the outputs vector (with late: true and out_place: None) and the inputs vector like normal. This works for me locally for the examples I've tried so far.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants