Cleanup a5 host_build_graph header guards and stale docs - #1670
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe PR updates Args Dump documentation for payload-based tensor metadata and renames include guards in A5 host-build-graph headers. Runtime structures, constants, and APIs remain unchanged. ChangesArgs Dump and A5 runtime alignment
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/dfx/args-dump.md`:
- Line 70: In the bash example in the a5 host_build_graph section, keep the
explanatory text as a comment and move the pytest invocation to its own
following line so it executes when copied.
- Around line 308-314: Update the documentation paragraphs around
host_build_graph, the corresponding section near lines 452-459, and the section
near lines 875-877 to state that tensor arguments provide metadata when existing
Tensors are passed. Describe TensorCreateInfo as the metadata source
specifically for allocated tensor outputs, rather than implying it is required
for every add_input, add_output, or add_inout call.
- Around line 872-874: Update the diagnosis text around
try_log_dump_args_layout_mismatch to apply only when tensors are expected,
explicitly treating tensor_count == 0 as valid for scalar-only add_scalar tasks.
Replace the stale tensor_info reference with the current
PTO2TaskPayload::tensors[] terminology and describe the issue as tensor-missing
rather than a generic payload inconsistency.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 81a8bff1-58d1-4b24-bf4f-492ad57a6dcc
📒 Files selected for processing (12)
docs/dfx/args-dump.mdsrc/a5/runtime/host_build_graph/common/pto_runtime_status.hsrc/a5/runtime/host_build_graph/runtime/aicore_completion_mailbox.hsrc/a5/runtime/host_build_graph/runtime/aicore_completion_mailbox_types.hsrc/a5/runtime/host_build_graph/runtime/dep_gen_host_graph.hsrc/a5/runtime/host_build_graph/runtime/host_tensor_access.hsrc/a5/runtime/host_build_graph/runtime/pto_completion_token.hsrc/a5/runtime/host_build_graph/runtime/pto_constants.hsrc/a5/runtime/host_build_graph/runtime/pto_dep_compute.hsrc/a5/runtime/host_build_graph/runtime/pto_runtime2_types.hsrc/a5/runtime/host_build_graph/runtime/pto_types.hsrc/a5/runtime/host_build_graph/runtime/runtime.h
ed0acc3 to
c1a736d
Compare
The 13 files under src/a5/runtime/host_build_graph/ that still used #ifndef SRC_A2A3_* / #define / #endif guards are replaced with #pragma once, matching the other half of the tree that already uses it. Also removes the two remaining docs/dfx/args-dump.md references to the old host_build_graph ABI — the deleted tensor_info.h / set_tensor_info_to_task API and the non-existent examples/a5/host_build_graph/ path. Four platforms compile clean; a5sim 35 passed; a2a3sim 17 passed / 4 skipped.
Summary
Two mechanical post-#1661 cleanups in one commit.
Header guard rename
The a5 tree was a verbatim copy of a2a3, so 33 header guards still spelled
SRC_A2A3_*and 9 still namedTENSORMAP_AND_RINGBUFFERrather thanHOST_BUILD_GRAPH. Rename toSRC_A5_RUNTIME_HOST_BUILD_GRAPHto match the actual path. 13 files, each with 3 consistent guard occurrences (ifndef / define / endif).docs/dfx/args-dump.md — stale host_build_graph references
Two references described the old
add_task/set_tensor_info_to_taskABI that #1661 deleted:examples/a5/host_build_graph/vector_example, which does not exist (a5 hbg has no examples). Replaced with the actual scene test path.TensorInfowiring andtensor_info.h, both gone. Rewritten to describe the post-convergence world where tensor metadata comes fromPTO2TaskPayload.No behaviour change — guard rename is a no-op, and the doc edit only touches prose.
Testing