Skip to content

Commit 273fbf4

Browse files
authored
Rollup merge of #111282 - scottmcm:remove-unneeded-assumes, r=workingjubilee
Remove some `assume`s from slice iterators that don't do anything Because the start pointer is iterators is already a `NonNull`, we emit the appropriate `!nonnull` metadata when loading the pointer to tell LLVM that it's non-null. Probably the best way to see that it's the metadata that's important (and not the `assume`) is to observe that LLVM actually *removes* the `assume` from the optimized IR: <https://rust.godbolt.org/z/KhE6G963n>. (I also checked that, yes, the if-not-ZST `assume` on `end` is still doing something: it's how there's a `!nonnull` metadata on its load, even though it's an ordinary raw pointer. The codegen test added in this PR fails if the other `assume` is removed.)
2 parents efe697e + ec3a9bc commit 273fbf4

File tree

2 files changed

+48
-10
lines changed

2 files changed

+48
-10
lines changed

library/core/src/slice/iter/macros.rs

+6-10
Original file line numberDiff line numberDiff line change
@@ -124,12 +124,10 @@ macro_rules! iterator {
124124
fn next(&mut self) -> Option<$elem> {
125125
// could be implemented with slices, but this avoids bounds checks
126126

127-
// SAFETY: `assume` calls are safe since a slice's start pointer
128-
// must be non-null, and slices over non-ZSTs must also have a
129-
// non-null end pointer. The call to `next_unchecked!` is safe
130-
// since we check if the iterator is empty first.
127+
// SAFETY: `assume` call is safe because slices over non-ZSTs must
128+
// have a non-null end pointer. The call to `next_unchecked!` is
129+
// safe since we check if the iterator is empty first.
131130
unsafe {
132-
assume(!self.ptr.as_ptr().is_null());
133131
if !<T>::IS_ZST {
134132
assume(!self.end.is_null());
135133
}
@@ -339,12 +337,10 @@ macro_rules! iterator {
339337
fn next_back(&mut self) -> Option<$elem> {
340338
// could be implemented with slices, but this avoids bounds checks
341339

342-
// SAFETY: `assume` calls are safe since a slice's start pointer must be non-null,
343-
// and slices over non-ZSTs must also have a non-null end pointer.
344-
// The call to `next_back_unchecked!` is safe since we check if the iterator is
345-
// empty first.
340+
// SAFETY: `assume` call is safe because slices over non-ZSTs must
341+
// have a non-null end pointer. The call to `next_back_unchecked!`
342+
// is safe since we check if the iterator is empty first.
346343
unsafe {
347-
assume(!self.ptr.as_ptr().is_null());
348344
if !<T>::IS_ZST {
349345
assume(!self.end.is_null());
350346
}

tests/codegen/slice-iter-nonnull.rs

+42
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
// no-system-llvm
2+
// compile-flags: -O
3+
// ignore-debug (these add extra checks that make it hard to verify)
4+
#![crate_type = "lib"]
5+
6+
// The slice iterator used to `assume` that the `start` pointer was non-null.
7+
// That ought to be unneeded, though, since the type is `NonNull`, so this test
8+
// confirms that the appropriate metadata is included to denote that.
9+
10+
// CHECK-LABEL: @slice_iter_next(
11+
#[no_mangle]
12+
pub fn slice_iter_next<'a>(it: &mut std::slice::Iter<'a, u32>) -> Option<&'a u32> {
13+
// CHECK: %[[ENDP:.+]] = getelementptr{{.+}}ptr %it,{{.+}} 1
14+
// CHECK: %[[END:.+]] = load ptr, ptr %[[ENDP]]
15+
// CHECK-SAME: !nonnull
16+
// CHECK-SAME: !noundef
17+
// CHECK: %[[START:.+]] = load ptr, ptr %it,
18+
// CHECK-SAME: !nonnull
19+
// CHECK-SAME: !noundef
20+
// CHECK: icmp eq ptr %[[START]], %[[END]]
21+
22+
// CHECK: store ptr{{.+}}, ptr %it,
23+
24+
it.next()
25+
}
26+
27+
// CHECK-LABEL: @slice_iter_next_back(
28+
#[no_mangle]
29+
pub fn slice_iter_next_back<'a>(it: &mut std::slice::Iter<'a, u32>) -> Option<&'a u32> {
30+
// CHECK: %[[ENDP:.+]] = getelementptr{{.+}}ptr %it,{{.+}} 1
31+
// CHECK: %[[END:.+]] = load ptr, ptr %[[ENDP]]
32+
// CHECK-SAME: !nonnull
33+
// CHECK-SAME: !noundef
34+
// CHECK: %[[START:.+]] = load ptr, ptr %it,
35+
// CHECK-SAME: !nonnull
36+
// CHECK-SAME: !noundef
37+
// CHECK: icmp eq ptr %[[START]], %[[END]]
38+
39+
// CHECK: store ptr{{.+}}, ptr %[[ENDP]],
40+
41+
it.next_back()
42+
}

0 commit comments

Comments
 (0)