Add SVE-accelerated Vec::retain_mut for aarch64 - #161034
Conversation
| pub(crate) const MIN_SVE_LEN_2: usize = 32; | ||
| pub(crate) const MIN_SVE_LEN_4: usize = 64; | ||
| pub(crate) const MIN_SVE_LEN_8: usize = 1024; |
There was a problem hiding this comment.
- These should be pub(super).
- Why is there such a large difference between the constant for LEN_4 and LEN_8? In fact these constants don't even make sense, I would have excepted smaller sizes to have a larger minimum length.
- There should be a separate constant for size 1, for consistency. Or alternatively have a single constant for minimum byte size.
- These should use SIZE instead of LEN. LEN refers to the vector length rather than the element size.
There was a problem hiding this comment.
Half-Retain:
| Len | u8 SVE | u8 Scalar | Ratio | u16 SVE | u16 Scalar | Ratio | u32 SVE | u32 Scalar | Ratio | u64 SVE | u64 Scalar | Ratio |
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| 4 | 20.57 | 7.95 | 0.39x | 20.00 | 6.91 | 0.35x | 26.87 | 10.19 | 0.38x | 26.64 | 10.29 | 0.39x |
| 8 | 21.98 | 9.88 | 0.45x | 22.60 | 10.20 | 0.45x | 30.25 | 12.30 | 0.41x | 30.36 | 12.98 | 0.43x |
| 16 | 22.71 | 15.55 | 0.68x | 19.70 | 15.54 | 0.79x | 32.51 | 18.61 | 0.57x | 31.24 | 18.67 | 0.60x |
| 32 | 21.50 | 26.58 | 1.24x | 23.62 | 27.19 | 1.15x | 29.62 | 31.14 | 1.05x | 39.00 | 31.40 | 0.81x |
| 64 | 27.71 | 53.85 | 1.94x | 28.36 | 55.55 | 1.96x | 38.52 | 58.14 | 1.51x | 54.43 | 60.41 | 1.11x |
| 128 | 43.50 | 99.20 | 2.28x | 50.17 | 100.11 | 2.00x | 57.06 | 105.99 | 1.86x | 97.07 | 113.08 | 1.16x |
| 256 | 78.77 | 189.70 | 2.41x | 75.25 | 191.59 | 2.55x | 154.53 | 199.86 | 1.29x | 163.49 | 230.88 | 1.41x |
| 512 | 129.10 | 368.45 | 2.85x | 158.73 | 375.65 | 2.37x | 300.51 | 403.08 | 1.34x | 309.97 | 463.14 | 1.49x |
| 1,000 | 304.11 | 721.12 | 2.37x | 329.30 | 748.53 | 2.27x | 580.19 | 783.04 | 1.35x | 599.89 | 907.82 | 1.51x |
| 10,000 | 2,510.14 | 7,177.15 | 2.86x | 3,105.05 | 7,456.09 | 2.40x | 5,755.80 | 7,925.75 | 1.38x | 6,493.38 | 9,123.48 | 1.41x |
| 100,000 | 25,468.15 | 71,774.77 | 2.82x | 27,775.71 | 74,522.75 | 2.68x | 57,397.32 | 79,699.40 | 1.39x | 65,362.17 | 92,278.75 | 1.41x |
Whole-Retain:
| Len | u8 SVE | u8 Scalar | Ratio | u16 SVE | u16 Scalar | Ratio | u32 SVE | u32 Scalar | Ratio | u64 SVE | u64 Scalar | Ratio |
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| 4 | 18.81 | 2.76 | 0.15x | 15.43 | 2.76 | 0.18x | 15.56 | 2.76 | 0.18x | 15.75 | 2.76 | 0.18x |
| 8 | 17.10 | 4.48 | 0.26x | 16.92 | 4.33 | 0.26x | 17.26 | 4.32 | 0.25x | 19.72 | 4.32 | 0.22x |
| 16 | 17.92 | 8.08 | 0.45x | 17.85 | 8.29 | 0.46x | 19.45 | 8.08 | 0.42x | 19.28 | 8.13 | 0.42x |
| 32 | 21.52 | 15.97 | 0.74x | 20.28 | 15.82 | 0.78x | 20.16 | 15.55 | 0.77x | 27.13 | 15.49 | 0.57x |
| 64 | 21.79 | 31.00 | 1.42x | 21.23 | 30.70 | 1.45x | 25.98 | 30.63 | 1.18x | 36.78 | 31.15 | 0.85x |
| 128 | 35.84 | 60.51 | 1.69x | 38.42 | 60.62 | 1.58x | 41.78 | 60.92 | 1.46x | 71.65 | 61.05 | 0.85x |
| 256 | 68.08 | 130.79 | 1.92x | 71.29 | 130.79 | 1.83x | 81.51 | 130.80 | 1.60x | 135.93 | 130.78 | 0.96x |
| 512 | 124.97 | 250.00 | 2.00x | 146.74 | 250.01 | 1.70x | 203.80 | 250.03 | 1.23x | 323.93 | 249.99 | 0.77x |
| 1,000 | 247.01 | 469.68 | 1.90x | 267.12 | 469.35 | 1.76x | 317.79 | 469.36 | 1.48x | 622.36 | 469.58 | 0.75x |
| 10,000 | 2,485.53 | 4,611.28 | 1.86x | 4,060.09 | 4,646.64 | 1.14x | 3,752.13 | 4,611.98 | 1.23x | 6,266.50 | 4,818.49 | 0.77x |
| 100,000 | 23,721.95 | 48,340.95 | 2.04x | 27,156.28 | 46,246.36 | 1.70x | 34,872.15 | 46,536.10 | 1.33x | 54,606.28 | 47,030.48 | 0.86x |
This is the result on my aarch64 machine(without SVE2.1). LEN_8 should be 64.
I'm not sure if it has smaller sizes for u16 and u8 because of lack of SVE2.1
There was a problem hiding this comment.
And for Whole-Retain, long sequences of u64 have regression here. I'm not sure if it's related to my environment or there is no a fast-path like scalar.
This comment has been minimized.
This comment has been minimized.
bfdfd18 to
4f1beb7
Compare
|
@rustbot ready |
There was a problem hiding this comment.
I was thinking of waiting for #158312 to land first so that we can reduce the amount of inline asm needed, but I don't think that is necessary.
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
@bors r+ |
Add SVE-accelerated Vec::retain_mut for aarch64 The PR adds SVE support for specified width types(8, 16, 32 and 64 bits) in `Vec::retain_mut`. Due to [pointer provenance being stripped by intrinsics](https://rust-lang.zulipchat.com/#narrow/channel/208962-t-libs.2Fstdarch/topic/MaybeUninit.20lane.20variants.20for.20vector.20data-movement.20intrinsic/with/615526760)) here it has to use inline asm instead of sve intrinsics. ## 1. retain half (ns/iter) | Elements | u32 SVE | u32 scalar | Change | u64 SVE | u64 scalar | Change | |---|---|---|---|---|---|---| | 4 | 10.32 | 12.36 | / | 10.25 | 11.83 | / | | 8 | 12.93 | 15.08 | / | 13.40 | 14.74 | / | | 16 | 18.66 | 19.46 | / | 19.08 | 18.89 | / | | 32 | 31.84 | 31.12 |/ | 31.78 | 31.26 | / | | 64 | 32.99 | 59.17 | **-44.2%** | 59.86 | 59.44 | / | | 1,000 | 471.51 | 811.71 | **-41.9%** | 820.98 | 827.16 | / | | 10,000 | 4,660 | 7,990 | **-41.7%** | 5,836 | 8,242 | **-29.2%** | | 100,000 | 46,414 | 79,608 | **-41.7%** | 57,561 | 82,861 | **-30.5%** | ## 2. retain whole | Elements | u32 SVE | u32 scalar | Change | u64 SVE | u64 scalar | Change | |---|---|---|---|---|---|---| | 4 | 3.46 | 3.11 | / | 3.45 | 3.45 | / | | 8 | 4.90 | 4.49 | / | 4.83 | 6.22 | / | | 16 | 8.44 | 8.14 | / | 8.44 | 11.74 | / | | 32 | 15.83 | 15.51 | / | 15.83 | 22.79 | / | | 64 | 21.57 | 30.66 | **-29.6%** | 30.72 | 44.89 | / | | 1,000 | 358.53 | 483.33 | **-25.8%** | 469.18 | 696.43 | / | | 10,000 | 3,127 | 4,745 | **-34.1%** | 5,082 | 6,912 | **-26.5%** | | 100,000 | 32,509 | 49,501 | **-34.3%** | 49,484 | 79,749 | **-38.0%** | r? @Amanieu
Rollup of 7 pull requests Successful merges: - #150075 (Implement clamp_to) - #161034 (Add SVE-accelerated Vec::retain_mut for aarch64) - #161628 (interpret: ensure that calls via no-unwind ABIs do not unwind) - #161012 (borrowck: Normalize non-rigid aliases in NLL type relating) - #161702 (Use `drop_guard` in some places in {core,alloc,std}) - #161813 (Change `is_eligible_for_coverage` from a hook to a query) - #161842 (chore: fix cargo lints)
Add SVE-accelerated Vec::retain_mut for aarch64 The PR adds SVE support for specified width types(8, 16, 32 and 64 bits) in `Vec::retain_mut`. Due to [pointer provenance being stripped by intrinsics](https://rust-lang.zulipchat.com/#narrow/channel/208962-t-libs.2Fstdarch/topic/MaybeUninit.20lane.20variants.20for.20vector.20data-movement.20intrinsic/with/615526760)) here it has to use inline asm instead of sve intrinsics. ## 1. retain half (ns/iter) | Elements | u32 SVE | u32 scalar | Change | u64 SVE | u64 scalar | Change | |---|---|---|---|---|---|---| | 4 | 10.32 | 12.36 | / | 10.25 | 11.83 | / | | 8 | 12.93 | 15.08 | / | 13.40 | 14.74 | / | | 16 | 18.66 | 19.46 | / | 19.08 | 18.89 | / | | 32 | 31.84 | 31.12 |/ | 31.78 | 31.26 | / | | 64 | 32.99 | 59.17 | **-44.2%** | 59.86 | 59.44 | / | | 1,000 | 471.51 | 811.71 | **-41.9%** | 820.98 | 827.16 | / | | 10,000 | 4,660 | 7,990 | **-41.7%** | 5,836 | 8,242 | **-29.2%** | | 100,000 | 46,414 | 79,608 | **-41.7%** | 57,561 | 82,861 | **-30.5%** | ## 2. retain whole | Elements | u32 SVE | u32 scalar | Change | u64 SVE | u64 scalar | Change | |---|---|---|---|---|---|---| | 4 | 3.46 | 3.11 | / | 3.45 | 3.45 | / | | 8 | 4.90 | 4.49 | / | 4.83 | 6.22 | / | | 16 | 8.44 | 8.14 | / | 8.44 | 11.74 | / | | 32 | 15.83 | 15.51 | / | 15.83 | 22.79 | / | | 64 | 21.57 | 30.66 | **-29.6%** | 30.72 | 44.89 | / | | 1,000 | 358.53 | 483.33 | **-25.8%** | 469.18 | 696.43 | / | | 10,000 | 3,127 | 4,745 | **-34.1%** | 5,082 | 6,912 | **-26.5%** | | 100,000 | 32,509 | 49,501 | **-34.3%** | 49,484 | 79,749 | **-38.0%** | r? @Amanieu
Rollup of 6 pull requests Successful merges: - #161034 (Add SVE-accelerated Vec::retain_mut for aarch64) - #161628 (interpret: ensure that calls via no-unwind ABIs do not unwind) - #161012 (borrowck: Normalize non-rigid aliases in NLL type relating) - #161702 (Use `drop_guard` in some places in {core,alloc,std}) - #161813 (Change `is_eligible_for_coverage` from a hook to a query) - #161842 (chore: fix cargo lints)
|
⌛ Testing commit 78b1808 with merge 6003ac7... Workflow: https://github.com/rust-lang/rust/actions/runs/33039897300 |
Add SVE-accelerated Vec::retain_mut for aarch64 The PR adds SVE support for specified width types(8, 16, 32 and 64 bits) in `Vec::retain_mut`. Due to [pointer provenance being stripped by intrinsics](https://rust-lang.zulipchat.com/#narrow/channel/208962-t-libs.2Fstdarch/topic/MaybeUninit.20lane.20variants.20for.20vector.20data-movement.20intrinsic/with/615526760)) here it has to use inline asm instead of sve intrinsics. ## 1. retain half (ns/iter) | Elements | u32 SVE | u32 scalar | Change | u64 SVE | u64 scalar | Change | |---|---|---|---|---|---|---| | 4 | 10.32 | 12.36 | / | 10.25 | 11.83 | / | | 8 | 12.93 | 15.08 | / | 13.40 | 14.74 | / | | 16 | 18.66 | 19.46 | / | 19.08 | 18.89 | / | | 32 | 31.84 | 31.12 |/ | 31.78 | 31.26 | / | | 64 | 32.99 | 59.17 | **-44.2%** | 59.86 | 59.44 | / | | 1,000 | 471.51 | 811.71 | **-41.9%** | 820.98 | 827.16 | / | | 10,000 | 4,660 | 7,990 | **-41.7%** | 5,836 | 8,242 | **-29.2%** | | 100,000 | 46,414 | 79,608 | **-41.7%** | 57,561 | 82,861 | **-30.5%** | ## 2. retain whole | Elements | u32 SVE | u32 scalar | Change | u64 SVE | u64 scalar | Change | |---|---|---|---|---|---|---| | 4 | 3.46 | 3.11 | / | 3.45 | 3.45 | / | | 8 | 4.90 | 4.49 | / | 4.83 | 6.22 | / | | 16 | 8.44 | 8.14 | / | 8.44 | 11.74 | / | | 32 | 15.83 | 15.51 | / | 15.83 | 22.79 | / | | 64 | 21.57 | 30.66 | **-29.6%** | 30.72 | 44.89 | / | | 1,000 | 358.53 | 483.33 | **-25.8%** | 469.18 | 696.43 | / | | 10,000 | 3,127 | 4,745 | **-34.1%** | 5,082 | 6,912 | **-26.5%** | | 100,000 | 32,509 | 49,501 | **-34.3%** | 49,484 | 79,749 | **-38.0%** | r? @Amanieu
|
@bors yield to rollup |
|
Auto build was cancelled. Cancelled workflows: The next pull request likely to be tested is #161849. |
Rollup of 7 pull requests Successful merges: - #161034 (Add SVE-accelerated Vec::retain_mut for aarch64) - #161628 (interpret: ensure that calls via no-unwind ABIs do not unwind) - #161012 (borrowck: Normalize non-rigid aliases in NLL type relating) - #161691 (Assorted bootstrap config refactors (part 1/N)) - #161813 (Change `is_eligible_for_coverage` from a hook to a query) - #161842 (chore: fix cargo lints) - #161843 (rustdoc: fix lint `cargo::non_kebab_case_bins`)
Rollup merge of #161034 - ywxt:retian-sve, r=Amanieu Add SVE-accelerated Vec::retain_mut for aarch64 The PR adds SVE support for specified width types(8, 16, 32 and 64 bits) in `Vec::retain_mut`. Due to [pointer provenance being stripped by intrinsics](https://rust-lang.zulipchat.com/#narrow/channel/208962-t-libs.2Fstdarch/topic/MaybeUninit.20lane.20variants.20for.20vector.20data-movement.20intrinsic/with/615526760)) here it has to use inline asm instead of sve intrinsics. ## 1. retain half (ns/iter) | Elements | u32 SVE | u32 scalar | Change | u64 SVE | u64 scalar | Change | |---|---|---|---|---|---|---| | 4 | 10.32 | 12.36 | / | 10.25 | 11.83 | / | | 8 | 12.93 | 15.08 | / | 13.40 | 14.74 | / | | 16 | 18.66 | 19.46 | / | 19.08 | 18.89 | / | | 32 | 31.84 | 31.12 |/ | 31.78 | 31.26 | / | | 64 | 32.99 | 59.17 | **-44.2%** | 59.86 | 59.44 | / | | 1,000 | 471.51 | 811.71 | **-41.9%** | 820.98 | 827.16 | / | | 10,000 | 4,660 | 7,990 | **-41.7%** | 5,836 | 8,242 | **-29.2%** | | 100,000 | 46,414 | 79,608 | **-41.7%** | 57,561 | 82,861 | **-30.5%** | ## 2. retain whole | Elements | u32 SVE | u32 scalar | Change | u64 SVE | u64 scalar | Change | |---|---|---|---|---|---|---| | 4 | 3.46 | 3.11 | / | 3.45 | 3.45 | / | | 8 | 4.90 | 4.49 | / | 4.83 | 6.22 | / | | 16 | 8.44 | 8.14 | / | 8.44 | 11.74 | / | | 32 | 15.83 | 15.51 | / | 15.83 | 22.79 | / | | 64 | 21.57 | 30.66 | **-29.6%** | 30.72 | 44.89 | / | | 1,000 | 358.53 | 483.33 | **-25.8%** | 469.18 | 696.43 | / | | 10,000 | 3,127 | 4,745 | **-34.1%** | 5,082 | 6,912 | **-26.5%** | | 100,000 | 32,509 | 49,501 | **-34.3%** | 49,484 | 79,749 | **-38.0%** | r? @Amanieu
View all comments
The PR adds SVE support for specified width types(8, 16, 32 and 64 bits) in
Vec::retain_mut. Due to pointer provenance being stripped by intrinsics) here it has to use inline asm instead of sve intrinsics.1. retain half (ns/iter)
2. retain whole
r? @Amanieu