Skip to content

Commit

Permalink
Auto merge of rust-lang#131572 - cuviper:ub-index_range, r=thomcc
Browse files Browse the repository at this point in the history
Avoid superfluous UB checks in `IndexRange`

`IndexRange::len` is justified as an overall invariant, and
`take_prefix` and `take_suffix` are justified by local branch
conditions. A few more UB-checked calls remain in cases that are only
supported locally by `debug_assert!`, which won't do anything in
distributed builds, so those UB checks may still be useful.

We generally expect core's `#![rustc_preserve_ub_checks]` to optimize
away in user's release builds, but the mere presence of that extra code
can sometimes inhibit optimization, as seen in rust-lang#131563.
  • Loading branch information
bors committed Oct 17, 2024
2 parents 3ed6e3c + 5365b3f commit d9c4b8d
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 4 deletions.
11 changes: 7 additions & 4 deletions library/core/src/ops/index_range.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ impl IndexRange {
#[inline]
pub const fn len(&self) -> usize {
// SAFETY: By invariant, this cannot wrap
unsafe { self.end.unchecked_sub(self.start) }
// Using the intrinsic because a UB check here impedes LLVM optimization. (#131563)
unsafe { crate::intrinsics::unchecked_sub(self.end, self.start) }
}

/// # Safety
Expand Down Expand Up @@ -82,7 +83,8 @@ impl IndexRange {
let mid = if n <= self.len() {
// SAFETY: We just checked that this will be between start and end,
// and thus the addition cannot overflow.
unsafe { self.start.unchecked_add(n) }
// Using the intrinsic avoids a superfluous UB check.
unsafe { crate::intrinsics::unchecked_add(self.start, n) }
} else {
self.end
};
Expand All @@ -100,8 +102,9 @@ impl IndexRange {
pub fn take_suffix(&mut self, n: usize) -> Self {
let mid = if n <= self.len() {
// SAFETY: We just checked that this will be between start and end,
// and thus the addition cannot overflow.
unsafe { self.end.unchecked_sub(n) }
// and thus the subtraction cannot overflow.
// Using the intrinsic avoids a superfluous UB check.
unsafe { crate::intrinsics::unchecked_sub(self.end, n) }
} else {
self.start
};
Expand Down
5 changes: 5 additions & 0 deletions tests/codegen/issues/issue-101082.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
//@ compile-flags: -O
//@ revisions: host x86-64-v3

// This particular CPU regressed in #131563
//@[x86-64-v3] only-x86_64
//@[x86-64-v3] compile-flags: -Ctarget-cpu=x86-64-v3

#![crate_type = "lib"]

Expand Down

0 comments on commit d9c4b8d

Please sign in to comment.