simplify slice::Iter[Mut]::next_chunk implementation - #158879
Conversation
This comment has been minimized.
This comment has been minimized.
|
i dont think theres any optimization problem here in anything but compile time, but i'm not sure as to the purpose of the pr? |
|
The purpose of the PR is to make the code simpler and easier to read/maintain. |
|
i feel as though its not exactly simpler, merely smaller. |
I verified that this doesn't pessimize codegen using the example from the
PR that inroduced the optimization of `next_chunk` (# 149131):
```rust
#![feature(iter_next_chunk)]
#[no_mangle]
pub fn simd_sum_slow(arr: &[u32]) -> u32 {
const STEP_SIZE: usize = 16;
let mut result = [0; STEP_SIZE];
let mut iter = arr.iter();
while let Ok(c) = iter.next_chunk::<STEP_SIZE>() {
for (&n, r) in c.iter().zip(result.iter_mut()) {
*r += n;
}
}
result.iter().sum()
}
```
I compiled this example with
```shell
./build/host/stage1/bin/rustc t.rs --emit=asm -O --crate-type=lib
```
Before and after this change; the only difference is the choice of the
jump instruction, which I think shouldn't make any difference:
```diff
28,29c28,29
< cmpq $64, %rsi
< jae .LBB0_2
---
> cmpq $60, %rsi
> ja .LBB0_2
```
|
I feel like it is simpler? It avoids the whole thing with copying things into a |
ac14155 to
aec1f2a
Compare
|
Let's perf-run just in case: |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
simplify `slice::Iter[Mut]::next_chunk` implementation
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (2277e36): comparison URL. Overall result: no relevant changes - no action neededBenchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up. @rustbot label: -S-waiting-on-perf -perf-regression Instruction countThis perf run didn't have relevant results for this metric. Max RSS (memory usage)Results (secondary -2.2%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (secondary -2.2%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis perf run didn't have relevant results for this metric. Bootstrap: 487.408s -> 488.179s (0.16%) |
|
@bors r+ |
Rollup of 11 pull requests Successful merges: - #158168 (Added implementation on `set_permissions_nofollow` for all primary platforms) - #160055 (Simplify `MaybeRequiresStorage`) - #157226 (Partially stabilize `box_vec_non_null`) - #158879 (simplify `slice::Iter[Mut]::next_chunk` implementation) - #159413 (Enable `#[diagnostic::on_unknown]` during late res) - #160091 (Fix rustdoc toolbar height when title is taller than one line) - #158615 (fix: don't fire `explicit_outlives_requirements` on `?Sized` type params) - #159666 (fix(ld64.lld): route version mismatch warnings to linker_info on macOS) - #160032 (rustdoc-json: Make `Stability` compatible with non-self-describing serde formats) - #160039 (Add regression test for enum unconstrained parameter ) - #160049 (Use assert_eq! in splat codegen tests)
Rollup of 11 pull requests Successful merges: - #158168 (Added implementation on `set_permissions_nofollow` for all primary platforms) - #160055 (Simplify `MaybeRequiresStorage`) - #157226 (Partially stabilize `box_vec_non_null`) - #158879 (simplify `slice::Iter[Mut]::next_chunk` implementation) - #159413 (Enable `#[diagnostic::on_unknown]` during late res) - #160091 (Fix rustdoc toolbar height when title is taller than one line) - #158615 (fix: don't fire `explicit_outlives_requirements` on `?Sized` type params) - #159666 (fix(ld64.lld): route version mismatch warnings to linker_info on macOS) - #160032 (rustdoc-json: Make `Stability` compatible with non-self-describing serde formats) - #160039 (Add regression test for enum unconstrained parameter ) - #160049 (Use assert_eq! in splat codegen tests)
Rollup merge of #158879 - WaffleLapkin:slice_iter_next_chunk_simp, r=JohnTitor simplify `slice::Iter[Mut]::next_chunk` implementation I verified that this doesn't pessimize codegen using the example from the PR that inroduced the optimization of `next_chunk` (#149131): ```rust #![feature(iter_next_chunk)] #[no_mangle] pub fn simd_sum_slow(arr: &[u32]) -> u32 { const STEP_SIZE: usize = 16; let mut result = [0; STEP_SIZE]; let mut iter = arr.iter(); while let Ok(c) = iter.next_chunk::<STEP_SIZE>() { for (&n, r) in c.iter().zip(result.iter_mut()) { *r += n; } } result.iter().sum() } ``` I compiled this example with ```shell ./build/host/stage1/bin/rustc t.rs --emit=asm -O --crate-type=lib ``` Before and after this change; the only difference is the choice of the jump instruction, which I think shouldn't make any difference: ```diff 28,29c28,29 < cmpq $64, %rsi < jae .LBB0_2 --- > cmpq $60, %rsi > ja .LBB0_2 ``` r? libs cc @bend-n
I verified that this doesn't pessimize codegen using the example from the PR that inroduced the optimization of
next_chunk(#149131):I compiled this example with
Before and after this change; the only difference is the choice of the jump instruction, which I think shouldn't make any difference:
r? libs
cc @bend-n