Skip to content

Conversation

eddyb
Copy link
Member

@eddyb eddyb commented Aug 15, 2020

This PR is the minimal change necessary to get LLVM to optimize if self.t.tv_nsec >= other.t.tv_nsec to branchless instructions (at least on x86_64), inspired by @m-ou-se's own attempts at optimizing Instant subtraction.

I stumbled over this by looking at the total number of instructions executed by rustc -Z self-profile, and found that after disabling ASLR, the largest source of non-determinism remaining was from this if taking one branch or the other, depending on the values involved.

The reason this code is even called so many times to make a difference, is that measureme (the -Z self-profile implementation) currently uses Instant::elapsed for its event timestamps (of which there can be millions).

I doubt it's critical to land this, although perhaps it could slightly improve some forms of benchmarking.

@rust-highfive
Copy link
Contributor

r? @sfackler

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 15, 2020
@sfackler
Copy link
Member

LGTM other than the 1 nit

@eddyb eddyb force-pushed the instant-sub-branchless branch from 40a6a17 to a7ad899 Compare August 15, 2020 01:13
@eddyb
Copy link
Member Author

eddyb commented Aug 15, 2020

@bors r=sfackler

@bors
Copy link
Collaborator

bors commented Aug 15, 2020

📌 Commit a7ad899 has been approved by sfackler

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 15, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 15, 2020
Rollup of 4 pull requests

Successful merges:

 - rust-lang#75376 (Set CMAKE_SYSTEM_NAME when cross-compiling)
 - rust-lang#75448 (merge `as_local_hir_id` with `local_def_id_to_hir_id`)
 - rust-lang#75513 (Recover gracefully from `struct` parse errors)
 - rust-lang#75545 (std/sys/unix/time: make it easier for LLVM to optimize `Instant` subtraction.)

Failed merges:

 - rust-lang#75514 (Replaced `log` with `tracing`)

r? @ghost
@bors bors merged commit 29a9462 into rust-lang:master Aug 15, 2020
@eddyb eddyb deleted the instant-sub-branchless branch August 15, 2020 16:40
@cuviper cuviper added this to the 1.47.0 milestone May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants