Refactor: share HBG Graph Execution across architectures - #1813
Open
zmnobug wants to merge 1 commit into
Open
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:
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (19)
💤 Files with no reviewable changes (2)
📝 WalkthroughWalkthroughGraph cache, graph execution, host state, documentation, and tests now use shared ChangesShared host-build-graph implementation
Estimated code review effort: 4 (Complex) | ~60 minutes 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/common/host_build_graph/graph_execution.cpp (1)
464-474: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winValidate the node argument counts before you assign them to the payload.
Lines 464-465 write
source.tensor_countandsource.scalar_countinto the payload. Lines 466-474 then validate the same values and returnGraphMaterializeResult::INVALID. On the failure path the payload keeps out-of-range counts, andconstructed_nodeswas already incremented at line 426. Move the validation above the assignments so a rejected node never publishes an out-of-range count.♻️ Proposed reordering
slot.graph_context = &execution; - payload.tensor_count = source.tensor_count; - payload.scalar_count = source.scalar_count; if (source.tensor_count < 0 || source.tensor_count > MAX_TENSOR_ARGS || source.scalar_count < 0 || source.scalar_count > MAX_SCALAR_ARGS || static_cast<uint32_t>(source.tensor_count) > definition.tensor_arg_count || static_cast<uint32_t>(source.scalar_count) > definition.scalar_arg_count || source.tensor_offset > definition.tensor_arg_count - static_cast<uint32_t>(source.tensor_count) || source.scalar_offset > definition.scalar_arg_count - static_cast<uint32_t>(source.scalar_count)) { execution.materialize_busy.store(0, std::memory_order_release); return GraphMaterializeResult::INVALID; } + payload.tensor_count = source.tensor_count; + payload.scalar_count = source.scalar_count;🤖 Prompt for 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. In `@src/common/host_build_graph/graph_execution.cpp` around lines 464 - 474, Move the argument-count validation in the node materialization flow before assigning source.tensor_count and source.scalar_count to payload. Ensure invalid nodes return GraphMaterializeResult::INVALID without publishing out-of-range payload counts, while preserving the existing validation conditions and failure handling.tests/ut/cpp/common/test_hbg_graph_cache.cpp (1)
12-26: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd
<utility>forstd::as_const.The test configuration already requires C++17. Add the declaring header directly.
🤖 Prompt for 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. In `@tests/ut/cpp/common/test_hbg_graph_cache.cpp` around lines 12 - 26, Add the <utility> header to the includes in test_hbg_graph_cache.cpp so std::as_const has its direct declaration available under C++17.
🤖 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 `@src/common/host_build_graph/graph_execution.cpp`:
- Around line 432-434: Update synthetic ID generation in the graph execution
code around outer_slot.task->task_id.local() so the full outer local ID is
preserved and distinct outer IDs cannot alias. Replace the current fixed 10-bit
packing with a collision-free encoding, or explicitly validate and enforce the
allocator’s required bit-width before constructing PTO2TaskId::make.
- Around line 485-510: Refresh every dynamic boundary descriptor field during
affine replay, not only buffer address data. Extend GraphTensorAddressPatch and
the replay logic around the BOUNDARY_EXACT and BOUNDARY_VIEW handling to
propagate current start_offset, version, owner_task_id, address_space, and
relevant buffer fields from execution.boundary_tensors; alternatively include
these fields in GraphBoundarySignature so stale affine reuse is rejected.
---
Nitpick comments:
In `@src/common/host_build_graph/graph_execution.cpp`:
- Around line 464-474: Move the argument-count validation in the node
materialization flow before assigning source.tensor_count and
source.scalar_count to payload. Ensure invalid nodes return
GraphMaterializeResult::INVALID without publishing out-of-range payload counts,
while preserving the existing validation conditions and failure handling.
In `@tests/ut/cpp/common/test_hbg_graph_cache.cpp`:
- Around line 12-26: Add the <utility> header to the includes in
test_hbg_graph_cache.cpp so std::as_const has its direct declaration available
under C++17.
🪄 Autofix
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: 57868e54-22d8-48ca-b7df-01228020b1c3
📒 Files selected for processing (19)
src/a2a3/runtime/host_build_graph/build_config.pysrc/a2a3/runtime/host_build_graph/docs/GRAPH_EXECUTION.mdsrc/a2a3/runtime/host_build_graph/runtime/graph_cache.hsrc/a2a3/runtime/host_build_graph/runtime/graph_execution.hsrc/a2a3/runtime/host_build_graph/runtime/graph_host_state.hsrc/a5/runtime/host_build_graph/build_config.pysrc/a5/runtime/host_build_graph/docs/GRAPH_EXECUTION.mdsrc/a5/runtime/host_build_graph/runtime/graph_cache.hsrc/a5/runtime/host_build_graph/runtime/graph_execution.hsrc/a5/runtime/host_build_graph/runtime/graph_host_state.hsrc/a5/runtime/host_build_graph/runtime/scheduler/graph_execution.cppsrc/common/host_build_graph/docs/GRAPH_EXECUTION.mdsrc/common/host_build_graph/graph_cache.hsrc/common/host_build_graph/graph_execution.cppsrc/common/host_build_graph/graph_execution.hsrc/common/host_build_graph/graph_host_state.htests/ut/cpp/CMakeLists.txttests/ut/cpp/a5/test_graph_cache.cpptests/ut/cpp/common/test_hbg_graph_cache.cpp
💤 Files with no reviewable changes (2)
- tests/ut/cpp/a5/test_graph_cache.cpp
- src/a5/runtime/host_build_graph/runtime/scheduler/graph_execution.cpp
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/common/host_build_graph/graph_execution.cpp (1)
464-474: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winValidate the node argument counts before you assign them to the payload.
Lines 464-465 write
source.tensor_countandsource.scalar_countinto the payload. Lines 466-474 then validate the same values and returnGraphMaterializeResult::INVALID. On the failure path the payload keeps out-of-range counts, andconstructed_nodeswas already incremented at line 426. Move the validation above the assignments so a rejected node never publishes an out-of-range count.♻️ Proposed reordering
slot.graph_context = &execution; - payload.tensor_count = source.tensor_count; - payload.scalar_count = source.scalar_count; if (source.tensor_count < 0 || source.tensor_count > MAX_TENSOR_ARGS || source.scalar_count < 0 || source.scalar_count > MAX_SCALAR_ARGS || static_cast<uint32_t>(source.tensor_count) > definition.tensor_arg_count || static_cast<uint32_t>(source.scalar_count) > definition.scalar_arg_count || source.tensor_offset > definition.tensor_arg_count - static_cast<uint32_t>(source.tensor_count) || source.scalar_offset > definition.scalar_arg_count - static_cast<uint32_t>(source.scalar_count)) { execution.materialize_busy.store(0, std::memory_order_release); return GraphMaterializeResult::INVALID; } + payload.tensor_count = source.tensor_count; + payload.scalar_count = source.scalar_count;🤖 Prompt for 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. In `@src/common/host_build_graph/graph_execution.cpp` around lines 464 - 474, Move the argument-count validation in the node materialization flow before assigning source.tensor_count and source.scalar_count to payload. Ensure invalid nodes return GraphMaterializeResult::INVALID without publishing out-of-range payload counts, while preserving the existing validation conditions and failure handling.tests/ut/cpp/common/test_hbg_graph_cache.cpp (1)
12-26: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd
<utility>forstd::as_const.The test configuration already requires C++17. Add the declaring header directly.
🤖 Prompt for 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. In `@tests/ut/cpp/common/test_hbg_graph_cache.cpp` around lines 12 - 26, Add the <utility> header to the includes in test_hbg_graph_cache.cpp so std::as_const has its direct declaration available under C++17.
🤖 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 `@src/common/host_build_graph/graph_execution.cpp`:
- Around line 432-434: Update synthetic ID generation in the graph execution
code around outer_slot.task->task_id.local() so the full outer local ID is
preserved and distinct outer IDs cannot alias. Replace the current fixed 10-bit
packing with a collision-free encoding, or explicitly validate and enforce the
allocator’s required bit-width before constructing PTO2TaskId::make.
- Around line 485-510: Refresh every dynamic boundary descriptor field during
affine replay, not only buffer address data. Extend GraphTensorAddressPatch and
the replay logic around the BOUNDARY_EXACT and BOUNDARY_VIEW handling to
propagate current start_offset, version, owner_task_id, address_space, and
relevant buffer fields from execution.boundary_tensors; alternatively include
these fields in GraphBoundarySignature so stale affine reuse is rejected.
---
Nitpick comments:
In `@src/common/host_build_graph/graph_execution.cpp`:
- Around line 464-474: Move the argument-count validation in the node
materialization flow before assigning source.tensor_count and
source.scalar_count to payload. Ensure invalid nodes return
GraphMaterializeResult::INVALID without publishing out-of-range payload counts,
while preserving the existing validation conditions and failure handling.
In `@tests/ut/cpp/common/test_hbg_graph_cache.cpp`:
- Around line 12-26: Add the <utility> header to the includes in
test_hbg_graph_cache.cpp so std::as_const has its direct declaration available
under C++17.
🪄 Autofix
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: 57868e54-22d8-48ca-b7df-01228020b1c3
📒 Files selected for processing (19)
src/a2a3/runtime/host_build_graph/build_config.pysrc/a2a3/runtime/host_build_graph/docs/GRAPH_EXECUTION.mdsrc/a2a3/runtime/host_build_graph/runtime/graph_cache.hsrc/a2a3/runtime/host_build_graph/runtime/graph_execution.hsrc/a2a3/runtime/host_build_graph/runtime/graph_host_state.hsrc/a5/runtime/host_build_graph/build_config.pysrc/a5/runtime/host_build_graph/docs/GRAPH_EXECUTION.mdsrc/a5/runtime/host_build_graph/runtime/graph_cache.hsrc/a5/runtime/host_build_graph/runtime/graph_execution.hsrc/a5/runtime/host_build_graph/runtime/graph_host_state.hsrc/a5/runtime/host_build_graph/runtime/scheduler/graph_execution.cppsrc/common/host_build_graph/docs/GRAPH_EXECUTION.mdsrc/common/host_build_graph/graph_cache.hsrc/common/host_build_graph/graph_execution.cppsrc/common/host_build_graph/graph_execution.hsrc/common/host_build_graph/graph_host_state.htests/ut/cpp/CMakeLists.txttests/ut/cpp/a5/test_graph_cache.cpptests/ut/cpp/common/test_hbg_graph_cache.cpp
💤 Files with no reviewable changes (2)
- tests/ut/cpp/a5/test_graph_cache.cpp
- src/a5/runtime/host_build_graph/runtime/scheduler/graph_execution.cpp
🛑 Comments failed to post (2)
src/common/host_build_graph/graph_execution.cpp (2)
432-434: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Description: Inspect PTO2TaskId packing and the local-id field width. set -euo pipefail fd -t f -g 'pto_task_id.h' | xargs -r cat -n echo '--- make/local definitions ---' ast-grep run --pattern 'struct PTO2TaskId { $$$ }' --lang cpp . || true rg -nP --type=cpp -C 4 '\blocal\s*\(\s*\)\s*const|static\s+\w*\s*PTO2TaskId\s+make\s*\('Repository: hw-native-sys/simpler
Length of output: 2936
🏁 Script executed:
#!/bin/bash set -euo pipefail echo '--- graph_execution.cpp target region ---' cat -n src/common/host_build_graph/graph_execution.cpp | sed -n '390,555p' echo '--- relevant constants and task-id uses ---' rg -n -C 3 --type=cpp \ 'GRAPH_MAX_NODES|synthetic_local|owner_task_id|task_id\s*=\s*PTO2TaskId::make|outer_slot\.task->task_id|local_task_id|local_id' \ src/common src/a2a3 src/a5 echo '--- task-id headers and graph task structures ---' fd -t f \( -g '*task*id*.h' -o -g '*types*.h' -o -g '*graph*.h' \) src/common src/a2a3 src/a5 \ | sort | head -80Repository: hw-native-sys/simpler
Length of output: 50378
🏁 Script executed:
#!/bin/bash set -euo pipefail echo '--- graph_execution.cpp ---' cat -n src/common/host_build_graph/graph_execution.cpp | sed -n '400,545p' echo '--- graph constants and declarations ---' rg -n -C 5 --type=cpp --type=c \ 'GRAPH_MAX_NODES|struct GraphExecution|class GraphExecution|synthetic_local|owner_task_id' \ src/common/host_build_graph src/common/platform/include echo '--- all PTO2TaskId definitions and make calls in the common graph path ---' rg -n -C 2 --type=cpp --type=c \ 'struct PTO2TaskId|PTO2TaskId::make|task_id\.local\(\)' \ src/common/host_build_graph src/common/task_interface src/common/platform/includeRepository: hw-native-sys/simpler
Length of output: 27902
🏁 Script executed:
#!/bin/bash set -euo pipefail echo '--- materialization function and callers ---' rg -n -C 8 --type=cpp \ 'materialize|outer_slot|GraphMaterializeResult' \ src/common/host_build_graph/graph_execution.cpp \ src/common/host_build_graph/*.h \ src/a2a3/runtime/host_build_graph \ src/a5/runtime/host_build_graph echo '--- local task-id allocation limits ---' rg -n -C 5 --type=cpp --type=c \ 'local_task_id_|initial_local_task_id|next_local|task_id\(\)|alloc_result\.task_id|PTO2_RING.*SIZE|TASK_WINDOW|task_window_size' \ src/a2a3/runtime/tensormap_and_ringbuffer \ src/a5/runtime/tensormap_and_ringbuffer \ src/common/host_build_graph echo '--- task-id packing verifier ---' python3 - <<'PY' MASK32 = (1 << 32) - 1 def synthetic(outer_local, node_index): return ((outer_local << 10) | node_index) & MASK32 for a, b in [(0, 1 << 22), (1, 1 << 22 | 7), (0xFFFFFFFF, 0x3FFFFF)]: print(f'outer={a:`#x`}, node={b & 1023}: synthetic={synthetic(a, b & 1023):`#x`}') print('injective for outer locals iff outer_local < 2^22 (with node_index < 2^10)') PYRepository: hw-native-sys/simpler
Length of output: 50378
🏁 Script executed:
#!/bin/bash set -euo pipefail echo '--- graph materialization symbols ---' rg -n --type=cpp \ 'GraphMaterializeResult|materialize_graph|materialize_nodes|outer_slot' \ src/common/host_build_graph/graph_execution.cpp \ src/common/host_build_graph/graph_execution.h echo '--- graph materialization callers ---' rg -n -C 6 --type=cpp \ 'graph_materialize|materialize_graph|materialize_nodes' \ src/a2a3/runtime/host_build_graph src/a5/runtime/host_build_graph echo '--- allocator declarations and allocation path ---' rg -n -C 4 --type=cpp \ 'class PTO2TaskAllocator|struct PTO2TaskAllocator|local_task_id_|alloc\(|task_id\(\)' \ src/a2a3/runtime/tensormap_and_ringbuffer/runtime/pto_ring_buffer.h \ src/a5/runtime/tensormap_and_ringbuffer/runtime/pto_ring_buffer.h echo '--- task-window configuration bounds ---' rg -n -C 4 --type=cpp \ 'PTO2_TASK_WINDOW_SIZE|PTO2_RING_TASK_WINDOW|window_size.*<=|window_size.*INT32|task_window_size.*<' \ src/a2a3/runtime/host_build_graph src/a5/runtime/host_build_graph \ src/a2a3/runtime/tensormap_and_ringbuffer src/a5/runtime/tensormap_and_ringbufferRepository: hw-native-sys/simpler
Length of output: 5348
🏁 Script executed:
#!/bin/bash set -euo pipefail echo '--- graph localization and materialization context ---' cat -n src/common/host_build_graph/graph_execution.cpp | sed -n '270,375p' rg -n -C 8 'graph_execution_materialize_slice|graph_execution_localize' src echo '--- allocator implementation ---' rg -n -C 5 \ 'PTO2TaskAllocator|local_task_id_|AllocResult|task_id\(\)|PTO2_TASK_WINDOW_SIZE' \ src/a2a3/runtime/tensormap_and_ringbuffer/runtime/pto_ring_buffer.h \ src/a5/runtime/tensormap_and_ringbuffer/runtime/pto_ring_buffer.h echo '--- runtime task-window validation ---' rg -n -C 5 \ 'PTO2_TASK_WINDOW_SIZE|PTO2_RING_TASK_WINDOW|task_window_size|window_size_' \ src/a2a3/runtime/tensormap_and_ringbuffer src/a5/runtime/tensormap_and_ringbufferRepository: hw-native-sys/simpler
Length of output: 50378
🏁 Script executed:
#!/bin/bash set -euo pipefail echo '--- graph execution caller search ---' rg -n -C 5 'graph_execution_materialize_slice|graph_execution_localize' src/common src/a2a3/runtime/host_build_graph src/a5/runtime/host_build_graph || true echo '--- allocator symbol locations ---' rg -n 'PTO2TaskAllocator|local_task_id_|AllocResult|task_id\(\)' \ src/a2a3/runtime/tensormap_and_ringbuffer/runtime/pto_ring_buffer.h \ src/a5/runtime/tensormap_and_ringbuffer/runtime/pto_ring_buffer.h || true echo '--- a2a3 allocator header ---' cat -n src/a2a3/runtime/tensormap_and_ringbuffer/runtime/pto_ring_buffer.h | sed -n '1,180p' cat -n src/a2a3/runtime/tensormap_and_ringbuffer/runtime/pto_ring_buffer.h | sed -n '180,380p' echo '--- a5 allocator header ---' cat -n src/a5/runtime/tensormap_and_ringbuffer/runtime/pto_ring_buffer.h | sed -n '1,180p' cat -n src/a5/runtime/tensormap_and_ringbuffer/runtime/pto_ring_buffer.h | sed -n '180,380p'Repository: hw-native-sys/simpler
Length of output: 50377
🏁 Script executed:
#!/bin/bash set -euo pipefail python3 - <<'PY' MASK32 = (1 << 32) - 1 NODE_MASK = (1 << 10) - 1 def synthetic_local(outer_local, node_index): return ((outer_local << 10) | node_index) & MASK32 outer_a = 0 outer_b = 1 << 22 for node_index in (0, 1, 1023): a = synthetic_local(outer_a, node_index) b = synthetic_local(outer_b, node_index) print(f'node={node_index}: {a:`#010x`} == {b:`#010x`}: {a == b}') PY echo '--- no 22-bit bound near outer task allocation ---' rg -n -C 3 \ 'local_task_id_|commit_task\(\)|task_id = local_task_id_\+\+' \ src/a2a3/runtime/tensormap_and_ringbuffer/runtime/pto_ring_buffer.h \ src/a5/runtime/tensormap_and_ringbuffer/runtime/pto_ring_buffer.hRepository: hw-native-sys/simpler
Length of output: 19629
Preserve the full outer local ID in synthetic node IDs.
The 32-bit shift by 10 discards the outer ID's top 10 bits. The allocator does not enforce a 22-bit limit. Therefore outer IDs
0and1 << 22produce identical node IDs, which can aliasowner_task_idand dependency identity. Use a collision-free encoding or enforce the required bound.🤖 Prompt for 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. In `@src/common/host_build_graph/graph_execution.cpp` around lines 432 - 434, Update synthetic ID generation in the graph execution code around outer_slot.task->task_id.local() so the full outer local ID is preserved and distinct outer IDs cannot alias. Replace the current fixed 10-bit packing with a collision-free encoding, or explicitly validate and enforce the allocator’s required bit-width before constructing PTO2TaskId::make.
485-510: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Description: Trace boundary signature and graph key construction against the affine replay contract. set -euo pipefail rg -n --type=cpp -C 6 'GraphBoundarySignature' echo '--- graph key / full_key construction ---' rg -n --type=cpp -C 6 'full_key|rt_graph_make_key|content_hash\s*=' echo '--- start_offset participation in keys or signatures ---' rg -n --type=cpp -C 4 'start_offset' | rg -n -C 4 'hash|signature|key' || trueRepository: hw-native-sys/simpler
Length of output: 159
🏁 Script executed:
#!/bin/bash set -euo pipefail echo '--- candidate files ---' git ls-files | rg '(^|/)(graph_execution|host_build_graph|graph).*\\.(cpp|h|hpp)$' | head -200 echo '--- symbols and key construction ---' rg -n -S -C 5 \ 'GraphBoundarySignature|reset_execution|full_key|rt_graph_make_key|content_hash|affine|BOUNDARY_(EXACT|VIEW)|start_offset|owner_task_id|version|payload\.tensors' \ src/common/host_build_graph \ --glob '*.{cpp,h,hpp,cc,cxx}' || trueRepository: hw-native-sys/simpler
Length of output: 183
🏁 Script executed:
#!/bin/bash set -euo pipefail echo '--- tracked graph-related files ---' git ls-files | rg -i 'graph|host_build_graph' | head -300 echo '--- exact implementation and header ---' wc -l src/common/host_build_graph/graph_execution.cpp src/common/host_build_graph/graph_execution.h cat -n src/common/host_build_graph/graph_execution.cpp | sed -n '1,120p;450,620p' cat -n src/common/host_build_graph/graph_execution.h | sed -n '1,220p' echo '--- relevant symbol references ---' rg -n -S -C 6 \ 'GraphBoundarySignature|reset_execution|full_key|rt_graph_make_key|content_hash|BOUNDARY_(EXACT|VIEW)|start_offset|owner_task_id|payload\.tensors' \ src --glob '*.{cpp,h,hpp,cc,cxx}' | head -1000Repository: hw-native-sys/simpler
Length of output: 50378
🏁 Script executed:
#!/bin/bash set -euo pipefail echo '--- common host graph symbol locations ---' rg -n -S \ 'GraphBoundarySignature|boundary_signatures|content_hash|full_key|graph_key|rt_graph_make_key|graph_cache|definition_affine_reuse|GraphTensorAddressPatch' \ src/common/host_build_graph src/common/task_interface \ --glob '*.{cpp,h,hpp,cc,cxx}' | head -500 echo '--- graph execution implementation sections ---' cat -n src/common/host_build_graph/graph_execution.cpp | sed -n '120,450p' cat -n src/common/host_build_graph/graph_execution.h | sed -n '220,467p' echo '--- graph cache API and implementation ---' cat -n src/common/host_build_graph/graph_cache.h | sed -n '1,260p'Repository: hw-native-sys/simpler
Length of output: 42508
🏁 Script executed:
#!/bin/bash set -euo pipefail echo '--- all signature and submission producers ---' rg -n -S -C 5 \ 'GraphBoundarySignature|graph_tensor_pack|GraphTensor\s*\{|boundary_tensors|tensors_offset|definition->full_key|rt_graph_make_key|materialized_graph_key|materialized_definition_hash' \ . \ --glob '!build/**' --glob '!dist/**' --glob '!node_modules/**' | head -1200 echo '--- graph execution reuse tests and documentation ---' rg -n -S -C 6 \ 'reuse|reus|affine|graph_key|content_hash|boundary|start_offset|owner_task_id|version' \ tests/ut/cpp/common tests/st/host_build_graph_validation src/common/host_build_graph/docs \ --glob '*.{cpp,h,hpp,py,md}' | head -1200 echo '--- deterministic gate/patch probe ---' python3 - <<'PY' old = { "buffer_addr": 0x1000, "buffer_size": 4096, "owner_task_id": 7, "start_offset": 8, "version": 1, } new = {**old, "start_offset": 32, "version": 2, "owner_task_id": 9} # These are the fields used by reset_execution's affine gate. gate = ( 123 != 0 and 123 == 123 and 456 == 456 and 1 == 1 and 2 <= 2 and 3 >= 3 ) # This is the complete affine update in graph_execution.cpp. replayed = {**old, "buffer_addr": new["buffer_addr"]} print("affine_gate_for_same_graph_and_definition:", gate) print("new_dynamic_fields:", {k: new[k] for k in ("start_offset", "version", "owner_task_id")}) print("replayed_dynamic_fields:", {k: replayed[k] for k in ("start_offset", "version", "owner_task_id")}) print("stale_fields:", [k for k in ("start_offset", "version", "owner_task_id") if replayed[k] != new[k]]) PYRepository: hw-native-sys/simpler
Length of output: 50378
🏁 Script executed:
#!/bin/bash set -euo pipefail for f in \ src/a2a3/runtime/host_build_graph/runtime/orchestrator_core/pto_orchestrator.cpp \ src/a5/runtime/host_build_graph/runtime/orchestrator_core/pto_orchestrator.cpp do echo "===== $f: source matching and signature construction =====" rg -n -S 'graph_tensor_exact|graph_tensor_from_boundary|graph_boundary_signature|graph_build_definition|graph_validate|signatures\[|signature\.' "$f" sed -n '350,490p' "$f" sed -n '590,690p' "$f" sed -n '1300,1405p' "$f" sed -n '1400,1450p' "$f" done echo '--- isolated probe output ---' python3 - <<'PY' old = {"buffer_addr": 0x1000, "owner_task_id": 7, "start_offset": 8, "version": 1} new = {**old, "start_offset": 32, "version": 2, "owner_task_id": 9} gate = (123 != 0 and 123 == 123 and 456 == 456 and 1 == 1 and 2 <= 2 and 3 >= 3) replayed = {**old, "buffer_addr": new["buffer_addr"]} print("affine_gate_for_same_graph_and_definition:", gate) print("new_dynamic_fields:", {k: new[k] for k in ("start_offset", "version", "owner_task_id")}) print("replayed_dynamic_fields:", {k: replayed[k] for k in ("start_offset", "version", "owner_task_id")}) print("stale_fields:", [k for k in ("start_offset", "version", "owner_task_id") if replayed[k] != new[k]]) PYRepository: hw-native-sys/simpler
Length of output: 46202
Refresh all dynamic boundary fields during affine replay.
GraphBoundarySignaturedoes not coverstart_offset,version,owner_task_id, oraddress_space, so these fields can change without disabling affine reuse. Lines 567-579 update onlybuffer.addr; replay can therefore retain stale descriptor state and address the wrong element range. ExtendGraphTensorAddressPatchand replay to copy the current boundary fields, or include them in the affine-reuse gate.🤖 Prompt for 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. In `@src/common/host_build_graph/graph_execution.cpp` around lines 485 - 510, Refresh every dynamic boundary descriptor field during affine replay, not only buffer address data. Extend GraphTensorAddressPatch and the replay logic around the BOUNDARY_EXACT and BOUNDARY_VIEW handling to propagate current start_offset, version, owner_task_id, address_space, and relevant buffer fields from execution.boundary_tensors; alternatively include these fields in GraphBoundarySignature so stale affine reuse is rejected.
zmnobug
force-pushed
the
refactor/issue-1737-deduplicate-hbg-graph-execution
branch
from
August 13, 2026 03:02
b216e44 to
dc5ff9d
Compare
This was referenced Aug 13, 2026
zmnobug
force-pushed
the
refactor/issue-1737-deduplicate-hbg-graph-execution
branch
from
August 13, 2026 06:27
dc5ff9d to
1952b5c
Compare
Contributor
|
@coderabbitai Addressed the review findings:
|
|
Tip For best results, initiate chat on the files or code changes.
The preserved node-count assignment order is consistent with the stated behavior-preservation goal. The invalid-definition path returns before publication.
🐇 ✅ 🧠 Learnings usedIf you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! You are interacting with an AI system. |
zmnobug
force-pushed
the
refactor/issue-1737-deduplicate-hbg-graph-execution
branch
4 times, most recently
from
August 14, 2026 04:42
f91ab4f to
88f8a48
Compare
ChaoWao
force-pushed
the
refactor/issue-1737-deduplicate-hbg-graph-execution
branch
from
August 15, 2026 01:59
88f8a48 to
451fff4
Compare
- Move the byte-identical Graph cache, materialization, and host state code into a common HBG component. - Compile the common AICPU source against each architecture's native include tree while keeping per-architecture forwarding headers. - Reuse the shared Graph cache unit source and design document across A2/A3 and A5. Fixes hw-native-sys#1737
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Compatibility
Testing
Fixes #1737