Conversation
|
@r-devulap ,Please trigger the CI CD pipeline |
|
Hi @r-devulap , |
There was a problem hiding this comment.
Pull request overview
This PR makes the argselect function const-correct by accepting const T* for the input array parameter, similar to the earlier fix for argsort (issue #223). Since argselect only reads from the input array to compute index ordering without modifying it, this change improves API clarity and type safety.
Changes:
- Updated all
argselectfunction signatures to acceptconst T*instead ofT*for the input array parameter - Applied the established const_cast pattern in wrapper functions with explanatory comments
- Updated documentation in both README files to reflect the new const-correct signatures
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/xss-common-argsort.h | Added const_cast with safety comments in avx512_argselect and avx2_argselect wrapper functions |
| src/x86simdsort-static-incl.h | Updated both argselect API signatures to accept const T* |
| lib/x86simdsort.h | Updated public API signature to const T* |
| lib/x86simdsort.cpp | Updated internal function pointer and template instantiation signatures |
| lib/x86simdsort-skx.cpp | Updated AVX512 specialization signature |
| lib/x86simdsort-avx2.cpp | Updated AVX2 specialization signature |
| lib/x86simdsort-scalar.h | Updated scalar fallback implementation signature |
| lib/x86simdsort-internal.h | Updated internal API declaration |
| README.md | Updated documentation example with const-correct signature |
| src/README.md | Updated API documentation with const-correct signature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
r-devulap
left a comment
r-devulap
left a comment
There was a problem hiding this comment.
LGTM. Thanks @AnkitAhlawat7742
Fix: Make
argselectconst-correctSummary
Similar to
argsort, argselectonly reads from the input array—it computes index order based on element values without modifying the array itself. Updating argselect to accept aconst T*improves API clarity and prevents accidental modification of input data.For more details about
argsortwe can refer issue :- #223Changes
Updated argselect function signatures to accept
const T* instead of T*.CC @r-devulap