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

RISC-V inline assembly ignores input register constraints #60391

Closed
fintelia opened this issue Apr 30, 2019 · 4 comments
Closed

RISC-V inline assembly ignores input register constraints #60391

fintelia opened this issue Apr 30, 2019 · 4 comments
Labels
A-inline-assembly Area: Inline assembly (`asm!(…)`) A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. O-riscv Target: RISC-V architecture requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@fintelia
Copy link
Contributor

This code:

#![no_std]
#![feature(asm)]
#![feature(start)]
#![feature(lang_items)]

#[lang = "eh_personality"] extern fn eh_personality() {}
#[panic_handler] fn panic(info: &::core::panic::PanicInfo) -> ! { loop {} }
#[start] fn start(_argc: isize, _argv: *const *const u8) -> isize {0}
#[no_mangle] pub fn abort() -> ! { loop {} }

#[no_mangle]
pub fn _start(a: [u32; 6]) {
    unsafe {
        asm!(""
             :: "r{sp}"(a[4]), "r{t0}"(a[5])
             : "volatile"
        );
    }
}

When compiled for the riscv64imac target:

$ cargo rustc --release --target riscv64imac-unknown-none-elf

Ignores the input register constraints which say to use sp and t0 (defaulting to a0 and a1 instead):

0000000000011000 <_start>:
   11000:	01456583          	lwu	a1,20(a0)
   11004:	01056503          	lwu	a0,16(a0)
   11008:	8082                	ret
@Centril Centril added A-inline-assembly Area: Inline assembly (`asm!(…)`) O-riscv Target: RISC-V architecture labels Apr 30, 2019
@jonas-schievink jonas-schievink added C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 30, 2019
@nagisa nagisa added the A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. label Apr 30, 2019
@lenary
Copy link
Contributor

lenary commented Aug 7, 2019

I have looked deeper into this and the issue lies both with your inline asm call, and with the LLVM backend.

On the one hand, as documented in https://llvm.org/docs/LangRef.html#constraint-codes (which the rust asm macro is a thin shim on top of), r and {<reg>} are mutually exclusive choices. Why LLVM doesn't issue a warning if you write a constraint r{<reg>}, I don't know. I am looking to prepare a patch on this.

However, this bug also shows the same underlying issue as #60392. If you specify {sp} or {t0}, LLVM will warn you that it couldn't allocate input reg for constraint '{sp}' (this warning is prefixed with error:, but that doesn't stop llvm writing out assembly for the function with no trace of the inline assembly). Again, if you specify the architectural register name, not the ABI name (so, x2 rather than sp), you get the behaviour you are looking for. It is likely that any fix I make for this part of the problem will also cover #60392.

@lenary
Copy link
Contributor

lenary commented Aug 7, 2019

Correction: The constraint code set r{<reg>} means either r or {<reg>} and LLVM is allowed to choose. LLVM does not have many heuristics around this, and seems in this case to choose the first constraint from the set that works, which in this case is the more-general r constraint. Potentially LLVM should issue a warning if these overlap, but with constraint code sets and constraint code set alternatives (LLVM IR: Constraint Codes), this may be hard to achieve.

Update: Looking at the code in LLVM, when you specify a constraint set, it will always try to choose the most general constraint, and doesn't have enough information at that moment to work out if the constraints overlap. This means if you specify r{<reg>}, it will always choose the constraint r, at which point you lose control of the specific register that the compiler will use.

@lenary
Copy link
Contributor

lenary commented Aug 8, 2019

I have just landed https://reviews.llvm.org/rL368303, which solves the bug where you can only use the architectural register names. This has not been back-ported to 9.0, so may take some time to arrive in Rust. For the moment, use the architectural names.

@Amanieu
Copy link
Member

Amanieu commented May 22, 2020

This issue does not apply to the new asm! (RFC 2850) which properly handles register names for operands.

The legacy llvm_asm! is deprecated and is no longer maintained.

@Amanieu Amanieu closed this as completed May 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-inline-assembly Area: Inline assembly (`asm!(…)`) A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. O-riscv Target: RISC-V architecture requires-nightly This issue requires a nightly compiler in some way. 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

6 participants