-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Add bad-reg inline assembly ui test for RISC-V and s390x #132516
Conversation
@taiki-e I took the liberty to adjust these tests to use |
`f32`, `f64` and `asm!` macro.
Co-authored-by: Jubilee <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a similar test for rv32e, actually: https://github.com/rust-lang/rust/blob/b3f75cc872cfd306860c3ad76a239e719015f855/tests/ui/abi/riscv32e-registers.rs
I suppose we should merge them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think both should be placed in ui/asm/riscv, but I don't think they can be merged since they test checks that are performed at different stages.
- riscv32e-registers.rs is to see if the LLVM's assembler can reject the use of unsupported registers in assembly code (i.e., invalid assembly code).
- bad-reg.rs is to see if the rustc can reject the use of unsupported registers in the
asm!
API (in,out,inout,etc.).
Only code that passes the latter check is passed to LLVM, so it is probably difficult to test that both work in a single test file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved tests/ui/abi/riscv32e-registers.rs to tests/ui/asm/riscv and added comments explaining the difference to bad-reg.rs. (b07232d)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see. That makes sense, thank you! I wasn't entirely sure of which things were checked where.
This also adds comments explaining the difference to bad-reg.rs.
@bors r+ |
…kingjubilee Rollup of 6 pull requests Successful merges: - rust-lang#126136 (Call the target libdir target libdir) - rust-lang#132516 (Add bad-reg inline assembly ui test for RISC-V and s390x) - rust-lang#132521 (replace manual time convertions with std ones, comptime time format parsing) - rust-lang#132560 (Remove outdated tidy license fixmes) - rust-lang#132563 (Modify `NonZero` documentation to reference the underlying integer type) - rust-lang#132574 (compiler: Directly use rustc_abi almost everywhere) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#132516 - taiki-e:asm-ui, r=workingjubilee Add bad-reg inline assembly ui test for RISC-V and s390x rust-lang#131341 (comment) > Btw, such unsupported registers are present in most architectures, but only aarch64/arm64ec, x86_64, and not yet merged [sparc/sparc64](https://github.com/rust-lang/rust/pull/132472/files#diff-02aebda3376c2b020265137f9ce2c387669ca5cfecd7d60494275c2387db5114) (and powerpc/powerpc64 by this PR) currently have ui tests for them. I plan to add tests for other arches later. Starting with RISC-V and s390x, which I'm familiar with and relatively easy to check for correctness. (Relevant rustc code are supported_types/def_regs/overlapping_regs in [compiler/rustc_target/src/asm/riscv.rs](https://github.com/rust-lang/rust/blob/588a4203508ed7c76750c96b482641261630ed36/compiler/rustc_target/src/asm/riscv.rs) and [compiler/rustc_target/src/asm/s390x.rs](https://github.com/rust-lang/rust/blob/588a4203508ed7c76750c96b482641261630ed36/compiler/rustc_target/src/asm/s390x.rs).) r? workingjubilee `@rustbot` label +A-inline-assembly
#131341 (comment)
Starting with RISC-V and s390x, which I'm familiar with and relatively easy to check for correctness.
(Relevant rustc code are supported_types/def_regs/overlapping_regs in compiler/rustc_target/src/asm/riscv.rs and compiler/rustc_target/src/asm/s390x.rs.)
r? workingjubilee
@rustbot label +A-inline-assembly