Skip to content

Add ctx tests for aie4 + fix get_bo_usage#1127

Open
hlaccabu wants to merge 1 commit intoamd:mainfrom
hlaccabu:shim_context
Open

Add ctx tests for aie4 + fix get_bo_usage#1127
hlaccabu wants to merge 1 commit intoamd:mainfrom
hlaccabu:shim_context

Conversation

@hlaccabu
Copy link
Contributor

No description provided.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the shim test suite to better cover AIE4/NPU3 devices and makes BO usage querying resilient to multiple accel nodes by discovering the correct /dev/accel/* node from sysfs.

Changes:

  • Add an NPU3 device filter and new multi-context IO test cases for NPU3.
  • Rework get_bo_usage() to find the device’s accel node via sysfs instead of hardcoding /dev/accel/accel0.
  • Expand/adjust several existing test case device filters to run on all XDNA/AIE devices (including AIE4).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Signed-off-by: Hayden Laccabue <hlaccabu@amd.com>
Copilot AI review requested due to automatic review settings February 26, 2026 22:38
Copy link
Contributor

Copilot AI left a 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 1 out of 1 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

closedir(dp);

if (accel_node.empty()) {
std::cerr << "Failed to find accel node under "<< accel_path << " for device" << std::endl;
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing space after "under" in the error message. The output will be "Failed to find accel node under/path/to/accel for device" instead of "Failed to find accel node under /path/to/accel for device".

Copilot uses AI. Check for mistakes.
Comment on lines +265 to +279
auto dp = opendir(accel_path.c_str());
if (!dp) {
std::perror(("opendir: " + accel_path).c_str());
return {0, 0, 0};
}

std::string accel_node;
while (auto entry = readdir(dp)) {
std::string dirname{entry->d_name};
if (dirname.find("accel") == 0) {
accel_node = "/dev/accel/" + dirname;
break;
}
}
closedir(dp);
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The directory handle 'dp' could potentially leak if an exception is thrown between opendir (line 265) and closedir (line 279). While unlikely with the current operations, consider using RAII (e.g., a smart pointer with a custom deleter or a wrapper class) to ensure the directory is always closed, even in exceptional circumstances.

Copilot uses AI. Check for mistakes.
@amd amd deleted a comment from Copilot AI Feb 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants