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. |
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