Skip to content

fix #6294 missing bytesize check for safe vectorization with LLVM - #1

Closed
mgovers wants to merge 1 commit into
mainfrom
GH006294-llvm-vectorization-is-safe
Closed

fix #6294 missing bytesize check for safe vectorization with LLVM#1
mgovers wants to merge 1 commit into
mainfrom
GH006294-llvm-vectorization-is-safe

Conversation

@mgovers

@mgovers mgovers commented May 26, 2026

Copy link
Copy Markdown
Owner

Fixes microsoft#6294

Here's my attempt at excluding odd-sized and large-sized structs from the vectorization in xutility. I have tried to follow all the conventions used elsewhere as much as possible, in particular:

  • for the implementation, I followed what was already there and the changes from Vectorize find-like algorithms for Clang for more types microsoft/STL#5767
  • for the tests, following microsoft@3da7d27
    • NOTE: I explicitly scoped out equivalent tests as for _Equal_memcmp_is_safe as the current main implementation of _Vector_alg_in_find_is_safe was not tested in the first place. I still opted to add a couple more tests than for _Vector_alg_in_search_is_safe to at the very least show that there's vectorization for MSVC and not only for Clang
  • Since _Vector_alg_in_search_is_safe already had a similar check as what was needed here, I re-used that one (had to move it outside the #ifdef statement, though)
    • I did multiple searches for strings like _VECTORIZED_FIND, static_assert(false, "unexpected size");, _vectorized( and _Nx <= 8 && (_Nx & (_Nx - 1)) == 0 to see if there were other cases that I missed. I believe (and hope) I got them all (at least in algorithm and xutility).
    • I explicitly did not include MINMAX-like things, because they are independently explicitly guarded where they are used.

Additional remarks/open questions

  • I am running all tests locally as we speak (adding this just in case I accidentally create the PR before the run is finished)
  • What is the best way (if any) to add the test case from <xutility>: std::find no longer compiles with structs with defaulted equality comparison with Clang-CL microsoft/STL#6294 ?
    • Is it worth the effort to create a very small test file that contains exactly that edge case?
    • Or should I add it to the one I already added?
  • Should I benchmark? Or will that be done on your end?
    • (cfr. For performance-sensitive code (e.g. containers and algorithms) you may wish to write and/or run benchmarks, and the STL team will likely run any benchmarks we do have in our PR process.)
    • It is touching sensitive code (very common algorithms) and explicitly disabling vectorization (which is, in fact, a bugfix and it should only be excluding things that never compiled in the first place, but still...)
… LLVM

Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
@mgovers mgovers closed this May 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

1 participant