-
Notifications
You must be signed in to change notification settings - Fork 100
Adding further aie4 shim test support #895
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR extends AIE4 support in the shim test suite by introducing a new debug buffer type (XRT_BO_USE_UC_DEBUG) and broadening test coverage from AIE2-specific to general AIE platform support. The changes enable debug buffer operations to work correctly on AIE4 devices while maintaining backward compatibility with AIE2.
- Introduced XRT_BO_USE_UC_DEBUG buffer type for AIE4 debug operations
- Extended test coverage from AIE2-only to all AIE platforms for several test cases
- Updated buffer creation logic to conditionally use appropriate debug flags based on device type
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| test/shim_test/shim_test.cpp | Conditionally selects debug buffer flags based on AIE4 detection and broadens test filters from AIE2-specific to general AIE support |
| src/shim/buffer.cpp | Adds mapping for new XRT_BO_USE_UC_DEBUG buffer type to firmware debug buffer type |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
xdavidz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With this, can we get a pass when run "shim_test" without any index number?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Currently yes, but there are a few more tests that I think I can enable with minimal changes before we merge. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
retest this please |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
We should enable all shim_test and just run shim_test without any args. Are there any shim_test that we decide to run for aie2 only? If not many, we should enable all possible shim_test for aie4. |
Yes with this PR we can pass 38 shim tests on aie4 by running "./shim_test.elf" without any arguments, the rest are skipped. There are a few tests that will stay as aie2 only, more will be enabled for aie4 after this PR but I did not want to do every change all in one PR that would be too large. |
src/shim/umq/hwq.h
Outdated
|
|
||
| // Cache for command buffer headers (firmware corrupts them) | ||
| // Key: BO handle, Value: cached header word | ||
| std::map<uint32_t, uint32_t> m_cmd_header_cache; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may not work well. The exec buf can be reused by different applications...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| hwq->wait_command(cbo->get(), 0); | ||
| auto cmd_hdl = cbo->get(); | ||
| auto cmd_pkt = reinterpret_cast<ert_start_kernel_cmd *>(cbo->map()); | ||
|
|
Copilot
AI
Jan 13, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Blank line with only whitespace. Remove trailing whitespace for consistency with coding standards.
| auto cbo = m_bo_array[IO_TEST_BO_CMD].tbo.get(); | ||
| auto chdl = cbo->get(); | ||
| auto cpkt = reinterpret_cast<ert_start_kernel_cmd *>(cbo->map()); | ||
|
|
Copilot
AI
Jan 13, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Blank line with only whitespace. Remove trailing whitespace for consistency with coding standards.
| } | ||
| return 0; | ||
| } | ||
|
|
Copilot
AI
Jan 13, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Blank line with only whitespace. Remove trailing whitespace for consistency with coding standards.
Signed-off-by: Hayden Laccabue <hlaccabu@amd.com>
Signed-off-by: Hayden Laccabue <hlaccabu@amd.com>
Signed-off-by: Hayden Laccabue <hlaccabu@amd.com>
Signed-off-by: Hayden Laccabue <hlaccabu@amd.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
We successfully confused AI :-) |

No description provided.