Commit 28928c7
committed
Auto merge of rust-lang#77617 - AnthonyMikh:slice_windows_no_bounds_checking, r=lcnr
Eliminate bounds checking in slice::Windows
This is how `<core::slice::Windows as Iterator>::next` looks right now:
```rust
fn next(&mut self) -> Option<&'a [T]> {
if self.size > self.v.len() {
None
} else {
let ret = Some(&self.v[..self.size]);
self.v = &self.v[1..];
ret
}
}
```
The line with `self.v = &self.v[1..];` relies on assumption that `self.v` is definitely not empty at this point. Else branch is taken when `self.size <= self.v.len()`, so `self.v` can be empty if `self.size` is zero. In practice, since `Windows` is never created directly but rather trough `[T]::windows` which panics when `size` is zero, `self.size` is never zero. However, the compiler doesn't know about this check, so it keeps the code which checks bounds and panics.
Using `NonZeroUsize` lets the compiler know about this invariant and reliably eliminate bounds checking without `unsafe` on `-O2`. Here is assembly of `Windows<'a, u32>::next` before and after this change ([goldbolt](https://godbolt.org/z/xrefzx)):
<details>
<summary>Before</summary>
```
example::next:
push rax
mov rcx, qword ptr [rdi + 8]
mov rdx, qword ptr [rdi + 16]
cmp rdx, rcx
jbe .LBB0_2
xor eax, eax
pop rcx
ret
.LBB0_2:
test rcx, rcx
je .LBB0_5
mov rax, qword ptr [rdi]
mov rsi, rax
add rsi, 4
add rcx, -1
mov qword ptr [rdi], rsi
mov qword ptr [rdi + 8], rcx
pop rcx
ret
.LBB0_5:
lea rdx, [rip + .L__unnamed_1]
mov edi, 1
xor esi, esi
call qword ptr [rip + core::slice::slice_index_order_fail@GOTPCREL]
ud2
.L__unnamed_2:
.ascii "./example.rs"
.L__unnamed_1:
.quad .L__unnamed_2
.asciz "\f\000\000\000\000\000\000\000\016\000\000\000\027\000\000"
```
</details>
<details>
<summary>After</summary>
```
example::next:
mov rcx, qword ptr [rdi + 8]
mov rdx, qword ptr [rdi + 16]
cmp rdx, rcx
jbe .LBB0_2
xor eax, eax
ret
.LBB0_2:
mov rax, qword ptr [rdi]
lea rsi, [rax + 4]
add rcx, -1
mov qword ptr [rdi], rsi
mov qword ptr [rdi + 8], rcx
ret
```
</details>
Note the lack of call to `core::slice::slice_index_order_fail` in second snippet.
#### Possible reasons _not_ to merge this PR:
* this changes the error message on panic in `[T]::windows`. However, AFAIK this messages are not covered by backwards compatibility policy.File tree
3 files changed
+52
-15
lines changed- library/core/src/slice
- src/test/codegen
3 files changed
+52
-15
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
10 | 10 | | |
11 | 11 | | |
12 | 12 | | |
| 13 | + | |
13 | 14 | | |
14 | 15 | | |
15 | 16 | | |
| |||
1187 | 1188 | | |
1188 | 1189 | | |
1189 | 1190 | | |
1190 | | - | |
| 1191 | + | |
1191 | 1192 | | |
1192 | 1193 | | |
1193 | 1194 | | |
1194 | 1195 | | |
1195 | | - | |
| 1196 | + | |
1196 | 1197 | | |
1197 | 1198 | | |
1198 | 1199 | | |
| |||
1211 | 1212 | | |
1212 | 1213 | | |
1213 | 1214 | | |
1214 | | - | |
| 1215 | + | |
1215 | 1216 | | |
1216 | 1217 | | |
1217 | | - | |
| 1218 | + | |
1218 | 1219 | | |
1219 | 1220 | | |
1220 | 1221 | | |
1221 | 1222 | | |
1222 | 1223 | | |
1223 | 1224 | | |
1224 | 1225 | | |
1225 | | - | |
| 1226 | + | |
1226 | 1227 | | |
1227 | 1228 | | |
1228 | | - | |
| 1229 | + | |
1229 | 1230 | | |
1230 | 1231 | | |
1231 | 1232 | | |
| |||
1237 | 1238 | | |
1238 | 1239 | | |
1239 | 1240 | | |
1240 | | - | |
| 1241 | + | |
1241 | 1242 | | |
1242 | 1243 | | |
1243 | 1244 | | |
| |||
1250 | 1251 | | |
1251 | 1252 | | |
1252 | 1253 | | |
1253 | | - | |
| 1254 | + | |
1254 | 1255 | | |
1255 | 1256 | | |
1256 | | - | |
| 1257 | + | |
1257 | 1258 | | |
1258 | 1259 | | |
1259 | 1260 | | |
| |||
1264 | 1265 | | |
1265 | 1266 | | |
1266 | 1267 | | |
1267 | | - | |
| 1268 | + | |
1268 | 1269 | | |
1269 | 1270 | | |
1270 | 1271 | | |
1271 | 1272 | | |
1272 | 1273 | | |
1273 | 1274 | | |
1274 | 1275 | | |
1275 | | - | |
| 1276 | + | |
1276 | 1277 | | |
1277 | 1278 | | |
1278 | | - | |
| 1279 | + | |
1279 | 1280 | | |
1280 | 1281 | | |
1281 | 1282 | | |
| |||
1284 | 1285 | | |
1285 | 1286 | | |
1286 | 1287 | | |
1287 | | - | |
| 1288 | + | |
1288 | 1289 | | |
1289 | 1290 | | |
1290 | 1291 | | |
1291 | | - | |
| 1292 | + | |
1292 | 1293 | | |
1293 | 1294 | | |
1294 | 1295 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
11 | 11 | | |
12 | 12 | | |
13 | 13 | | |
| 14 | + | |
14 | 15 | | |
15 | 16 | | |
16 | 17 | | |
| |||
751 | 752 | | |
752 | 753 | | |
753 | 754 | | |
754 | | - | |
| 755 | + | |
755 | 756 | | |
756 | 757 | | |
757 | 758 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
0 commit comments