Add SVE implementation of replace#6195
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
|
Something is wrong with this PR in the MSVC-internal repo again and I need to pull it from the merge batch. This is breaking the ARM64-native (arm64chk) compiler such that it ICEs. On a D32ps_v6 VM, the error is uninformative: @cdacamar helped out tremendously by getting more repro details, and our partners at Arm are now investigating. |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
|
I just read (in MSVC-PR-757601) that "The ARM64EC ABI does not include the SVE register state (z/p/FFR are not preserved across EC/x64 transitions - armasm64 rejects them with A4266)", so should we be activating this for original recipe ARM64 only and not extra crispy ARM64EC? |
I am just consulting with people more in the know regarding ARM64EC than I, but as far as I understand https://learn.microsoft.com/en-us/windows/arm/arm64ec-abi does indeed imply that SVE Z and P registers are forbidden entirely. So I guess the fix is to remove |
But can't we keep Neon for |
|
Yes, my understanding is that ARM64EC supports NEON, so we shouldn't mess with that. I believe that we should (thanks @AlexGuteniev for remembering the tests):
|
Here is another page with explicit table: https://learn.microsoft.com/en-us/cpp/build/arm64ec-windows-abi-conventions?view=msvc-170#register-mapping-for-integer-registers I'm wondering if the decision not to support SVE at all on ARM64EC is permanent in long term, or it may be lifted in a near future. |
This PR adds an SVE implementation of
replace. This algorithm was previously not vectorized using Neon, due to the absence of masked stores in the instruction set. See #4433 for why this is an issue.Benchmark results ⏲️
Results are speedup values relative to the existing C code as a baseline - higher is better. Benchmark results were obtained running on a Neoverse N2 machine.
r<std::uint8_t>r<std::uint16_t>r<std::uint32_t>r<std::uint64_t>