Move test::black_box to std and stabilize it

The test crate is not stable, and I’m not aware of plans to stabilize it soon. It’s probably better to spend some time experimenting on crates.io using harness = false in Cargo.toml (see rust-lang/cargo#2305).

I’ve extracted the test crate to eventually publish it separately and am removing usage of unstable features one by one. One of them is asm, used in:

/// 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(all(target_os = "nacl", target_arch = "le32")))]
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(all(target_os = "nacl", target_arch = "le32"))]
#[inline(never)]
pub fn black_box<T>(dummy: T) -> T {
    dummy
}

It’s an important part of benchmarking. Since asm! is not stable and also unlikely to be stable soon, I’d like to have black_box stabilized in std, as a building block for external test and benchmarking harnesses.

I’m not sure what module it would go into. Maybe std::time?

std::mem might make more sense as a location, but this seems like a reasonable thing to do regardless.

I like the idea too, especially if it lets folks experiment with benchmarking in an external crate. std::mem sounds betterish than std::time I think.

Is the documentation of black_box presented here something we can guarantee? What happens on nacl?

Is the documentation of black_box presented here something we can guarantee?

Probably not. E.g. a quick scan of the LLVM inline-asm docs implies it doesn't seem to say that it won't ever do any optimisations.

What happens on nacl?

I imagine black_box has absolutely no effect. I guess something like the following might work better (at some cost), although it will likely fail with LTO.

#[inline(never)]
fn not_cross_crate(x: *mut u8) {}

pub fn black_box<T>(dummy: T) -> T {
    not_cross_crate(&dummy as *const _ as *mut u8);
    dummy
}

@huonw

I don't think that we should stabilize black_box as it's currently "just a hack that appears to work". We don't really have any guarantees on behalf of LLVM nor do we really know what it does, it just emperically seems to not optimize things away. A solution like @huonw mentioned of a non-cross-crate function (e.g. something that forces codegen) would unfortunately have runtime overhead (which in theory this doesn't).

Should we file an issue on LLVM about them providing some kind of black box with this guarantee? Between benchmark and timing attacks it’s not like we’re the only ones dealing with this problem.

Perhaps, yeah, but it may not be too productive to do so without clearly figuring out what we want to actually do here. For example:

Do we just want the value to not be optimized away?
Do we want to prevent constant propagation?
Do we want LLVM to think this isn't a constant even though it is?
Are there other optimizations we want to prevent from happening? (we probably can't just say "don't optimize this value")

The point of black_box is to behave as an "optimization barrier" for a variable. It should do two things:

On entry to black_box, it should force the computation of its argument. The optimizer has to assume black_box depends on its argument and anything it might possibly point to;
On exit from black_box, it should assume nothing about its return value. The optimizer has to assume black_box might have produced its return value from thin air. In particular, even if the argument to black_box was &T, the optimizer has to assume black_box might have returned a different &T, pointing to different contents.

Since it's a compiler barrier, a good place to look is the Linux kernel. What we want is a mixture of barrier_data and OPTIMIZER_HIDE_VAR. That is, what we want is the equivalent of the following inline assembly statement in GCC's syntax (Rust's syntax is slightly different):

__asm__ __volatile__ ("" : "=r" (var) : "0" (var) : "memory")

This statement produces no assembly (and the input and output registers are the same), but the optimizer is forced to assume:

This statement should not be reordered with other statements, even if they seem to be unrelated;
This statement might depend unpredictably on the input variable, and whatever it points to;
This statement might return a different unpredictable value for the input variable, or if allowed (in case of &mut T or similar) might unpredictably modify whatever it points to.

It's not a no-op, and probably a naive idea, but would copying the value with ptr::read_volatile() and/or ptr::write_volatile() have the intended effect?

It definitely prevented the elision of memset() for zeroing a buffer on-drop in this case, though I used it to read the first byte in the buffer instead of the Vec object on-stack.

I would expect reading/rewriting the first byte of the value on-stack to be safe and of negligible cost while inducing the desired effect. It can be a no-op for ZSTs since they don't have any state to make assumptions of.

Addendum: this seems to produce the desired effect of preventing elision of memset(), even without #[inline(never)]: https://is.gd/T6Vbb4

Version that covers ZSTs, branch is optimized out: https://is.gd/ApKyY7

@abonander

@abonander Just for exploration and for future reference, the read_volatile thing is not enough.

playpen link

use std::ptr;

fn main() {
    let buf = (Vec::<u8>::with_capacity(128), Vec::<u8>::with_capacity(64));

    black_box(buf);
}

fn black_box<T>(val: T) -> T {
    let _ = unsafe { ptr::read_volatile(&val as *const T as *const u8) };
    val
}

The allocation of the second vector will be optimized out, so it's using detailed information that only the first element of the tuple is needed. Using test::black_box, none of the vecs are optimized out, like we want.

@bluss

@bluss Copying the whole value, however, does prevent the elision of the second allocation: https://is.gd/6Oc0xR

This also doesn't need to be special-cased. However, because it does force a copy of the full value it can be slow for larger types.

@abonander

@abonander Thanks, that does work well so far

@cesarb

Standardizing a black_box function with the behaviour @cesarb described would be very useful in a bunch of different contexts beyond just benchmarking, and it's much smaller than, say, stabilizing inline asm.

As an alternative, what happens if we pass a pointer to the value to an extern "C" function defined somewhere else, maybe in the stdlib or in some C code? LLVM can't introspect that, right, and must assume it's going to be used? But I'm guessing LTO can introspect that and optimize it out unless it's in a dylib, but LTO would probably also optimize out the current impl so I don't know.

One workaround for the LTO issue is to use the same inline asm trick (inside the .c file), for instance https://github.com/cesarb/clear_on_drop/blob/master/src/hide.c

Since the LLVM clobbers-in-asm-block behaviour is what's being relied on anyways, it seems cleaner and simpler for Rust to provide a black_box function instead of wrapping the inline asm in a .c file.

But LLVM can still reorder writes to e.g. stack variables, based on escape/alias analysis, right?
Since they can never alias the volatile read without UB.

But LLVM can still reorder writes to e.g. stack variables, based on escape/alias analysis, right?

Right. The PR to the RFC precises a bit more what I meant with clobbering memory:

Clobber is specified to guarantee that writes through pointers that have previously been black_boxed are flushed to memory. So for example in the case of benchmarking Vec::push, we first need to black_box Vec's pointer and then after pushing we flush the writes through that pointer:

fn bench_vec_push_back(bench: Bencher) -> BenchResult {
    let n = 100_000_000;
    let mut v = Vec::with_capacity(n);
    bench.iter_n(n, || {
        // Allow vector data to be clobbered:
        mem::black_box(v.as_ptr());
        v.push_back(42_u8);
        // Forces writes through `v.as_ptr()` to be written back to memory,
        // that is, 42 is written back to memory:
        mem::clobber();
    })
}

The Google Benchmark library that these functions are based on specify them exactly like this and the docs particularly insist that clobber only works for variables that have previously been black_boxed (they call their black_box function DoNotOptimize).

Clobber is specified to guarantee that writes through pointers that have previously been black_boxed are flushed to memory.

Ah, this makes a lot more sense, thanks!

Also, because this all happens within a single thread no memory fences / synchronization instructions need to be emitted (e.g. data-races can't happen).

I linked compiler_fence and not fence specifically because of this.
Oh and I think black_box on the pointer means LLVM has to assume the pointer might have escaped to another thread, so I'm not sure we need additional machinery here.

I think that the difference between black_box and clobber is confusing and very error-prone. Perhaps the name black_box is itself to vague and a better name should be chosen?

IMO, the crux of the problem is that an entire computation may be omitted because of optimisation guarantees. So, IMO, there are two possible functions to aid this:

/// The compiler forces `x` to have a known value. This prevents optimisations across the boundaries of this function, although this does not guarantee that `x` is evaluated if the result is ultimately unused.
fn value_fence<T>(X: T) -> T;

/// The compiler forces `x` to have a known value, ensuring that all the computations required to do so are executed. After this, the value is dropped.
fn evaluate_and_drop<T>(X: T);

Note that the former is basically a read_volatile and forget, while the latter appears to be a write_volatile and forget.

To clarify exactly:

fn value_fence<T>(x: T) -> T {
    let y = unsafe { (&x as *const T).read_volatile() };
    std::mem::forget(x);
    y
}

fn evaluate_and_drop<T>(x: T) {
    let y = unsafe { std::mem::uninitialised() };
    unsafe { (&y as *const T).write_volatile(x); }
    drop(y); // not necessary but for clarity
}

I misremembered the API for write_volatile and no forget is required.

@clarcharr Could you show how to benchmark Vec::push with them? That would help me understand them better. I posted the Vec::push example here: #1484 (comment)

Is it possible to use the inline asm approach, which is a no-op, instead of a volatile read, which isn't?

@gnzlbg

@gnzlbg I'd probably do a vec.push(); evaluate_and_drop(vec.last())

@hdevalence I don't think that any approach will really be a no-op, but considering how the value will already be in cache when the function runs, performance penalty will only be a few cycles max.

So I've tried @clarcharr suggestion here: https://godbolt.org/g/HvZJZW and the three alternatives (test::black_box, clobber, and ptr::write_volatile) all produce slightly different code:

test::black_box: 28 asm instructions
ptr::write_volatile: 27 asm instructions
clobber: 26 asm instructions

Maybe someone with more asm experience than me could look at their disassemblies (shown in godbolt) and comment on what they are exactly doing. Running these through cargo bench, they all report the exact same timings for Vec::push (in my machine 3ns per iter +/- 0).

I've let this settle a bit, but I wanted to add two things:

First, ptr::{read,write}_volatile are already available in std, that is, one can already use them when writing benchmarks. The benchmarking docs do not mention that these might be useful for this purpose, but in my opinion that is a documentation issue.

The purpose of test::black_box (and similar functions, like clobber) is to disable compiler optimizations. I think that functions with this purpose should not generate any code.

That would rule out value_fence and evaluate_and_drop, at least with their proposed implementation, but both test::black_box, and google_benchmark::{black_box, clobber} pass this litmus test.

So I think it would be important to agree if this is a property that we want to preserve. Should functions whose intent is to just disable compiler optimizations be allowed to generate any code when, for example, used in isolation?

As mentioned, I think that ruling out value_fence and evaluate_and_drop is not a downside, because they can be implemented in stable rust already without issues. OTOH the other functions being discussed require inline assembly tailored to LLVM, so we would currently need to expose them via std to make them available on stable and also to make sure that they work if we ever get another backend like Cranelift (Google Benchmark has implementations for MSVC, GCC, and other compilers, and these are tailored to the inline assembly functionality of each compiler, which support different features).

I'm still a proponent of using ptr::write_volatile(); the version that performs the volatile write on a pointer to the data is O(1) to the size of the data itself, meaning its overhead is constant across all invocations and thus can be factored out of performance comparisons. I haven't done enough testing to be sure that it guarantees everything we want out of black_box() but it seems like not too bad of a place to start.

I'm still a proponent of using ptr::write_volatile(); the version that performs the volatile write on a pointer to the data is O(1) to the size of the data itself, meaning its overhead is constant across all invocations and thus can be factored out of performance comparisons.

I agree with this statement. For completeness it is worth mentioning that a consequence of doing this is that the code being measured in the benchmarks using ptr::write_volatile() is not the same code that will run in your application. The assembly tests I showed above seem to corroborate this statement.

The google benchmark intrinsics intent is to allow benchmarking the exact same code that the compiler would have emitted in your real application. They might or might not succeed at this (this is hard, and it is not only up to them, but also up to the benchmark writer who must be extra careful), but that is at least what they attempt to enable doing. Since doing this is hard, this might not be the only tools that people will have while writing benchmarks though. One cool thing about test::black_box is that is extremely simple to use.

I've opened a PR for this here: #2360

Closing in favor of #2360.

Navigation Menu

Repository navigation

Move test::black_box to std and stabilize it #1484

11 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Move test::black_box to std and stabilize it #1484

Description

Activity

sfackler commented on Feb 1, 2016

BurntSushi commented on Feb 1, 2016

huonw commented on Feb 1, 2016

alexcrichton commented on Feb 1, 2016

SimonSapin commented on Feb 1, 2016

alexcrichton commented on Feb 1, 2016

cesarb commented on Feb 10, 2016

abonander commented on Jul 4, 2016

bluss commented on Aug 18, 2016

abonander commented on Aug 18, 2016

bluss commented on Aug 22, 2016

hdevalence commented on Nov 21, 2017

abonander commented on Nov 21, 2017

hdevalence commented on Nov 21, 2017

11 remaining items

eddyb commented on Feb 1, 2018

gnzlbg commented on Feb 1, 2018

eddyb commented on Feb 1, 2018

clarfonthey commented on Feb 2, 2018

clarfonthey commented on Feb 2, 2018

gnzlbg commented on Feb 2, 2018

hdevalence commented on Feb 3, 2018

clarfonthey commented on Feb 3, 2018

gnzlbg commented on Feb 3, 2018

gnzlbg commented on Feb 9, 2018

abonander commented on Feb 9, 2018

gnzlbg commented on Feb 9, 2018

gnzlbg commented on Mar 12, 2018

Centril commented on Oct 7, 2018

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions