Skip to content

Moves test::black_box to core::hint and fix black_box on wasm32 and asm.js #59336

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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Mar 28, 2019
Merged

Moves test::black_box to core::hint and fix black_box on wasm32 and asm.js #59336

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
f5d6b3a
Moves test::black_box to core::hint
gnzlbg Mar 21, 2019
f244383
Remove dupplicated config
gnzlbg Mar 25, 2019
cfa76c4
black_box should inhibit optimizations on platforms without inline as…
gnzlbg Mar 25, 2019
24db517
black_box should use inline assembly on wasm32
gnzlbg Mar 25, 2019
d189cab
Use fallback on emscripten targets
gnzlbg Mar 26, 2019
1ea57aa
Add exception for libcore/hint.rs to pal lint of tidy script
gnzlbg Mar 26, 2019
3b6b489
Document why the volatile read is used
gnzlbg Mar 26, 2019
0c127e8
Life's too short not to use cfg_if
gnzlbg Mar 26, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions src/libcore/hint.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,3 +91,42 @@ pub fn spin_loop() {
}
}
}

/// A function that is opaque to the optimizer, to allow benchmarks to
/// pretend to use outputs to assist in avoiding dead-code
/// elimination.
///
/// This function is a no-op, and does not even read from `dummy`.
#[unstable(feature = "test", issue = "27812")]
pub fn black_box<T>(dummy: T) -> T {
#[cfg(not(
any(
target_arch = "asmjs",
all(
target_arch = "wasm32",
target_os = "emscripten"
)
)
))] {
// we need to "use" the argument in some way LLVM can't
// introspect.
unsafe { asm!("" : : "r"(&dummy)) }
dummy
}
#[cfg(
any(
target_arch = "asmjs",
all(
target_arch = "wasm32",
target_os = "emscripten"
)
)
)] {
// asm.js and emscripten do not support inline assembly
unsafe {
let ret = crate::ptr::read_volatile(&dummy);
crate::mem::forget(dummy);
ret
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To simplify the #[cfg] here, perhaps this could be:

pub fn black_box<T>(dummy: T) -> {
    #[cfg(supports_asm)]
    {
        unsafe { asm!(...); }
        return dummy;
    }
    unsafe {
        let ret = read_volatile(...);
        ...;
    }
}

Maybe with an #[allow(unreachable_code)] or something like that to pacify the linter?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The last commit adds cfg_if! to the internal_macros.rs and uses it here. I find this looks infinitely better with cfg_if! than with any of the other options, and I suppose we could access it from core::arch as well.

}
18 changes: 1 addition & 17 deletions src/libtest/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,23 +27,7 @@ pub use libtest::{
TestResult, TrFailed, TrFailedMsg, TrIgnored, TrOk, stats::Summary
};

/// A function that is opaque to the optimizer, to allow benchmarks to
/// pretend to use outputs to assist in avoiding dead-code
/// elimination.
///
/// This function is a no-op, and does not even read from `dummy`.
#[cfg(not(any(target_arch = "asmjs", target_arch = "wasm32")))]
pub fn black_box<T>(dummy: T) -> T {
// we need to "use" the argument in some way LLVM can't
// introspect.
unsafe { asm!("" : : "r"(&dummy)) }
dummy
}
#[cfg(any(target_arch = "asmjs", target_arch = "wasm32"))]
#[inline(never)]
pub fn black_box<T>(dummy: T) -> T {
dummy
}
pub use std::hint::black_box;

#[cfg(test)]
mod tests {
Expand Down
4 changes: 4 additions & 0 deletions src/tools/tidy/src/pal.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ const EXCEPTION_PATHS: &[&str] = &[
"src/libpanic_abort",
"src/libpanic_unwind",
"src/libunwind",
// black_box implementation is LLVM-version specific and it uses
// target_os to tell targets with different LLVM-versions appart
// (e.g. `wasm32-unknown-emscripten` vs `wasm32-unknown-unknown`):
"src/libcore/hint.rs",
"src/libstd/sys/", // Platform-specific code for std lives here.
// This has the trailing slash so that sys_common is not excepted.
"src/libstd/os", // Platform-specific public interfaces
Expand Down