Make matrix layout invariants compile-time#29643
Open
GopalakrishnanN wants to merge 3 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR strengthens core/util/matrix_layout.h by making fixed-rank coordinate/layout helpers usable in constant expressions and by moving key invariants (positive ranks, nonnegative static dimensions, same-rank conversions) to compile-time checks. It also fixes the make_Position 3D factory to actually construct a Position<3> and adds compile-time + runtime tests to validate the behavior.
Changes:
- Make
Position,MatrixShape, and row/column-major layout helpersconstexprwhere applicable, and add compile-time invariant checks (static_assert). - Fix
make_Position(T,T,T)to returnPosition<3, T, L>directly (avoiding the prior rank-mismatch conversion path). - Add compile-time assertions and a small runtime smoke test covering layout mapping for runtime coordinates.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| onnxruntime/core/util/matrix_layout.h | Converts core coordinate/layout utilities to constexpr and enforces key invariants at compile time; fixes 3D make_Position. |
| onnxruntime/test/common/matrix_layout_test.cc | Adds compile-time assertions for Position/layout behavior plus a runtime coordinate mapping smoke test. |
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
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.
Description
Positionarithmetic, factories, and row/column layout mapping usable in constant expressions.make_Positionoverload to constructPosition<3>instead of converting fromPosition<2>.Motivation and Context
The matrix layout helpers describe fixed-rank metadata, but their invariants and arithmetic were previously evaluated at runtime. In addition, the three-dimensional factory constructed a two-dimensional position and converted it to rank three, which could read a nonexistent third element from the source.
Moving these operations and constraints to compile time catches invalid layouts during compilation while preserving the existing runtime API and generated layout arithmetic.
Validation:
.venv\Scripts\lintrunner.exe -a onnxruntime/core/util/matrix_layout.h onnxruntime/test/common/matrix_layout_test.cccmake --build build/Windows/RelWithDebInfo --config RelWithDebInfo --target onnxruntime_test_all --parallel 8onnxruntime_test_all.exe --gtest_filter="MatrixLayoutTest.*"(1 test passed)