[Code Health] tensor_dump mask pool should hash opaque task_id, not unpack runtime ring_id

[ChaoWao]

Category

Technical Debt (cleanup, refactor)

Component

Platform (a2a3 / a2a3sim)

Description

The dump-args mask pool in the platform layer (tensor_dump_aicpu.cpp) unpacks the runtime's task_id into (ring_id, slot) and gates on PLATFORM_DUMP_MASK_POOL_MAX_RINGS, making the platform aware of runtime ring-depth. This is unnecessary coupling.

The pool is a plain open-addressed hash table keyed by the full 64-bit task_id (each entry stores the complete task_id and probes on collision):

static bool resolve_dump_args_task_slot(uint64_t task_id, uint32_t *idx_out) {
    uint32_t ring_id = static_cast<uint32_t>(task_id >> 32);
    if (ring_id >= TENSOR_DUMP_MASK_POOL_MAX_RINGS) return false;   // (a) bound check
    uint32_t slot = static_cast<uint32_t>(task_id) & TENSOR_DUMP_MASK_POOL_DEFAULT_SLOT_MASK;
    *idx_out = (ring_id * TENSOR_DUMP_MASK_POOL_MAX_SLOTS + slot) & (DUMP_TASK_MASK_TABLE_CAPACITY - 1);  // (b) hash
    return true;
}

task_id.raw ((ring_id << 32) | local_id) is already a globally unique key. For a hash table keyed by it, the platform can treat task_id as an opaque 64-bit integer:

• (a) the ring_id >= MAX_RINGS bound-check is not needed — any task_id can enter the table.
• (b) the ring_id * MAX_SLOTS + slot split is just one hash function; hashing the whole task_id (e.g. task_id ^ (task_id >> 32)) distributes at least as well.

Why this matters: because the platform assumes the ring layout, it must know "how many rings max", which is exactly what caused the recent coupling bug — tensor_dump.h keyed TENSOR_DUMP_MASK_POOL_MAX_RINGS to a runtime's PTO2_MAX_RING_DEPTH, so host_build_graph (single-ring) lowering its depth silently broke tensormap_and_ringbuffer's --dump-args (rings 1/2/3 rejected by the bound check). PR #1185 stopped the bleeding by making the constants platform-level (PLATFORM_DUMP_MASK_POOL_MAX_RINGS = 4 = max over runtimes), but that preserved the unnecessary ring-depth awareness rather than removing it.

Contrast — where ring_id awareness IS legitimate: scope_stats_collector_aicpu.cpp uses ring_id to index physical per-ring heap regions (s_heap_wraps[ring_id][side] * heap_cap[ring_id]). There the ring is real device layout, so the platform must know it. tensor_dump's pool is a pure key lookup — no per-ring physical structure — so the ring split is superfluous.

Location

src/common/platform/shared/aicpu/tensor_dump_aicpu.cpp — resolve_dump_args_task_slot, set_dump_args_task_mask, get_dump_args_task_masks (the three ring_id / MAX_RINGS sites).

Constants to remove: PLATFORM_DUMP_MASK_POOL_MAX_RINGS / PLATFORM_DUMP_MASK_POOL_MAX_SLOTS in src/a2a3/platform/include/common/platform_config.h and src/a5/platform/include/common/platform_config.h; and TENSOR_DUMP_MASK_POOL_MAX_RINGS / TENSOR_DUMP_MASK_POOL_MAX_SLOTS / TENSOR_DUMP_MASK_POOL_DEFAULT_SLOT_MASK in the two tensor_dump.h.

Proposed Fix

Hash the opaque task_id directly for the mask-pool index and drop the ring-depth awareness entirely:

static bool resolve_dump_args_task_slot(uint64_t task_id, uint32_t *idx_out) {
    uint64_t h = task_id ^ (task_id >> 32);              // or a stronger 64->32 mix
    *idx_out = static_cast<uint32_t>(h) & (DUMP_TASK_MASK_TABLE_CAPACITY - 1);
    return true;                                          // no ring bound-check
}

Then delete the PLATFORM_DUMP_MASK_POOL_MAX_* and TENSOR_DUMP_MASK_POOL_MAX_* constants. Net effect: the platform tensor-dump code becomes fully agnostic to any runtime's ring depth — a runtime changing PTO2_MAX_RING_DEPTH can never again affect dump-args. Verify --dump-args still selects the right tasks on both tensormap_and_ringbuffer (4-ring) and host_build_graph (1-ring), sim + onboard.

Related: #1252 (rename TensorDump implementation to ArgsDump) touches the same subsystem — worth coordinating so the rename and this decoupling land coherently.