Refactor: extract shared compact kernel machinery to common header - #29
Merged
Conversation
Rename the ormqr implementation files to the project's per-routine naming scheme, matching geqrf/potrf/trsm: cqr_compact.hpp -> cqr_ormqr_compact.hpp cqr_compact_dispatch.cpp -> cqr_ormqr_compact_dispatch.cpp cqr_mkl_ext.cpp -> cqr_mkl_ormqr.cpp test_cqr_compact.cpp -> test_cqr_ormqr_compact.cpp test_cqr_mkl_ext.cpp -> test_cqr_ormqr_mkl.cpp The shared C API headers (cqr_mkl_ext.h, cqr_compact.h) and the cqr_mkl_ext library target keep their umbrella names. Split the common helpers and the packed vector type out of the old cqr_compact.hpp into a new cqr_compact_common.hpp, shared by every routine header: pack<T,V>, vsqrt, broadcast, BatchView, and the make_view/make_const_view factories. cqr_ormqr_compact.hpp now holds only the ormqr-specific Direction enum and kernels and includes the common header the same way geqrf/potrf/trsm already do. Update the CMake sources and ctest names (portable_kernel -> portable_ormqr, mkl_compact_suites -> mkl_ormqr_suites) and the README/PLANS references. Verified: full MKL build (11/11 ctest) and portable -DCQR_WITH_MKL=OFF build (4/4 ctest) pass; clang-format clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CbYs39BXPvFUejjDq7kYsD
The MKL adapter switches on MKL_COMPACT_PACK directly (like potrf's row already states) rather than unwrapping it to a runtime interleave width V; align the file-table description with the code. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CbYs39BXPvFUejjDq7kYsD
BatchView::operator() cast both indices to size_t before scaling by the strides, forcing the strided sweep's address arithmetic to 64-bit. The batch and matrix dimensions all fit in the kernel's Int (the compact format targets many small matrices), so type the element strides as Int and compute i*special + p*panel in Int -- no widening of the loop induction variables, keeping the strided sweep vectorizable. make_view / make_const_view take Int strides to match, and the four routine drivers drop their (size_t) element-stride casts. The large per-group base offset stays size_t and is applied to the pointer before the view is built. Mark operator() inline. No behavior change; full MKL suite (11/11) and portable -DCQR_WITH_MKL=OFF suite (4/4) pass, clang-format clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CbYs39BXPvFUejjDq7kYsD
Both MKL adapters switch on MKL_COMPACT_PACK directly, like potrf and ormqr; drop the older "unwraps -> V" wording so all four cqr_mkl_*.cpp file-table rows describe the dispatch the same way. Each row still names the specific modes its adapter maps (geqrf: MKL_LAYOUT; trsm: the full MKL enum set). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CbYs39BXPvFUejjDq7kYsD
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR refactors the compact (interleaved-batch) kernel infrastructure by extracting shared machinery into a dedicated common header file. This eliminates code duplication across multiple kernel implementations (geqrf, potrf, ormqr, trsm) and improves maintainability.
Key Changes
New file
cqr_compact_common.hpp: Centralizes shared machinery previously duplicated in individual kernel headers:pack<T,V>template: the V-wide SIMD element type with GNU vector attributesvsqrt()andbroadcast()lane-wise vector helpersBatchViewtemplate: strided 2-D view for interleaved matrix groupsmake_view()andmake_const_view()factory functionsRenamed and refactored kernel headers:
cqr_compact.hpp→cqr_ormqr_compact.hpp(now ormqr-specific)pack,BatchView,vsqrt,broadcast#include "cqr_compact_common.hpp"to import shared typesUpdated other kernel headers to include the common header:
cqr_geqrf_compact.hppcqr_potrf_compact.hppcqr_trsm_compact.hppRenamed dispatch and implementation files for clarity:
cqr_compact_dispatch.cpp→cqr_ormqr_compact_dispatch.cppcqr_mkl_ext.cpp→cqr_mkl_ormqr.cpptest_cqr_compact.cpp→test_cqr_ormqr_compact.cpptest_cqr_mkl_ext.cpp→test_cqr_ormqr_mkl.cppUpdated CMakeLists.txt and documentation (README.md, PLANS.md) to reflect new file names and structure
Implementation Details
The refactoring maintains full API and ABI compatibility while improving code organization. The common header is included by all compact kernel implementations, ensuring consistent definitions of the V-wide pack type and addressing machinery across geqrf, potrf, ormqr, and trsm kernels. File naming now explicitly indicates which routine each file implements (e.g.,
cqr_ormqr_compact.hppvs genericcqr_compact.hpp).https://claude.ai/code/session_01CbYs39BXPvFUejjDq7kYsD