Skip to content

Commit 29a9462

Browse files
authored
Rollup merge of #75545 - eddyb:instant-sub-branchless, r=sfackler
std/sys/unix/time: make it easier for LLVM to optimize `Instant` subtraction. 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.
2 parents e38eaf2 + a7ad899 commit 29a9462

File tree

1 file changed

+20
-8
lines changed

1 file changed

+20
-8
lines changed

library/std/src/sys/unix/time.rs

+20-8
Original file line numberDiff line numberDiff line change
@@ -20,17 +20,29 @@ impl Timespec {
2020

2121
fn sub_timespec(&self, other: &Timespec) -> Result<Duration, Duration> {
2222
if self >= other {
23-
Ok(if self.t.tv_nsec >= other.t.tv_nsec {
24-
Duration::new(
25-
(self.t.tv_sec - other.t.tv_sec) as u64,
26-
(self.t.tv_nsec - other.t.tv_nsec) as u32,
27-
)
23+
// NOTE(eddyb) two aspects of this `if`-`else` are required for LLVM
24+
// to optimize it into a branchless form (see also #75545):
25+
//
26+
// 1. `self.t.tv_sec - other.t.tv_sec` shows up as a common expression
27+
// in both branches, i.e. the `else` must have its `- 1`
28+
// subtraction after the common one, not interleaved with it
29+
// (it used to be `self.t.tv_sec - 1 - other.t.tv_sec`)
30+
//
31+
// 2. the `Duration::new` call (or any other additional complexity)
32+
// is outside of the `if`-`else`, not duplicated in both branches
33+
//
34+
// Ideally this code could be rearranged such that it more
35+
// directly expresses the lower-cost behavior we want from it.
36+
let (secs, nsec) = if self.t.tv_nsec >= other.t.tv_nsec {
37+
((self.t.tv_sec - other.t.tv_sec) as u64, (self.t.tv_nsec - other.t.tv_nsec) as u32)
2838
} else {
29-
Duration::new(
30-
(self.t.tv_sec - 1 - other.t.tv_sec) as u64,
39+
(
40+
(self.t.tv_sec - other.t.tv_sec - 1) as u64,
3141
self.t.tv_nsec as u32 + (NSEC_PER_SEC as u32) - other.t.tv_nsec as u32,
3242
)
33-
})
43+
};
44+
45+
Ok(Duration::new(secs, nsec))
3446
} else {
3547
match other.sub_timespec(self) {
3648
Ok(d) => Err(d),

0 commit comments

Comments
 (0)