feat: add Ascend950 direct SDMA backend - #84
Conversation
|
Superseded by #85, rebuilt directly on origin/main so the PR contains no CCU stack commits. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 454cb7ffce
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const aclError destroyStatus = isolated == nullptr | ||
| ? ACL_SUCCESS | ||
| : aclrtDestroyContext(isolated); | ||
| isolated = nullptr; |
There was a problem hiding this comment.
Retain isolated contexts when destruction fails
If aclrtDestroyContext(isolated) returns an error, this unconditionally discards the only handle to the potentially live query context. The caller then falls back without SDMA and cannot retry cleanup, so repeated communicator initialization under a transient runtime cleanup failure can leak contexts and eventually exhaust device resources, undermining the required best-effort fallback. Keep the handle until destruction succeeds or transfer it to retryable backend state.
AGENTS.md reference: AGENTS.md:L51-L51
Useful? React with 👍 / 👎.
| for bytes in "${SIZES[@]}"; do | ||
| echo "---- bytes=${bytes} ----" | ||
| "${bin}" "${bytes}" | ||
| for ((repeat = 1; repeat <= REPEATS; ++repeat)); do |
There was a problem hiding this comment.
Reject zero repeat counts before running the matrix
When TILEXR_SDMA_DEMO_REPEATS is 0 or negative, this loop executes no demo invocation and the runner exits successfully, allowing hardware automation to record a passing SDMA matrix without testing any transfer. Validate REPEATS as a positive integer before entering the loop.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| std::cout << "PASS TileXR SDMA copied " << bytes << " bytes correctly" << std::endl; | ||
| std::cout << "PASS TileXR SDMA copied " << bytes << " bytes on " << blocks |
There was a problem hiding this comment.
Fail the demo when communicator teardown fails
The new A5 teardown path can make TileXRCommDestroy return an error while retaining the communicator, but Cleanup discards that result and this path still prints PASS and returns zero. Thus an unmap, stream-destroy, or context-restore failure can be reported as a successful lifecycle validation before the process resets the device, preventing the promised retry; propagate cleanup status into the demo result.
Useful? React with 👍 / 👎.
| std::vector<uint8_t> hostSrc(bytes); | ||
| std::vector<uint8_t> hostDst(bytes, 0); | ||
| std::vector<int32_t> hostDebug(kDebugWords, 0); | ||
| std::vector<uint8_t> hostSrc(totalBytes); |
There was a problem hiding this comment.
Give each source block a distinct validation pattern
For the new multi-block mode, FillPattern repeats every 256 bytes, so documented block sizes such as 4 KiB give every source block identical contents. A source-address or block-routing defect that copies block 0 into another block's destination can therefore still satisfy the full-buffer equality check; seed the pattern with the block index or otherwise make each block distinct.
Useful? React with 👍 / 👎.
Summary
Validation
Remaining Validation
Stack