BUG: Mirror VQSORT_ENABLED logic in Quicksort #27050
Conversation
This patch disables Highway VQSort if the same criteria is met as sort/shared-inl.h, to prevent it aborting at runtime. I'm unsure whether this would look neater using Highway's dynamic dispatch.
|
When I compile with ASAN, I still see a missing symbol error at import: |
Argh, my Highway branch was updated for another thing, sorry about that! Just synced with what's in |
|
Nice, this does fix the problem - I can run the sorting tests under ASAN locally using this branch. @r-devulap you may also want to look this over, but I'll probably merge this in the next day or two assuming the tests pass. |
| // This replicates VQSORT_ENABLED from hwy/contrib/sort/shared-inl.h | ||
| // without checking the scalar target as this is not built within the dynamic | ||
| // dispatched sources. | ||
| #if (HWY_COMPILER_MSVC && !HWY_IS_DEBUG_BUILD) || \ |
There was a problem hiding this comment.
why not just use VQSORT_ENABLED instead duplicating the macro?
There was a problem hiding this comment.
This is included by quicksort.cpp, which isn't dynamically dispatched, meaning the VQSORT_ENABLED flag would be set incorrectly due to having HWY_TARGET == HWY_SCALAR.
|
Note to self: wait on this one. |
This patch disables Highway VQSort if the same criteria is met as sort/shared-inl.h, to prevent it aborting at runtime.
I'm unsure whether this would look neater using Highway's dynamic dispatch.