Refactor: retire five PTO2 identifiers, one per commit - #1969
Merged
ChaoWao merged 5 commits intoAug 24, 2026
Merged
Conversation
|
Important Review skippedToo many files! This PR contains 323 files, which is 223 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. Usage-priced reviews support at most 300 files. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (323)
You can disable this status message by setting the 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 |
`PTO2OrchestrationConfig` becomes `OrchestrationConfig`. The name was free — nothing else in the tree spells it — so this is a plain prefix strip, 308 occurrences across 156 files, one identifier in one commit (`.claude/rules/codestyle.md` rule 10). The shorter return type lets clang-format rejoin 110 orchestration-entry signatures onto one line, which is the rest of the diff. The type is named in orchestration sources outside this repo, so this lands without a compatibility alias by the same explicit decision as hw-native-sys#1963. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`PTO2TaskSlotState` becomes `ChipTaskSlotState` — not `TaskSlotState`, because that name is taken by a different type: `src/common/hierarchical/types.h` holds the host orchestrator's per-task bookkeeping, built out of `std::mutex`, `std::vector` and `std::atomic`. This one is the chip runtime's 64-byte slot in the shared-memory task window, carrying a spinlock and refcounts the AICPU scheduler touches. Two types, two contexts, so the prefix `PTO2` was providing real disambiguation here and stripping it alone would have merged them. `.claude/rules/codestyle.md` rule 13 says which context keeps the bare name: the L3+ recursive scheduler does, and the chip runtime takes the `Chip` prefix, as `ChipWorker` / `ChipTensor` / `ChipTaskArgs` already do. 856 occurrences across 109 files. The name is the same length, so every changed line is the identifier and nothing re-flowed. The shared-memory layout diagrams in `runtime/shared_memory.h` and `docs/RUNTIME_LOGIC.md` keep spelling the three slot arrays `TaskDescriptor[]` / `TaskPayload[]` / `TaskSlotState[]` — they strip the prefix from all three uniformly and name no type that exists under that exact spelling, so they move as one piece when the sibling types are renamed rather than half now. The type is named in orchestration sources outside this repo, so this lands without a compatibility alias by the same explicit decision as hw-native-sys#1963. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The orchestration API's scoped-block macro becomes `SIMPLER_SCOPE`, and its sibling `SIMPLER_SCOPE_GUARD`. Not a bare `SCOPE`: a macro has no namespace, and every translation unit that includes `orchestration_api.h` would inherit a four-letter object-like collision magnet. `RT_` is not available either — CANN owns that prefix for its runtime macros (`RT_ERROR_NONE` and friends, consumed by `device_runner_helpers.cpp`). `SIMPLER_` is the prefix the status codes took in hw-native-sys#1963, so the macro surface now spells its owner the same way throughout. The RAII type the macro expands to (`PTO2ScopeGuard` -> `ScopeGuard`) and the token-pasting helpers (`_PTO2_CONCATENATE`, `_PTO2_CONCATENATE_IMPL`) move in the same commit. They exist only to implement these two macros and sit within twenty lines of them in the same header, so splitting them out would leave `SIMPLER_SCOPE` expanding to a `PTO2ScopeGuard` on the adjacent line. 265 occurrences across 77 files. `PTO2_SCOPE_TASKS_CAP` and the `PTO2_SCOPE_STATS_*` capacity constants are untouched — they are separate identifiers that only share a spelling prefix, not part of this macro. `SIMPLER_SCOPE` is typed by orchestration sources outside this repo, so this lands without a compatibility alias by the same explicit decision as hw-native-sys#1963. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`PTO2Runtime` becomes `RuntimeContext` — not `Runtime`, because that name is
taken, in the same header tree, by the AICPU/AICore handshake class whose own
docstring already draws the line: "Task graph construction is handled by
PTO2Runtime; this class only handles execution control and device orchestration
state." The struct being renamed is the context the orchestration `.so` receives
— ops table, shared-memory handle, orchestrator, scheduler, mailbox, heap, mode,
statistics — and "context" is what its docstring called it all along, under a
brand name (`PTO Runtime2 context`) rather than a role.
Its three satellites move with it, in one commit, because they are the same
cluster and splitting them would put `RuntimeContext` and `PTO2RuntimeOps` on
adjacent lines of one struct:
- `PTO2RuntimeOps` -> `RuntimeOps` (the ops table, first field)
- `PTO2RuntimeMode` -> `RuntimeMode`
- `PTO2RuntimeArenaLayout` -> `RuntimeArenaLayout`
The last one had already converged: the only occurrences of the bare name in the
tree were the gtest suite names in `tests/ut/cpp/{a2a3,a5}/test_shared_memory.cpp`,
which is what the type is now spelled.
758 occurrences across 69 files. The longer name re-wraps some multi-line
signatures, which is the rest of the diff.
Tier-A brand prose in the same files goes too (rule 10 fixes it on sight): the
`PTO Runtime2 - Main Interface` / `- Orchestrator Interface` /
`- Main Implementation` banners, the two `static_assert` messages that said
"PTO2 runtime ring depth", and the title and overview of both
`tensormap_and_ringbuffer/docs/RUNTIME_LOGIC.md` copies, which now name the
runtime the way the `host_build_graph` copies already do. The a2a3 and a5 copies
of that doc had drifted apart on the `(PTO2)` heading suffix; they agree again.
`RuntimeContext` is the handle every orchestration entry point takes, including
in sources outside this repo, so this lands without a compatibility alias by the
same explicit decision as hw-native-sys#1963.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`PTO2_MAX_RING_DEPTH` becomes `CHIP_MAX_RING_DEPTH` — not `MAX_RING_DEPTH`, because that name is taken by a constant this one is deliberately equal to but distinct from: `src/common/hierarchical/types.h` defines the host orchestrator's `MAX_RING_DEPTH = 4` heap-slab count, which `python/bindings/worker_bind.h` exports to Python. `docs/orchestrator.md` states the relationship outright — the host slab count "matches L2's `PTO2_MAX_RING_DEPTH`" — so collapsing the two spellings into one would have turned a documented coincidence into an apparent identity. `.claude/rules/codestyle.md` rule 13 assigns the bare name to the L3+ context and the `Chip` prefix to the chip runtime, the same split this branch already applied to `ChipTaskSlotState`. 484 occurrences across 55 files. After hw-native-sys#1965 the macro is defined only in the two `tensormap_and_ringbuffer/runtime/runtime_types.h` copies — host_build_graph carries no ring dimension at all — so this is now a single-runtime constant. `PTO2_SCOPE_STATS_MAX_RING_DEPTH` in `src/common/platform/include/common/scope_stats.h` is untouched: it is a separate identifier that bounds the scope-stats collector's own per-ring arrays. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ChaoWao
force-pushed
the
refactor/retire-pto2-identifiers
branch
from
August 24, 2026 03:23
c04d398 to
1c2d11f
Compare
8 tasks
ChaoWao
added a commit
that referenced
this pull request
Aug 25, 2026
Every `PTO2` name this repository reads is gone: 5804 occurrences before, 62 after (`git grep -Io -E "PTO2|pto2_" | wc -l`), across 315 files. 22 commits, one identifier or one coherent cluster each, so no reader ever has to know two spellings for one thing (`.claude/rules/codestyle.md` rule 10). Most of it is a prefix strip: `ResourceShape`, `TaskDescriptor`, `TaskPayload`, `SharedMemoryHeader` / `RingHeader` / `Handle`, `SchedulerState`, `OrchestratorState`, `TaskAllocator`, the `Fanin*` and `DepList*` types, the early-dispatch and sync-drain state machines with their fourteen enumerators, `SubtaskSlot`, `DispatchPayload`, `ScopeMode`, `LaunchSpec`, `OutputLayout`, the `sm_layout` namespace, and the constants that size each of them. Nine identifiers could not take the bare name, and the reason is worth stating because the check that finds it is not obvious: a target name has three independent owners to clear, and only the first is greppable here. - The **host orchestrator** under `src/common/hierarchical/` already declares `TensorMap`, `ReadyQueue`, `TaskState` (which is bound into Python), `Ring` / `HeapRing`, `AllocResult` and `MAX_SCOPE_DEPTH`. These are the same concepts one tier up, which is exactly where a prefix was doing real work. Rule 13 gives the bare name to the L3+ context, so the chip runtime's versions are `ChipTensorMap`, `ChipReadyQueue`, `ChipTaskState`, `ChipTaskRing` / `ChipHeapRing` / `ChipRingSet` and `CHIP_MAX_SCOPE_DEPTH`. - **External headers the same translation units include.** CANN defines `ALIGN_UP` in `acl/ops/acl_dvpp.h` and `acl/dvpp/hi_media_common.h`, so `PTO2_ALIGN_UP` becomes `CHIP_ALIGN_UP`. No grep of this tree can see that. - **Any declaration at all, if the name is an object-like macro**, because a macro rewrites its identifier textually instead of colliding where the compiler can report it. `#define HEAP_SIZE (256 * 1024 * 1024)` rewrote three cpput fixtures' `static constexpr uint64_t HEAP_SIZE = 4096;` into `static constexpr uint64_t (256 * 1024 * 1024) = 4096;`. So the macros in `runtime/runtime_types.h` keep `CHIP_` even where the bare name is free, while their `inline constexpr` siblings in `submit_types.h` take it. That third case also exposed a gap in the gate: `pip install -e .` builds the four runtime `.so` files and not the C++ unit tests, which are a separate CMake project. Both are compiled per commit now, and the commits that had chosen a bare macro name were replayed from the corrected plan rather than patched afterwards, so every commit here builds both trees. The `PTO2_RING_TASK_WINDOW` / `PTO2_RING_HEAP` / `PTO2_RING_DEP_POOL` environment variables are **removed rather than renamed**. `CallConfig.runtime_env` already carried the same three values per task, and per task is strictly more expressive: two tasks in one process can hold different ring sizes, which a process-wide export never allowed. So the whole parsing path goes -- `trim_copy`, `parse_uint_token`, `apply_env_ring_value[s]` and the scalar / comma-separated grammar in all four `host/runtime_maker.cpp` copies -- leaving per-task entries over the compile-time default with nothing in between, and taking two now-unused includes with it. An unrecognised environment variable is *ignored*, so a caller still exporting one of those names would run on the 256 MB default with its requested sizing nowhere in evidence, and the symptom would be a heap-exhaustion deadlock that reads as a runtime regression rather than a configuration change. `warn_on_retired_ring_env()` therefore names each retired variable that is set, once per bind, and points at `CallConfig.runtime_env`. It has no behavioral effect and is deletable once no caller sets them. This edge is real and the repo documents it itself: `.claude/skills/multi-repo-qwen-setup` instructed exporting `PTO2_RING_HEAP=4294967296` for batch-16 qwen prefill -- 4 GiB against that 256 MB default -- and the deepseek example records that pypto-lib's daily CI sizes its network the same way. Both now name `CallConfig.runtime_env` fields, and the skill says outright that a runner still exporting the old names deadlocks exactly as its own gotcha section describes. Removing an env knob is only half the change: every diagnostic that advertised one has to move with it, or the result is a hint naming a variable nothing reads. This change fixes three instances of precisely that defect, two of them pre-existing and found by reading every `PTO2_*` string: - `tools/benchmark_rounds.sh --serial-orch-sched` set `PTO2_SERIAL_ORCH_SCHED`, while the runtime reads `SIMPLER_TMR_SERIAL_ORCH_SCHED_ENABLE`. The serial arm ran with serial orch->sched still off, so the script measured the parallel path twice and reported the pair as a serial-versus-parallel comparison. Any measurement taken through that flag is void. - `error_hint(SIMPLER_ERROR_SCHEDULER_TIMEOUT)` told the user to raise `PTO2_SCHEDULER_TIMEOUT_MS`; the knob is `SIMPLER_SCHEDULER_TIMEOUT_MS`, which CI already sets by that name. Following the hint changed nothing and made a slow kernel look like a deadlock. - The ring knobs themselves: the device-side ring-buffer and orchestrator deadlock reports, the fanin-spill capacity report, five `error_hint()` strings, both capacity tables under `docs/troubleshooting/`, the `running-onboard` triage table and the runnable recipe in `docs/dfx/scope-stats.md` now name `runtime_env.ring_task_window` / `ring_heap` / `ring_dep_pool`. Two unit tests assert on that hint text and move with it, and `MULTI_RING.md` loses the two env tiers from its precedence ladder along with the section that was the only documentation of the comma-separated grammar. Three more references named things that do not exist under any spelling, which is how a stale comment stops being noise and becomes a lie: `pto2_task_slot()`, cited by `runtime_types.h` and `runtime_init.cpp` as the way to compute a slot (it is `local_id` masked by the window size, which the comment now says, including why the window must be a power of two); `pto2_run_runtime`, named five times in `docs/task-flow.md` as the L2 ABI edge (it is `simpler_run`); and `pto2_init_*` flags, listed in `docs/dynamic-linking.md` among the fields `SchedulerContext::deinit()` resets (it resets no such flag). The four shared-memory layout diagrams deferred by #1969 are correct now that every row -- `SharedMemoryHeader`, `TaskDescriptor[]`, `TaskPayload[]`, `ChipTaskSlotState[]` -- names a type that exists under exactly that spelling; the two multi-ring boxes widen by four columns to fit. Rule 10 previously described a 6.6k-occurrence backlog across 367 files; it now records that the retirement is complete, so a new `PTO2` match is a defect rather than backlog for a regression to hide behind. What remains is 62 occurrences in 17 files, none of them read here: this rule file's own illustrations of the banned spelling, the retired-name list inside the warning, prose recording that these names are gone, and `PTO2_MANUAL_MAX_SEQ` -- a pypto-lib knob two example READMEs document and this repo never reads. These names are reachable from orchestration sources outside this repo, so this lands without compatibility aliases by the same explicit decision as #1963, #1966 and #1969; the matching renames and the merge order are coordinated outside this change. One commit here is not about naming at all. The clang-tidy pre-commit hook was failing on this branch, and because every gated CI job declares `pre-commit` in `needs:`, that one failure skipped the entire test matrix rather than failing a single job. #1845 added `include("${SIMPLER_CMAKE_DIR}/host_log_sources.cmake")` to six platform `CMakeLists.txt` without an install rule to match, so `simpler_setup/_assets/cmake/` never shipped and any runtime cmake configure from a wheel install died on that include. All four files under `cmake/` are reachable from a platform configure, so the whole directory ships now; installing only the file the error named would have failed at the next `include()`. It stayed hidden because the hook only configures on a build-cache miss and `tools/verify_packaging.sh` checks entry points rather than the asset tree. Verified by building the wheel both ways: `pip wheel --no-build-isolation --no-deps .` produces zero `simpler_setup/_assets/cmake/*` entries before the change and all four after. Verified on an a2a3 box at the final commit: all four runtime variants and the cpput targets build at every commit; a2a3sim 59 passed / 8 skipped; a5sim 52 passed; a2a3 onboard sweep 153 passed / 1 skipped; the quarantined SDMA step 2 passed; cpput 119/119 plus the hardware-labelled `test_comm_lifecycle`; pyut 1908 passed, 14 skipped. Exporting all three retired ring variables makes an a2a3sim scene test log all three warnings and still pass, which is what proves both that the warning fires and that a `PTO2_RING_TASK_WINDOW=64` is now inert. clang-format, markdownlint, ruff and `bash -n` clean across the diff.
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
1 participant
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
Five commits, each retiring one
PTO2identifier (or one coherent macro/type cluster) across every occurrence, in the order the user asked for..claude/rules/codestyle.mdrule 10 requires one identifier per commit — a rename that leaves both spellings alive is worse than no rename — and rule 9 requires "clear names or anamespace" where the prefix was actually disambiguating something.PTO2OrchestrationConfigOrchestrationConfigPTO2TaskSlotStateChipTaskSlotStatePTO2_SCOPE+ guard/concat clusterSIMPLER_SCOPE…PTO2Runtime+ 3 satellitesRuntimeContext…PTO2_MAX_RING_DEPTHCHIP_MAX_RING_DEPTHOnly #1 was a plain prefix strip. Three of the five could not take the bare name, and all three for the same reason: the name is already held by the host L3+ hierarchical orchestrator, which models the same concept one tier up.
TaskSlotState—src/common/hierarchical/types.his the host orchestrator's per-task bookkeeping (std::mutex,std::vector); the renamed one is the chip runtime's 64-byte shared-memory slot with a spinlock and refcounts.Runtime—runtime.halready has aclass Runtimefor AICPU↔AICore handshake, whose own docstring drew the line: "Task graph construction is handled by PTO2Runtime; this class only handles execution control."MAX_RING_DEPTH—hierarchical/types.hdefines the host's= 4heap-slab count andworker_bind.hexports it to Python.docs/orchestrator.mdsays the host count "matches L2'sPTO2_MAX_RING_DEPTH", i.e. two deliberately equal but distinct constants.So the
PTO2prefix was doing real work in exactly the places where the chip runtime and the host orchestrator name the same idea. Rule 13 says which side keeps the bare name — L3+ does, the chip runtime takesChip— so this branch spells themChipTaskSlotStateandCHIP_MAX_RING_DEPTH, matching the existingChipWorker/ChipTensor/ChipTaskArgs, and names the context struct for what its docstring already called it.SIMPLER_SCOPEis the one that gets a macro-shaped answer rather than a role. A bareSCOPEdoes in fact compile — it is function-like, so it only expands atSCOPE(, and neither this tree nor the CANN headers nor pto-isa spells that token (all four runtimes build and thea2a3simsweep passes with it applied; measured, not assumed). It is rejected on cost, not feasibility: a macro obeys no namespace, so naming itSCOPEclaims an English word followed by(in every translation unit that includesorchestration_api.h, in this repo and in the orchestration sources of every consumer repo, forever. It also stops being greppable —git grep -w SCOPEtoday returns 7 hits and all of them are=== SCOPE STACK ===comment banners.RT_is not available either (CANN owns it:RT_ERROR_NONE, consumed bydevice_runner_helpers.cpp), andSIMPLER_is what the status codes took in #1963, so the orchestration macro surface now spells its owner the same way the error codes do.Two clusters move as a unit rather than one identifier at a time, because splitting them would leave the two spellings on adjacent lines of the same declaration:
SIMPLER_SCOPEwith itsScopeGuard/_SIMPLER_CONCATENATEimplementation, andRuntimeContextwith theRuntimeOpstable it carries as its first field (plusRuntimeMode,RuntimeArenaLayout).Two of the new names were already in the tree as prose:
RuntimeArenaLayoutwas the gtest suite name in bothtest_shared_memory.cppcopies, and the shared-memory layout diagrams have long written the slot arrays with the prefix stripped. The diagrams stay as they are for now — they strip it from all three arrays uniformly, so they move as one piece whenPTO2TaskDescriptor/PTO2TaskPayloadland.Tier-A brand prose in the touched files goes with commit 4: the
PTO Runtime2 - Main Interface/- Orchestrator Interface/- Main Implementationbanners, twostatic_assertmessages, and the title and overview of bothtensormap_and_ringbuffer/docs/RUNTIME_LOGIC.mdcopies, which now name the runtime the way thehost_build_graphcopies already do. Those two copies had drifted apart on a(PTO2)heading suffix; they agree again.Base
Rebased onto #1968 / #1925, and before that onto #1965. That last merge is why commit 5 is here at all:
PTO2_MAX_RING_DEPTHhad 148 lines of overlap with it while it was open, and after it landed the macro is defined only in the twotensormap_and_ringbuffercopies —host_build_graphcarries no ring dimension — so the rename is smaller and no longer collides with in-flight work. Both rebases conflicted (thescheduler_cold_path.cpp/shared_memory.cpp/runtime_core.h/runtime_init.cpp/runtime_maker.cpppairs against #1965;orchestrator.cpp/scheduler.h/types.hand one troubleshooting doc against #1968). Every one was resolved the same way — discard our side of the file, take upstream's, re-run the rename over it — so no upstream restructuring is dropped and the resolution is not hand-authored. #1968 also introduced two newPTO2Runtime/PTO2_SCOPEmentions in aruntime_core.hcomment after this branch was cut; they are absorbed into the commit that owns each identifier, so the count still only goes down.Remaining surface
5862 occurrences across 319 files, led by
PTO2ResourceShape(368),PTO2TaskPayload(309),PTO2SchedulerState(305),PTO2TensorMapEntry(224),PTO2_ALIGN_SIZE(216). Given what this PR found, expect more of the collisions above rather than fewer as the chip-runtime structs get closer to the host orchestrator's vocabulary — each needs a name decided, not a prefix deleted.Cross-repo
Every identifier here is reachable from orchestration sources outside this repo —
SIMPLER_SCOPEis typed directly in orchestration code andRuntimeContext *rtis the handle every entry point takes. This lands without compatibility aliases by the same explicit decision as #1963 and #1966; the matching renames and the merge order are coordinated outside this PR.Testing
All on this box, at the final commit, after
pip install --no-build-isolation -e .:a2a3simscene sweep — 59 passed, 8 skippeda5simscene sweep — 52 passeda2a3onboard sweep (-m "not sdma" --exclude-level 4) — 153 passed, 1 skippeda2a3quarantined SDMA step — 2 passedtest_comm_lifecycle— 1/1test_second_child_failure_reaps_firstis the open flake in [Bug] SIGTERM during INIT_FAILED publication can escape forked child main #1948 (a SIGTERM-vs-os._exitrace in_forked_child_main). It passes alone, reproduces 2/2 in the full sweep here, and the whole path it exercises —python/simpler/worker.py, the test,conftest.py— is byte-identical toupstream/mainon this branch. Two more occurrences appended to that issue.clang-format --dry-run --Werror,ruff,markdownlint-cli2clean over all 323 changed files