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

asm! can't let you use lateout("rbx") (LLVM needs it) #90083

Open
noncombatant opened this issue Oct 20, 2021 · 5 comments
Open

asm! can't let you use lateout("rbx") (LLVM needs it) #90083

noncombatant opened this issue Oct 20, 2021 · 5 comments
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools A-inline-assembly Area: Inline assembly (`asm!(…)`) C-bug Category: This is a bug. F-asm `#![feature(asm)]` (not `llvm_asm`) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@noncombatant
Copy link
Contributor

This is about an unstable feature, so maybe you don't consider that a regression, exactly. This seemed like the best bug template to me; I hope it's OK.

I'm reading the asm! document at https://doc.rust-lang.org/beta/unstable-book/library-features/asm.html. The cpuid example has this code:

#![allow(unused)]
#![feature(asm)]
fn main() {
let ebx: u32;
let ecx: u32;

unsafe {
    asm!(
        "cpuid",
        // EAX 4 selects the "Deterministic Cache Parameters" CPUID leaf
        inout("eax") 4 => _,
        // ECX 0 selects the L0 cache information.
        inout("ecx") 0 => ecx,
        lateout("ebx") ebx,
        lateout("edx") _,
    );
}

println!(
    "L1 Cache: {}",
    ((ebx >> 22) + 1) * (((ebx >> 12) & 0x3ff) + 1) * ((ebx & 0xfff) + 1) * (ecx + 1)
);
}

But when you press the Play button to try it out, you get:

   Compiling playground v0.0.1 (/playground)
error: invalid register `ebx`: rbx is used internally by LLVM and cannot be used as an operand for inline asm
  --> src/main.rs:15:9
   |
15 |         lateout("ebx") ebx,
   |         ^^^^^^^^^^^^^^^^^^

error: could not compile `playground` due to previous error

I hacked it up locally (building with Rust Nightly), and this seems to work:

    unsafe {
        asm!(
            "cpuid",
            // EAX 4 selects the "Deterministic Cache Parameters" CPUID leaf
            inout("eax") 4 => _,
            // ECX 0 selects the L0 cache information.
            inout("ecx") 0 => ecx,
            //lateout("ebx") ebx,
            lateout("edx") _,
        );
        asm!(
                "mov {:e}, ebx", out(reg) ebx
        );
    }

I.e. I comment out the lateout("ebx") line, and add a second macro invocation to get the value of ebx.

But I assume there is a better way. :) The docs should be updated to show the better way. Thanks!

@noncombatant noncombatant added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels Oct 20, 2021
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Oct 20, 2021
@JohnTitor
Copy link
Member

It's allow_fail at first (3233565) so not a regression, we should just update the doc.

@JohnTitor JohnTitor added A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools A-inline-assembly Area: Inline assembly (`asm!(…)`) F-asm `#![feature(asm)]` (not `llvm_asm`) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. regression-untriaged Untriaged performance or correctness regression. labels Oct 20, 2021
@asquared31415
Copy link
Contributor

Isn't it still incorrect to call cpuid then? If it does clobber ebx then we're also just not telling llvm that we're breaking it. I think the correct code saves and restores ebx around the cpuid code, and doesn't use explicit register operands, instead getting generic out(reg) and moving to those registers from the cpuid output registers.

@asquared31415
Copy link
Contributor

Here is a working example of code that does not clobber rbx by saving it.

Additionally, I opted to use CPUID 0 because 4 is not supported on AMD processors whereas 0 should be universal to all x86 processors that implement CPUID. It's reasonable that users could copy this code, and it should report a more reasonable value for more users.

@asquared31415
Copy link
Contributor

I could make the pr to correct this if nobody has any issues with that change

@JohnTitor
Copy link
Member

@asquared31415 Feel free to go ahead :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools A-inline-assembly Area: Inline assembly (`asm!(…)`) C-bug Category: This is a bug. F-asm `#![feature(asm)]` (not `llvm_asm`) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants