From f5d6b3af6966470f7f3beac16ec2bd77b04d8905 Mon Sep 17 00:00:00 2001 From: gnzlbg Date: Thu, 21 Mar 2019 09:15:52 +0100 Subject: [PATCH 1/8] Moves test::black_box to core::hint This changes removes a cyclic dependency between the "test" and "libtest" crates, where "libtest" depends on "test" for "black_box", but "test" depends on "libtest" for everything else. I've chosen the "hint" module because there seems to be enough consensus in the discussion of RFC2360 that this module is where such an intrinsic would belong, but this PR does not implement that RFC! (note: if that RFC ever gets merged, the API, docs, etc. of this API will need to change). For backwards compatibility reasons I've chosen to also keep the "test" feature gate for these instead of adding a new feature gate. If we change the feature gate, we'll potentially all benchmarks, and while that's something that we could do, it seems unnecessary to do that now - if RFC2360 gets merged, we'll need to do that anyways. --- src/libcore/hint.rs | 19 +++++++++++++++++++ src/libtest/lib.rs | 18 +----------------- 2 files changed, 20 insertions(+), 17 deletions(-) diff --git a/src/libcore/hint.rs b/src/libcore/hint.rs index b2f82ef0d175c..d6ddab0d8f5fb 100644 --- a/src/libcore/hint.rs +++ b/src/libcore/hint.rs @@ -91,3 +91,22 @@ 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`. +#[cfg_attr(any(target_arch = "asmjs", target_arch = "wasm32"), inline(never))] +#[unstable(feature = "test", issue = "27812")] +pub fn black_box(dummy: T) -> T { + #[cfg(not(any(target_arch = "asmjs", target_arch = "wasm32")))] { + // 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"))] { + dummy + } +} diff --git a/src/libtest/lib.rs b/src/libtest/lib.rs index cb0ce480e4273..5c91c0ec43b19 100644 --- a/src/libtest/lib.rs +++ b/src/libtest/lib.rs @@ -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(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(dummy: T) -> T { - dummy -} +pub use std::hint::black_box; #[cfg(test)] mod tests { From f2443831e97279d94902997d2c012d2e92d5d996 Mon Sep 17 00:00:00 2001 From: gnzlbg Date: Mon, 25 Mar 2019 11:42:21 +0100 Subject: [PATCH 2/8] Remove dupplicated config --- src/libcore/hint.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libcore/hint.rs b/src/libcore/hint.rs index d6ddab0d8f5fb..75d64bbe0fb08 100644 --- a/src/libcore/hint.rs +++ b/src/libcore/hint.rs @@ -97,7 +97,6 @@ pub fn spin_loop() { /// elimination. /// /// This function is a no-op, and does not even read from `dummy`. -#[cfg_attr(any(target_arch = "asmjs", target_arch = "wasm32"), inline(never))] #[unstable(feature = "test", issue = "27812")] pub fn black_box(dummy: T) -> T { #[cfg(not(any(target_arch = "asmjs", target_arch = "wasm32")))] { @@ -107,6 +106,7 @@ pub fn black_box(dummy: T) -> T { dummy } #[cfg(any(target_arch = "asmjs", target_arch = "wasm32"))] { - dummy + #[inline(never)] fn black_box_(x: T) -> T { x } + black_box_(dummy) } } From cfa76c438a907bf223e4487ab4da2500277474cc Mon Sep 17 00:00:00 2001 From: gnzlbg Date: Mon, 25 Mar 2019 11:49:08 +0100 Subject: [PATCH 3/8] black_box should inhibit optimizations on platforms without inline assembly --- src/libcore/hint.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/libcore/hint.rs b/src/libcore/hint.rs index 75d64bbe0fb08..abaf0b31b468f 100644 --- a/src/libcore/hint.rs +++ b/src/libcore/hint.rs @@ -106,7 +106,10 @@ pub fn black_box(dummy: T) -> T { dummy } #[cfg(any(target_arch = "asmjs", target_arch = "wasm32"))] { - #[inline(never)] fn black_box_(x: T) -> T { x } - black_box_(dummy) - } + unsafe { + let ret = crate::ptr::read_volatile(&dummy); + crate::mem::forget(dummy); + ret + } + } } From 24db5174191c915bb7a61809837aaa5cb98cfa11 Mon Sep 17 00:00:00 2001 From: gnzlbg Date: Mon, 25 Mar 2019 18:11:42 +0100 Subject: [PATCH 4/8] black_box should use inline assembly on wasm32 --- src/libcore/hint.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libcore/hint.rs b/src/libcore/hint.rs index abaf0b31b468f..e73a1a2879347 100644 --- a/src/libcore/hint.rs +++ b/src/libcore/hint.rs @@ -99,17 +99,17 @@ pub fn spin_loop() { /// This function is a no-op, and does not even read from `dummy`. #[unstable(feature = "test", issue = "27812")] pub fn black_box(dummy: T) -> T { - #[cfg(not(any(target_arch = "asmjs", target_arch = "wasm32")))] { + #[cfg(not(target_arch = "asmjs"))] { // 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"))] { + #[cfg(target_arch = "asmjs")] { unsafe { let ret = crate::ptr::read_volatile(&dummy); crate::mem::forget(dummy); ret } - } + } } From d189cab027bee94f4a4ef09484e30354c9843764 Mon Sep 17 00:00:00 2001 From: gnzlbg Date: Tue, 26 Mar 2019 09:12:06 +0100 Subject: [PATCH 5/8] Use fallback on emscripten targets --- src/libcore/hint.rs | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/libcore/hint.rs b/src/libcore/hint.rs index e73a1a2879347..856720bd55427 100644 --- a/src/libcore/hint.rs +++ b/src/libcore/hint.rs @@ -99,13 +99,29 @@ pub fn spin_loop() { /// This function is a no-op, and does not even read from `dummy`. #[unstable(feature = "test", issue = "27812")] pub fn black_box(dummy: T) -> T { - #[cfg(not(target_arch = "asmjs"))] { + #[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(target_arch = "asmjs")] { + #[cfg( + any( + target_arch = "asmjs", + all( + target_arch = "wasm32", + target_os = "emscripten" + ) + ) + )] { unsafe { let ret = crate::ptr::read_volatile(&dummy); crate::mem::forget(dummy); From 1ea57aa3f98b9e7fffd49f95ed05503e9ddf2d7f Mon Sep 17 00:00:00 2001 From: gnzlbg Date: Tue, 26 Mar 2019 12:15:06 +0100 Subject: [PATCH 6/8] Add exception for libcore/hint.rs to pal lint of tidy script --- src/tools/tidy/src/pal.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/tools/tidy/src/pal.rs b/src/tools/tidy/src/pal.rs index ed2218f09d26b..96dfb2f1acd9b 100644 --- a/src/tools/tidy/src/pal.rs +++ b/src/tools/tidy/src/pal.rs @@ -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 From 3b6b4899bf699f4098b8cdd773a474b483f23462 Mon Sep 17 00:00:00 2001 From: gnzlbg Date: Tue, 26 Mar 2019 13:43:57 +0100 Subject: [PATCH 7/8] Document why the volatile read is used --- src/libcore/hint.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libcore/hint.rs b/src/libcore/hint.rs index 856720bd55427..6a6f8893bfbd6 100644 --- a/src/libcore/hint.rs +++ b/src/libcore/hint.rs @@ -122,6 +122,7 @@ pub fn black_box(dummy: T) -> T { ) ) )] { + // asm.js and emscripten do not support inline assembly unsafe { let ret = crate::ptr::read_volatile(&dummy); crate::mem::forget(dummy); From 0c127e849487323a9f6be09d25c0da0aeb57314d Mon Sep 17 00:00:00 2001 From: gnzlbg Date: Tue, 26 Mar 2019 16:14:32 +0100 Subject: [PATCH 8/8] Life's too short not to use cfg_if --- src/libcore/hint.rs | 43 +++++++++--------- src/libcore/internal_macros.rs | 81 ++++++++++++++++++++++++++++++++++ 2 files changed, 101 insertions(+), 23 deletions(-) diff --git a/src/libcore/hint.rs b/src/libcore/hint.rs index 6a6f8893bfbd6..d1ccc148654ca 100644 --- a/src/libcore/hint.rs +++ b/src/libcore/hint.rs @@ -97,36 +97,33 @@ pub fn spin_loop() { /// elimination. /// /// This function is a no-op, and does not even read from `dummy`. +#[inline] #[unstable(feature = "test", issue = "27812")] pub fn black_box(dummy: T) -> T { - #[cfg(not( - any( + cfg_if! { + if #[cfg(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 + ))] { + #[inline] + unsafe fn black_box_impl(d: T) -> T { + // these targets do not support inline assembly + let ret = crate::ptr::read_volatile(&d); + crate::mem::forget(d); + ret + } + } else { + #[inline] + unsafe fn black_box_impl(d: T) -> T { + // we need to "use" the argument in some way LLVM can't + // introspect. + asm!("" : : "r"(&d)); + d + } } } + unsafe { black_box_impl(dummy) } } diff --git a/src/libcore/internal_macros.rs b/src/libcore/internal_macros.rs index b5c20582986b2..ee6b7d3db48a6 100644 --- a/src/libcore/internal_macros.rs +++ b/src/libcore/internal_macros.rs @@ -119,3 +119,84 @@ macro_rules! impl_fn_for_zst { )+ } } + +/// A macro for defining `#[cfg]` if-else statements. +/// +/// The macro provided by this crate, `cfg_if`, is similar to the `if/elif` C +/// preprocessor macro by allowing definition of a cascade of `#[cfg]` cases, +/// emitting the implementation which matches first. +/// +/// This allows you to conveniently provide a long list `#[cfg]`'d blocks of code +/// without having to rewrite each clause multiple times. +/// +/// # Example +/// +/// ``` +/// #[macro_use] +/// extern crate cfg_if; +/// +/// cfg_if! { +/// if #[cfg(unix)] { +/// fn foo() { /* unix specific functionality */ } +/// } else if #[cfg(target_pointer_width = "32")] { +/// fn foo() { /* non-unix, 32-bit functionality */ } +/// } else { +/// fn foo() { /* fallback implementation */ } +/// } +/// } +/// +/// # fn main() {} +/// ``` +macro_rules! cfg_if { + // match if/else chains with a final `else` + ($( + if #[cfg($($meta:meta),*)] { $($it:item)* } + ) else * else { + $($it2:item)* + }) => { + cfg_if! { + @__items + () ; + $( ( ($($meta),*) ($($it)*) ), )* + ( () ($($it2)*) ), + } + }; + + // match if/else chains lacking a final `else` + ( + if #[cfg($($i_met:meta),*)] { $($i_it:item)* } + $( + else if #[cfg($($e_met:meta),*)] { $($e_it:item)* } + )* + ) => { + cfg_if! { + @__items + () ; + ( ($($i_met),*) ($($i_it)*) ), + $( ( ($($e_met),*) ($($e_it)*) ), )* + ( () () ), + } + }; + + // Internal and recursive macro to emit all the items + // + // Collects all the negated cfgs in a list at the beginning and after the + // semicolon is all the remaining items + (@__items ($($not:meta,)*) ; ) => {}; + (@__items ($($not:meta,)*) ; ( ($($m:meta),*) ($($it:item)*) ), $($rest:tt)*) => { + // Emit all items within one block, applying an approprate #[cfg]. The + // #[cfg] will require all `$m` matchers specified and must also negate + // all previous matchers. + cfg_if! { @__apply cfg(all($($m,)* not(any($($not),*)))), $($it)* } + + // Recurse to emit all other items in `$rest`, and when we do so add all + // our `$m` matchers to the list of `$not` matchers as future emissions + // will have to negate everything we just matched as well. + cfg_if! { @__items ($($not,)* $($m,)*) ; $($rest)* } + }; + + // Internal macro to Apply a cfg attribute to a list of items + (@__apply $m:meta, $($it:item)*) => { + $(#[$m] $it)* + }; +}