feat: add direct CCU runtime support#52
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 217dbddb85
ℹ️ 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".
| remote.memoryTokenId = peerWindow.tokenId; | ||
| remote.rawMemoryTokenId = peerWindow.rawTokenId; | ||
| remote.memoryTokenValue = peerWindow.tokenValue; | ||
| remote.remoteEid = ReverseEndpointEid(peerWindow.remoteEid); |
There was a problem hiding this comment.
When peer-route import is unavailable or disabled (for example env/provider/collector routes or TILEXR_CCU_DIRECT_ENDPOINT_ROUTE_EXCHANGE_MODE=exported_peer), peerWindow.remoteEid is already the peer's exported endpoint EID and this fallback value is passed on to lower-layer channel construction. Reversing it here byte-swaps verified routes before TileXRCcuBuildLowerLayerTransportTemplate consumes them, so the generated channel specs address the wrong endpoint; only transform the EID on the path that actually requires the imported-route byte order.
Useful? React with 👍 / 👎.
| class TileXRCcuDirectRuntime { | ||
| public: | ||
| int Init(const TileXRCcuDirectRuntimeOptions& options, TileXRCcuDirectRuntimeReport* report); | ||
| void Shutdown(); |
There was a problem hiding this comment.
When RA-ctx resource-window registration or RA-ctx endpoint collection has created local memory registrations, QPs, CQs, or imported peer QPs, destroying TileXRComm only destroys the unique_ptr and this class has no destructor that calls Shutdown(). That means ReleaseRegisteredResourceWindow()/ReleaseLocalEndpointRoute() never run before the loader unloads, leaking RA resources until process exit and potentially exhausting handles across communicator lifetimes.
Useful? React with 👍 / 👎.
| static_cast<uint32_t>(channelBoundRemoteXnId) < | ||
| static_cast<uint32_t>(peerResources.remoteXnStartId) + peerResources.remoteXnCount && | ||
| routeIndex < allocation.channels.num && | ||
| peerResources.channelStartId != 0 && |
There was a problem hiding this comment.
When the allocated channel window starts at 0, which TileXRCcuResourceAllocator::Init accepts and zero-initialized public options naturally request, this extra nonzero check marks channelResourceOwnerVerified false even though the peer channel range check succeeds. The install provider later rejects those proofs as an unverified channel owner, so valid zero-based channel allocations can never become submitReady.
Useful? React with 👍 / 👎.
| ((spec.doorbellTokenValue >> 20U) & 0xfffU) | | ||
| ((static_cast<uint32_t>(Log2PowerOfTwo(wqeBasicBlocks)) & 0xfU) << 12U))); | ||
| Store16(ctx->raw, 22, static_cast<uint16_t>( | ||
| (static_cast<uint32_t>(spec.wqeBasicBlockStartId) & 0xfU) << 12U)); | ||
| Store16(ctx->raw, 24, static_cast<uint16_t>((spec.wqeBasicBlockStartId >> 4U) & 0xffU)); |
There was a problem hiding this comment.
When an endpoint provider/env route supplies a large sqDepth whose sqDepth * 4 needs more than the 4 encoded bits, or hcomm_cap mode creates wqeBasicBlockStartId >= 4096 (for example 17+ routes at stride 256), these stores mask/truncate the values while returning success. The emitted local-jetty context then points at the wrong SQ/WQE window; validate those ranges before packing.
Useful? React with 👍 / 👎.
| uint16_t remoteNotifyCke = | ||
| static_cast<uint16_t>(static_cast<uint32_t>(peerResources.localWaitCkeStartId) + | ||
| peerLocalWaitCkeOffset); |
There was a problem hiding this comment.
When there are more than two ranks, this uses the local route index as an offset into the peer's localWaitCke window, but each peer orders its wait CKEs by that peer's own peer list. For example rank 0 targeting rank 2 chooses rank 2's wait CKE slot 1 even though rank 2's slot for rank 0 is 0; the proof is still marked verified, so the installed barrier can notify the wrong CKE and hang.
Useful? React with 👍 / 👎.
# Conflicts: # tests/comm/unit/test_tilexr_source_guards.cpp
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8e63d9e1e3
ℹ️ 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".
| << ", raInitialized " << (runtimeReport.raInitialized ? 1 : 0) | ||
| << ", ccuTlvInitialized " << (runtimeReport.ccuTlvInitialized ? 1 : 0) | ||
| << ", " << runtimeReport.message << ", direct CCU disabled"; | ||
| g_ccuDirectRuntimeUnavailable = true; |
There was a problem hiding this comment.
When one rank hits this init-failure path, the unavailable state is recorded only in that process; other ranks in other processes can still enter PrepareDirectCcuInstallAttempt and proceed to the socket allgathers in ExportRemoteCcuRmaBuffers, while the failed rank returns before participating. In a multi-process job where CCU runtime init fails on just one rank/device, prepare can hang instead of failing collectively, so exchange the availability/error state across ranks before any rank skips the direct-CCU collectives.
Useful? React with 👍 / 👎.
| TileXRCcuEndpointTpHandleExchange localTpHandles {}; | ||
| std::vector<TileXRCcuEndpointTpHandleExchange> allTpHandles; | ||
| bool peerTpHandlesReady = false; | ||
| if (canImportPeerRoutes) { |
There was a problem hiding this comment.
When canImportPeerRoutes differs by rank (for example one rank lacks a verified endpoint route/QP key or has a different exchange-mode env), only ranks that evaluate true enter this options_.allGather; ranks that skip it continue to later exchanges, so the socket allgather sequence diverges and can block or consume the wrong payload. Make this exchange unconditional with an explicit capability/empty handle, or collectively decide import availability before any rank skips the TP-handle allgather.
Useful? React with 👍 / 👎.
| program->sqeLoad.clear(); | ||
| program->sync.clear(); | ||
| if (plan.taskWindows.size() > 1) { | ||
| if (AppendSqeLoadProgram(plan, TILEXR_CCU_SQE_ARGS_LEN, &program->sqeLoad, report) != TILEXR_SUCCESS) { |
There was a problem hiding this comment.
When callers request a partial SQE load (sqeArgCount from 1 to 12), the allocator accepts it and sizes the SQE task window to that count, but this always emits 13 load instructions. The repository builder then rejects the otherwise valid plan because the SQE task window no longer matches the generated microcode, so use the requested task-window count here or reject partial counts before allocation.
Useful? React with 👍 / 👎.
1 participant
Summary
src/comm/ccu.tilexr_api.handcomm_wrap.cpp.tests/ccu.Test plan
git diff --cached --checkbefore commitPATH="/tmp/tilexr-python-bin:$PATH" python3 -m unittest tests.ccu.test_tilexr_ccu_direct_smoke_probe tests.ccu.test_tilexr_ccu_public_comm_api tests.ccu.test_tilexr_ccu_direct_smoke_runner tests.ccu.test_tilexr_ccu_direct_orchestrator tests.ccu.test_tilexr_ccu_memory_program tests.ccu.test_tilexr_ccu_microcodepython3 -m unittest discover tests/ccuis not clean on this macOS host; failures are local environment/platform gaps: missing3rdparty/hcommreference source, Apple linker handling of-Wl,--gc-sections, and Linux dynamic-library behavior differences.