riscv64: miri test: error: unsupported operation: inline assembly is not supported

[yuzibo]

When run ./x test --stage 2 src/tools/miri/cargo-miri on riscv64, got:

  Compiling cargo-miri-test v0.1.0 (/home/jenkins/rust/src/tools/miri/test-cargo-miri)
    Finished `test` profile [unoptimized + debuginfo] target(s) in 10.78s
     Running unittests src/main.rs (build/riscv64gc-unknown-linux-gnu/stage3-tools/riscv64gc-unknown-linux-gnu/debug/deps/cargo_miri_test-a0d35f444b5f81e1)

running 2 tests
test test::dev_dependency ... error: unsupported operation: inline assembly is not supported
  --> /home/jenkins/rust/library/core/src/../../stdarch/crates/core_arch/src/riscv_shared/mod.rs:50:9
   |
50 | /         asm!(
51 | |             ".option push",
52 | |             ".option arch, +zihintpause",
53 | |             "pause",
54 | |             ".option pop",
55 | |             options(nomem, nostack, preserves_flags)
56 | |         );
   | |_________^ unsupported operation occurred here
   |
   = help: this is likely not a bug in the program; it indicates that the program performed an operation that Miri does not support
   = note: BACKTRACE on thread `test::dev_depen`:
   = note: inside `std::arch::riscv64::pause` at /home/jenkins/rust/library/core/src/../../stdarch/crates/core_arch/src/riscv_shared/mod.rs:50:9: 56:10
   = note: inside `std::hint::spin_loop` at /home/jenkins/rust/library/core/src/hint.rs:281:36: 281:65
   = note: inside `std::sync::mpmc::utils::Backoff::spin_light` at /home/jenkins/rust/library/std/src/sync/mpmc/utils.rs:116:13: 116:37
   = note: inside `std::sync::mpmc::list::Channel::<test::test::event::CompletedTest>::start_send` at /home/jenkins/rust/library/std/src/sync/mpmc/list.rs:254:21: 254:41
   = note: inside `std::sync::mpmc::list::Channel::<test::test::event::CompletedTest>::send` at /home/jenkins/rust/library/std/src/sync/mpmc/list.rs:410:17: 410:39
   = note: inside `std::sync::mpmc::Sender::<test::test::event::CompletedTest>::send` at /home/jenkins/rust/library/std/src/sync/mpmc/mod.rs:395:41: 395:61
   = note: inside `std::sync::mpsc::Sender::<test::test::event::CompletedTest>::send` at /home/jenkins/rust/library/std/src/sync/mpsc.rs:608:9: 608:27
   = note: inside `test::test::run_test_in_process` at /home/jenkins/rust/library/test/src/lib.rs:701:5: 701:29
   = note: inside closure at /home/jenkins/rust/library/test/src/lib.rs:607:43: 615:18
   = note: inside closure at /home/jenkins/rust/library/test/src/lib.rs:637:41: 637:83
   = note: inside `std::sys::backtrace::__rust_begin_short_backtrace::<{closure@test::test::run_test::{closure#1}}, ()>` at /home/jenkins/rust/library/std/src/sys/backtrace.rs:166:18: 166:21
   = note: inside closure at /home/jenkins/rust/library/std/src/thread/lifecycle.rs:91:13: 91:67
   = note: inside `<std::panic::AssertUnwindSafe<{closure@std::thread::lifecycle::spawn_unchecked<'_, {closure@test::test::run_test::{closure#1}}, ()>::{closure#1}::{closure#0}}> as std::ops::FnOnce<()>>::call_once` at /home/jenkins/rust/library/core/src/panic/unwind_safe.rs:274:9: 274:19
   = note: inside `std::panicking::catch_unwind::do_call::<std::panic::AssertUnwindSafe<{closure@std::thread::lifecycle::spawn_unchecked<'_, {closure@test::test::run_test::{closure#1}}, ()>::{closure#1}::{closure#0}}>, ()>` at /home/jenkins/rust/library/std/src/panicking.rs:581:40: 581:43
   = note: inside `std::panicking::catch_unwind::<(), std::panic::AssertUnwindSafe<{closure@std::thread::lifecycle::spawn_unchecked<'_, {closure@test::test::run_test::{closure#1}}, ()>::{closure#1}::{closure#0}}>>` at /home/jenkins/rust/library/std/src/panicking.rs:544:19: 544:88
   = note: inside `std::panic::catch_unwind::<std::panic::AssertUnwindSafe<{closure@std::thread::lifecycle::spawn_unchecked<'_, {closure@test::test::run_test::{closure#1}}, ()>::{closure#1}::{closure#0}}>, ()>` at /home/jenkins/rust/library/std/src/panic.rs:359:14: 359:40
   = note: inside closure at /home/jenkins/rust/library/std/src/thread/lifecycle.rs:89:26: 92:12
   = note: inside `<{closure@std::thread::lifecycle::spawn_unchecked<'_, {closure@test::test::run_test::{closure#1}}, ()>::{closure#1}} as std::ops::FnOnce<()>>::call_once - shim(vtable)` at /home/jenkins/rust/library/core/src/ops/function.rs:250:5: 250:71
   = note: inside `<std::boxed::Box<dyn std::ops::FnOnce() + std::marker::Send> as std::ops::FnOnce<()>>::call_once` at /home/jenkins/rust/library/alloc/src/boxed.rs:2206:9: 2206:52
   = note: inside `std::sys::thread::unix::Thread::new::thread_start` at /home/jenkins/rust/library/std/src/sys/thread/unix.rs:119:17: 119:29

error: aborting due to 1 previous error

error: test failed, to rerun pass `--bin cargo-miri-test`

Caused by:
  process didn't exit successfully: `/home/jenkins/rust/build/riscv64gc-unknown-linux-gnu/stage2/bin/cargo-miri runner /home/jenkins/rust/build/riscv64gc-unknown-linux-gnu/stage3-tools/riscv64gc-unknown-linux-gnu/debug/deps/cargo_miri_test-a0d35f444b5f81e1` (exit status: 1)
note: test exited abnormally; to see the full output pass --no-capture to the harness.
Command `/home/jenkins/rust/build/riscv64gc-unknown-linux-gnu/stage2/bin/cargo-miri miri test --target riscv64gc-unknown-linux-gnu -Zbinary-dep-depinfo -j 64 -Zroot-dir=/home/jenkins/rust --manifest-path /home/jenkins/rust/src/tools/miri/test-cargo-miri/Cargo.toml -- [workdir=/home/jenkins/rust]` failed with exit code 1
Created at: src/bootstrap/src/core/builder/mod.rs:1430:23
Executed at: src/bootstrap/src/core/build_steps/test.rs:820:19

Command has failed. Rerun with -v to see more details.

See the build log.

Meta

rustc --version --verbose:

rustc 1.93.0-nightly

[folkertdev]

The riscv implementation of spin_loop uses inline assembly. @a4lg you touched that implementation in stdarch recently, is there no intrinsic that we can use? (is this one of the ones that is just not available yet?)

The miri implementation of the x86 _mm_pause yields the current thread, but just doing nothing is a valid implementation, so maybe we should add a miri branch to the cfg_select here that is just a no-op?

rust/library/core/src/hint.rs

Lines 270 to 295 in 6e7dd2c

	pub fn spin_loop() {
	crate::cfg_select! {
	target_arch = "x86" => {
	// SAFETY: the `cfg` attr ensures that we only execute this on x86 targets.
	crate::arch::x86::_mm_pause()
	}
	target_arch = "x86_64" => {
	// SAFETY: the `cfg` attr ensures that we only execute this on x86_64 targets.
	crate::arch::x86_64::_mm_pause()
	}
	target_arch = "riscv32" => crate::arch::riscv32::pause(),
	target_arch = "riscv64" => crate::arch::riscv64::pause(),
	any(target_arch = "aarch64", target_arch = "arm64ec") => {
	// SAFETY: the `cfg` attr ensures that we only execute this on aarch64 targets.
	unsafe { crate::arch::aarch64::__isb(crate::arch::aarch64::SY) }
	}
	all(target_arch = "arm", target_feature = "v6") => {
	// SAFETY: the `cfg` attr ensures that we only execute this on arm targets
	// with support for the v6 feature.
	unsafe { crate::arch::arm::__yield() }
	}
	target_arch = "loongarch32" => crate::arch::loongarch32::ibar::<0>(),
	target_arch = "loongarch64" => crate::arch::loongarch64::ibar::<0>(),
	_ => { /* do nothing */ }
	}
	}

[a4lg]

@folkertdev
There is a LLVM intrinsic llvm.riscv.pause but IIRC this intrinsic requires the Zihintpause extension, which does not match to a behavior I want.
My intention here is, emit PAUSE hint instruction even if Zihintpause extension is not explicitly enabled on the target and let the processor decide what to deal with that hint (lower the priority for spin locks or just handle as regular NOP).

So, though this is not a good idea, I have to recommend branching with cfg(miri).

[folkertdev]

@RalfJung what do you think, would special-casing be OK here? Or should this be handled in miri itself somehow? We could also see if the LLVM implementation could just do what the rust asm does today (that is, emit nop if unsupported).

[RalfJung]

There is a LLVM intrinsic llvm.riscv.pause but IIRC this intrinsic requires the Zihintpause extension, which does not match to a behavior I want.
My intention here is, emit PAUSE hint instruction even if Zihintpause extension is not explicitly enabled on the target and let the processor decide what to deal with that hint (lower the priority for spin locks or just handle as regular NOP).

If the instruction works even without the extension, then why is the LLVM intrinsic marked as "requiring" the extension?

Not yielding the current thread means Miri might get entirely stuck if one runs it with preemption disabled, which can be a useful configuration for debugging and testing purposes. So it'd be better if we had some way to yield the current thread. We could add a miri_spin_loop magic function for that and just use that on all targets... or we make it an intrinsic with the current implementation as fallback body. This feels like something that should be a portable LLVM intrinsic, but it isn't so we have to hand-roll one ourselves.

[a4lg]

TL;DR: It has a good reason not to use LLVM intrinsics but I have an alternative solution (at the end of this post).

@RalfJung

If the instruction works even without the extension, then why is the LLVM intrinsic marked as "requiring" the extension?

Considering the fact that the PAUSE hint instruction is a part of the Zihintpause extension, the LLVM decision at llvm.riscv.pause (requiring the Zihintpause extension) makes sense in one view but not another.

Important keyword here is, hint instructions.

Due to RISC-V's design, many of equivalent-to-NOP instructions exist (e.g. regular register-based arithmetic instruction with the destination of x0 (zero), the hardwired zero register). Some of those are reserved to represent HINTs with no visible architectural side effects (one big exception may be LPAD landing pads that may raise a trap in the Zicfilp extension but that's another story). PAUSE is one of them, using an equivalent-to-NOP encoding of the FENCE instruction.

All RISC-V implementations support executing those hint instructions. Some may treat HINT instruction encodings as true hints and others treat them as NOPs.

Not yielding the current thread

So, yielding the current thread is not guaranteed by the PAUSE hint instruction even if the Zihintpause extension is enabled, just like x86's PAUSE (encoded as REP NOP and runs as regular NOP on older processors). Quoting the RISC-V ISA Manual (version 20250508, 2.9 "HINT Instructions"):

Implementations are always allowed to ignore the encoded hints.

My decisions at pause Rust intrinsic for RISC-V architectures are as follows:

1. Preserve backward compatibility
   Before I rewrote the code, it already used an inline assembly (I just made the inline assembly symbolic) and allowed to use this intrinsic even when the Zihintpause is not enabled. To preserve backward compatibility, I had to support environment without this extension.
2. Developer expects something when he/she use this intrinsic
   For instance, the processor may actually handle the PAUSE instruction even if not explicitly enabled in the toolchain. Or, even if the processor is said to support this extension, the processor may be just ignoring the PAUSE hint instruction and just stating that they support it (this is completely valid per the specification).
   In both cases, emitting PAUSE instruction even if the Zihintpause extension is not enabled at Rust level would be a better choice considering the fact that the user expects something when they use this intrinsic.

So at least, I'm not going to make pause Zihintpause-only.

I could instead write a branch in the pause intrinsic to either:

1. Use llvm.riscv.pause as possible (and fallback to inline assembly; in this case (without option 2), cfg_select! above should be rewritten to detect target_feature = "zihintpause") and/or
2. Detect miri and switch the behavior to perform llvm.riscv.pause (when the Zihintpause extension is enabled on the Rust target) or nothing (just like other targets without pause support).

I would like to hear which direction would be better for both of you. @RalfJung @folkertdev

[RalfJung]

I understand that this is not guaranteed to do anything, but as a quality of implementation concern and for Miri to be consistent across targets, I think it always should yield on Miri. Miri generally aims to make non-deterministic choices that make it most likely to find actual bugs, and in a spin loop, that means we should go find another thread to run.

It would be particularly surprising if behavior differed across targets, given that conceptually this is an entirely target-independent operation.

[a4lg]

@RalfJung I see your concern but it seems it's beyond the scope of an intrinsic (any intrinsic I mean). If the platform does not perform actual yielding/pausing based on the Zihintpause extension, it seems we can do nothing about it here just like this branch:

_ => { /* do nothing */ }

In my view, I guess the option 1 (alone) in my previous post fits best to you.
This is a best-effort attempt, assuming that the Zihintpause extension suggests an actual yielding/pausing behavior on PAUSE (yet no guarantee) on the target and does not change the behavior depending on Miri existence.

1. Modify RISC-V branches of spin_lock to also check target_feature = "zihintpause".
2. Modify pause intrinsic to use llvm.riscv.pause LLVM intrinsic when target_feature = "zihintpause" is satisfied.

[RalfJung]

We can absolutely do something, as I already suggested above:

miri => {
  // Invoke special Miri function to inform it about the spin loop hint.
  extern "Rust" { fn miri_spin_loop(); }
  miri_spin_loop();
}

We have a bunch of such special Miri functions already, it seems reasonable to me to add one for this purpose.

[a4lg]

Ah, I see. Since that's not a field I usually contribute to, I didn’t think of it, and I’m sorry I didn’t read your comments carefully.
Considering need for the deterministic behavior, I think that’s the most reasonable approach.