-
Notifications
You must be signed in to change notification settings - Fork 90
Replace black_box with std::hint::black_box #96
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
Conversation
Rust version 1.66 now comes with a stabilized version of a black_box function that emits an "empty" assembly directive. This patch replaces the older optimization barrier, which was based on a volatile read with the newly standardized function. Announcement from Mara: <https://hachyderm.io/@Mara/109518823276877083> Current implementation of black_box in rustc: https://github.com/rust-lang/rust/blob/a803f313fdf8f6eb2d674d7dfb3694a2b437ee1e/compiler/rustc_codegen_llvm/src/intrinsic.rs#L341-L375
a2143c8
to
4978f42
Compare
This commit partially reverts the changes from 4978f42, and adds the new optimization barrier implementation under a new feature `core_hint_black_box`.
@tarcieri Thanks for the review! I added back the original implementation (based on the volatile read) and added the new barrier implementation under the I also updated the README file a bit to better represent how the barrier got updated over time |
FYI: I don't actually have commit access to I've asked about what to do with these PRs. Looks good to me now aside from bikeshedding the feature name. |
Mild preference for |
@dsprenkels Thanks for updating the documentation as well! |
Heads up: the rustdoc for this function on the beta channel of rustc now contains a big scary warning saying https://doc.rust-lang.org/beta/core/hint/fn.black_box.html#when-is-this-useful I think this should probably be reverted. |
I completely agree. |
The clarification docs were added in rust-lang/rust#108088. When I try to follow the rabbit hole I cannot seem to find where it was exactly specified first that |
Rust version 1.66 now comes with a stabilized version of a black_box function that emits an "empty" assembly directive. This patch replaces the older optimization barrier, which was based on a volatile read with the newly standardized function.
Announcement from Mara:
https://hachyderm.io/@Mara/109518823276877083
Current implementation of black_box in rustc:
https://github.com/rust-lang/rust/blob/a803f313fdf8f6eb2d674d7dfb3694a2b437ee1e/compiler/rustc_codegen_llvm/src/intrinsic.rs#L341-L375
Review suggestions:
cfg_version
is stabilized, and then write a fallbackblack_box
based for rustc before 1.66.