test(sync): add minimal A5 TMOV col-major alignment repro#440
Open
TaoTao-real wants to merge 2 commits intohw-native-sys:mainfrom
Open
test(sync): add minimal A5 TMOV col-major alignment repro#440TaoTao-real wants to merge 2 commits intohw-native-sys:mainfrom
TaoTao-real wants to merge 2 commits intohw-native-sys:mainfrom
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces new test samples and regression guards for TMOV alignment on the A5 architecture, specifically covering 16x1 column-major and 1x16 row-major tile configurations. The changes include new .pto and .py test files and updates to the runop.sh test runner to validate the emitted C++ code. A critical issue was identified in the test runner where an undefined variable target_arch_lc would cause the new tests to be incorrectly skipped; a suggestion was provided to use a consistent inline transformation for the architecture check.
| fi | ||
| if [[ ( "$base" == "test_tmov_col_major_16x1_align_a5" || \ | ||
| "$base" == "test_tmov_row_major_1x16_control_a5" ) && \ | ||
| "${target_arch_lc}" != "a5" ]]; then |
There was a problem hiding this comment.
The variable target_arch_lc is not defined in this script. This will cause the condition to always evaluate to true (as an undefined variable expands to an empty string), resulting in these tests being skipped even when the target architecture is correctly set to a5. You should use the inline transformation consistent with the rest of the file to check the architecture.
Suggested change
| "${target_arch_lc}" != "a5" ]]; then | |
| "$(printf '%s' "$target_arch" | tr '[:upper:]' '[:lower:]')" != "a5" ]]; then |
Contributor
Author
|
/run a5 test_tmov_col_major_16x1_align_a5 test_tmov_row_major_1x16_control_a5 --pto-level=level3 |
A5 板测失败
日志尾部 |
Contributor
Author
|
/run a5 test_tmov_col_major_16x1_align_a5 test_tmov_row_major_1x16_control_a5 --pto-level=level3 |
A5 板测失败
日志尾部 |
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
TMOVon16x1 f32 col_majortile intest/samples/Sync/test_tmov_col_major_16x1_align_a5.{py,pto}.test/samples/Sync/test_tmov_row_major_1x16_control_a5.{py,pto}.test/samples/runop.sh:--pto-archis nota5.TMOVand expected tile layout (ColMajor 16x1/RowMajor 1x16).Why
Provide a focused TMOV-only pair to isolate A5 runtime behavior in the suspected col-major alignment-sensitive path.
Validation
bash -n test/samples/runop.shptoas --pto-arch=a5 --enable-insert-syncon both samples.TMOV+Tile<...,16,1,BLayout::ColMajor,...>TMOV+Tile<...,1,16,BLayout::RowMajor,...>