Skip to content

Commit 05be850

Browse files
lvyufengclaude
andauthored
fix: pop the MSPTI external-correlation stack with a real address (flagos-ai#434)
CannDeviceTracer::popCorrelation passed a null out-parameter to msptiActivityPopExternalCorrelationId. MSPTI's header documents that parameter as optional ("can be NULL"), but CANN 9.0 rejects a null with MSPTI_ERROR_INVALID_PARAMETER without unwinding the stack, and the tracer discarded the return value. The external-correlation stack therefore grew for the life of the process, so every activity record was stamped with the id most recently pushed -- the CPU op that starts after the launch rather than the one that issued it. The effect on Ascend: aten::matmul's five MatMulV2 kernels carried the External ids of the aten::empty calls that followed them, aten::relu's kernels those of the following aten::empty_strided calls, and the Sort and Cast kernels collapsed onto a single shared id. aten::matmul read self_device_time_total 0.0 against a device_time_total of 98.0us (the duration arrives through Kineto's child aggregation, only the direct attribution was wrong), so any consumer selecting on self device time -- including test_profiler_device_time_linkage -- found nothing. The CUPTI and MUPTI tracers already pass a real address; CANN was the only one passing null. Fix: pass a real address, as the other two tracers do. Measured on Ascend 910 with CANN 9.0: - before: aten::matmul self_dev=0.0 dev_total=181.9, every device record carrying the id of the next op; - after: aten::matmul self_dev=126.7 dev_total=126.7, aten::empty self_dev=0.0, and each kernel record carries the id of its dispatcher. tests/integration/test_profiler_contract.py -m profiler: 11 passed, 1 skipped (with the process-start MSPTI preload) 10 passed, 2 skipped (without it) The skip is gpu_memset in both cases, a correct-by-design consequence of routing torch.zeros() through aclnnInplaceZero rather than the allocator. test_profiler_device_time_linkage was previously absorbed as an XFAIL on Ascend; it now asserts, so _MATMUL_DEVICE_TIME_XFAIL names cuda alone. Also adds tests/unit/test_device_tracer_correlation.py, a source guard on the call shape for every vendor pop site, wired into the platform-agnostic CI job, and updates the four places that documented the old behaviour. Fixes flagos-ai#425 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 41f1721 commit 05be850

8 files changed

Lines changed: 199 additions & 33 deletions

File tree

‎.github/configs/ascend.yml‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -163,13 +163,14 @@ integration_tests:
163163
-v -m profiler --tb=short
164164
165165
# Device-side contract cases run on Ascend now, not only CPU/Trace: kernel,
166-
# runtime, flow, and metadata are capability rows the tracer satisfies, and
167-
# the step below was measured on 910 with CANN 9.0 at "10 passed, 1 skipped,
168-
# 1 xfailed" (the skip is memset, the xfail is
169-
# test_profiler_device_time_linkage -- Ascend's device time is attributed to
170-
# the allocation op that follows the launch rather than to the launcher,
171-
# issue #425; that case's xfail is now scoped to the two platforms with a
172-
# measured reason for it, cuda and ascend, where it used to be unconditional).
166+
# runtime, flow, metadata and linkage are capability rows the tracer
167+
# satisfies, and the step below was measured on 910 with CANN 9.0 at
168+
# "11 passed, 1 skipped". The skip is memset (see below); the eleven include
169+
# test_profiler_device_time_linkage, whose xfail #425 retired by fixing the
170+
# tracer -- popCorrelation passed a null out-parameter, which CANN rejects
171+
# without unwinding the external-correlation stack, so every launch was
172+
# stamped with the id pushed after it. The marker now names cuda alone, so on
173+
# this platform the case asserts rather than reporting.
173174
# The runtime `cbid` argument stays off: MSPTI runtime records carry the
174175
# vendor-resolved API name and no callback id.
175176
#

‎.github/workflows/agnostic-checks.yml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ jobs:
8383
tests/unit/test_cupti_runtime_cbid_table.py \
8484
tests/unit/test_dcu_core_decoupling.py \
8585
tests/unit/test_dcu_env_script.py \
86+
tests/unit/test_device_tracer_correlation.py \
8687
tests/unit/test_device_tracer_selection.py \
8788
tests/unit/test_env_registry.py \
8889
tests/unit/test_gen_vendor_confs.py \

‎csrc/profiler/cann_device_tracer.cc‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -391,7 +391,13 @@ class CannDeviceTracer final : public DeviceTracer {
391391
void popCorrelation() override {
392392
std::lock_guard<std::mutex> lock(mutex_);
393393
if (active_) {
394-
api_.pop_external(MSPTI_EXTERNAL_CORRELATION_KIND_CUSTOM0, nullptr);
394+
// MSPTI documents `lastId` as optional ("can be NULL") but rejects a null
395+
// pointer with MSPTI_ERROR_INVALID_PARAMETER without unwinding the stack,
396+
// so a null here leaves every later launch stamped with the most recently
397+
// pushed id -- the op that starts after it rather than the op that issued
398+
// it. Pass a real address, as the CUPTI and MUPTI tracers do.
399+
uint64_t last = 0;
400+
api_.pop_external(MSPTI_EXTERNAL_CORRELATION_KIND_CUSTOM0, &last);
395401
}
396402
}
397403

‎docs/architecture/profiler.md‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -473,11 +473,13 @@ The shared cross-backend profiler contract (`tests/integration/test_profiler_con
473473
- `test_profiler_memcpy_events` is gated on `mspti_preload_active()`, which checks `/proc/self/maps` for the actually loaded `libmspti.so` rather than trusting the `LD_PRELOAD` string. When the environment script has made the library available before process start, the workload's host-to-device copy produces a real positive-duration `gpu_memcpy` record; a missing or invalid library causes the test to skip explicitly instead of claiming a capability it does not have. The link map is sampled once at `profiler_support` import, which is the only moment that answers the question: the module is loaded as a pytest plugin before anything starts a profiler session, so a hit there can only come from a process-start preload. Sampling later would also see the lazy `dlopen` in `CannDeviceTracer::start()`, which does *not* enable interposition — measured on 910, a late check reports "preloaded" and then the memcpy assertion fails on a workload that produced no records.
474474
- `test_profiler_memset_events` stays disabled for Ascend regardless of preload. The direct `ctypes` probe above proves the CANN interception path itself works: `aclrtMemset`/`aclrtMemsetAsync` produce real `gpu_memset` records when called explicitly under preload. But nothing reachable from the shared workload calls that allocator path. `torch.zeros()` -- the only zeroing op the workload exercises -- routes to the `aclnnInplaceZero` kernel (`csrc/aten/backends/ascend/generated/ascend_kernels.cc`), not to the allocator's `aclrtMemset`/`aclrtMemsetAsync` calls in `csrc/runtime/accelerator/ascend/memory.cc`. Switching it only to make a profiler record appear would regress measured 910 latency from 12.7us to 133us at 1 MiB and from 17.4us to 9080us at 64 MiB (`aclrtMemsetAsync` is slower still). The high-performance kernel routing is therefore intentional, and the absent `gpu_memset` record is a correct-by-design capability difference.
475475

476-
The device-side capability rows are a separate question from the memcpy/memset pair, and Ascend's were the last held back. `profiler_support.capabilities_for_platform` set `device = platform != "ascend"` and propagated that to `kernel`, `runtime`, `flow`, `linkage`, and `metadata`, so ten of the contract's twelve cases skipped on Ascend and the two that ran could not fail. That row described the tracer as it stood before MSPTI activity capture, not the tracer as it is. Measured on 910 with CANN 9.0 and the preload above, ten of the twelve pass, one skips (memset), and the twelfth xfails on linkage. Re-run without the preload the same way and the result is `9 passed, 2 skipped` — the preload gates memcpy alone; kernel and runtime/API records arrive through `CannDeviceTracer`'s lazy `dlopen`, which does not need it.
476+
The device-side capability rows are a separate question from the memcpy/memset pair, and Ascend's were the last held back. `profiler_support.capabilities_for_platform` set `device = platform != "ascend"` and propagated that to `kernel`, `runtime`, `flow`, `linkage`, and `metadata`, so ten of the contract's twelve cases skipped on Ascend and the two that ran could not fail. That row described the tracer as it stood before MSPTI activity capture, not the tracer as it is. Measured on 910 with CANN 9.0 and the preload above, `11 passed, 1 skipped` — the skip is memset, and the eleven include the linkage case below. Re-run without the preload the same way and the result is `10 passed, 2 skipped` — the preload gates memcpy alone; kernel and runtime/API records arrive through `CannDeviceTracer`'s lazy `dlopen`, which does not need it.
477477

478-
Opening those rows does not make device-time linkage work. Ascend links each device event to the CPU op that *follows* its launch rather than to the op that launched it: the five `MatMulV2_*` kernels dispatched by `aten::matmul` carry the `External id`s of the `aten::empty` calls that follow, the five `relu_forward_kernel_rank_1_0` kernels dispatched by `aten::relu` carry those of the `aten::empty_strided` calls, and 24 of the 37 CPU-op ids in the shared workload own no device event at all. The duration is not lost — `aten::matmul.device_time_total` reads 98.0us against a `self_device_time_total` of 0.0us, so it arrives through Kineto's child aggregation — but the launching operator itself reads zero, which is what `test_profiler_device_time_linkage` selects on. The `linkage` row is deliberately left True so that case reports as an XFAIL rather than disappearing into a skip; the tracer-side cause is issue #425.
478+
Device-time linkage was the last of those rows to report a real result, and the defect was in this repository rather than in CANN. `CannDeviceTracer::popCorrelation` called `msptiActivityPopExternalCorrelationId(kind, nullptr)`; MSPTI's own header documents that out-parameter as optional (`lastId [in] ... can be NULL`), but CANN 9.0 returns `MSPTI_ERROR_INVALID_PARAMETER` for a null pointer *without unwinding the stack*, and the tracer discarded the result. The external-correlation stack therefore grew monotonically for the process's life, so every activity record was stamped with the id most recently pushed — the op that starts *after* the launch rather than the one that issued it. Measured on 910 with the shared workload, the five `MatMulV2_*` kernels dispatched by `aten::matmul` carried the `External id`s of the `aten::empty` calls that followed them, the five `relu_forward_kernel_rank_1_0` kernels dispatched by `aten::relu` carried those of the `aten::empty_strided` calls, and the `Sort` and `Cast` kernels collapsed onto one id. The duration was not lost — `aten::matmul.device_time_total` read 98.0us against a `self_device_time_total` of 0.0us, so it arrived through Kineto's child aggregation — but the launching operator read zero, which is what `test_profiler_device_time_linkage` selects on. Passing a real address, as the CUPTI and MUPTI tracers already did, restores the pairing: measured again on 910, `aten::matmul` reads `self_device_time_total` 126.7us against `aten::empty` 0.0us, and every kernel record carries the id of its dispatcher. That is issue #425.
479479

480-
That case carried an unconditional `xfail` until this change, added by #272 when `mm`/`bmm` moved from `cuda` to `flaggems` and the CUDA matmul stopped surfacing device time (FlagGems issue #6223). Unconditional was the wrong shape. `xfail` is not a skip — it runs the body and absorbs the outcome — so an all-platform marker does not narrow the contract, it converts the assertion's failures into XFAILs on every platform, including the four the known defect is not about. DCU, PPU and MUSA had each been recording `1 xpassed` in their profiler-contract runs, which is the assertion passing under a marker that would equally have swallowed it failing. The marker is now conditioned on the two platforms with a measured reason to need it, `cuda` and `ascend`, and the reason names both; the rest assert.
480+
The stack-not-unwound model is what the evidence distinguishes, not merely one candidate: it predicts the observed id for all fifteen device records of the shared workload and for every launch in an independent probe, and the C-level reproduction (`push(100)`, `push(200)`, `pop(NULL)` → error, `pop(&last)` → `200`) confirms the null call is a no-op that leaves both entries on the stack.
481+
482+
That case carried an unconditional `xfail` until #426, added by #272 when `mm`/`bmm` moved from `cuda` to `flaggems` and the CUDA matmul stopped surfacing device time (FlagGems issue #6223). Unconditional was the wrong shape. `xfail` is not a skip — it runs the body and absorbs the outcome — so an all-platform marker does not narrow the contract, it converts the assertion's failures into XFAILs on every platform, including the four the known defect is not about. DCU, PPU and MUSA had each been recording `1 xpassed` in their profiler-contract runs, which is the assertion passing under a marker that would equally have swallowed it failing. #426 conditioned the marker on the two platforms with a measured reason to need it, `cuda` and `ascend`; #425 removed `ascend` again by fixing it, so the marker now names `cuda` alone and Ascend asserts.
481483

482484
The one row that is genuinely absent is `cbid`. MSPTI's runtime record has no callback-id field — it carries the API name the vendor already resolved — so `cann_device_tracer.cc` names each `privateuse1_runtime` event from `record->name` and the union of its argument keys is `{External id, correlation, thread}`. The tracers that resolve an id through a table (CUPTI, roctracer, MUPTI, TOPSPTI) stamp it onto the event args, which is what the contract's `assert "cbid" in ...` was reading; that assertion is now gated on a `cbid` capability row, with an unconditional `assert keys` floor so the gate cannot degrade into asserting nothing. Ascend reaches the underlying property — distinct runtime calls stay distinguishable — by the other road, and `test_profiler_runtime_names_are_not_all_fallback` measures it directly. Synthesizing a `cbid` for MSPTI would mean keying one on the activity correlation, and CANN 9.0's callback surface returns a repeated stack-address-shaped value there rather than a stable per-callsite id (issue #195).
483485

‎docs/reference/compatibility.md‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
|---|---|---|---|---|---|---|---|---|
2525
| NVIDIA CUDA | `FLAGOS_ACCELERATOR=cuda` (default) | CUDA boxing over an external `libtorch_cuda.so` | Stable | Experimental (inductor GPU device registered; no CI test step) | Beta (FlagCX + NCCL fallback, DDP live-verified) | Stable (CUPTI parity) | Beta (Python + C++ dispatch paths) | Stable |
2626
| MetaX | `FLAGOS_ACCELERATOR=metax` | CUDA boxing via `cu-bridge` against the vendor libtorch | Stable (FP16/BF16 autocast and GradScaler measured in boxing mode) | Experimental (vendor Triton and FlagTree MetaX measured on C550; vendor Triton CI-covered) | Experimental (NCCL-shaped `mccl` fallback; not CI-covered) | Experimental (MCPTI parity measured on C550; not CI-covered) | Experimental (Python dispatch; not CI-tested on MetaX) | Stable |
27-
| Ascend | `FLAGOS_ACCELERATOR=ascend` | Native ACLNN operator backend, FlagGems via FlagTree (Triton 3.5) | Stable (CI-covered ops, RNG suite) | Experimental (inductor measured on 910 against triton-ascend 3.2.0 only; **not** revalidated on FlagTree, three toolchain workarounds, serial compile, no CI step) | Experimental (HCCL fallback; architectural routing only, no collective-level CI) | Beta (MSPTI kernel/runtime/flow/memcpy events CI-covered by the shared contract; device-time linkage misattributes to the following allocation op, #425; parity suite excluded from CI) | Beta (Python dispatch; FlagGems-first conf, float64 and bool `neg` routes fall back to ACLNN) | Beta |
27+
| Ascend | `FLAGOS_ACCELERATOR=ascend` | Native ACLNN operator backend, FlagGems via FlagTree (Triton 3.5) | Stable (CI-covered ops, RNG suite) | Experimental (inductor measured on 910 against triton-ascend 3.2.0 only; **not** revalidated on FlagTree, three toolchain workarounds, serial compile, no CI step) | Experimental (HCCL fallback; architectural routing only, no collective-level CI) | Beta (MSPTI kernel/runtime/flow/memcpy events plus device-time linkage, all CI-covered by the shared contract; parity suite excluded from CI) | Beta (Python dispatch; FlagGems-first conf, float64 and bool `neg` routes fall back to ACLNN) | Beta |
2828
| PPU | `FLAGOS_ACCELERATOR=ppu` | Same CUDA-boxing path as NVIDIA CUDA, against the PPU's CUDA-13-compatible SDK, bundling its own libtorch | Experimental (FP16/BF16 autocast and GradScaler measured on PPU hardware, not in CI) | Not validated | Experimental (NCCL fallback via vendor-adapted `libnccl.so.2`; not CI-covered) | Not validated on this vendor's tracer | Experimental (vendor-index Triton required) | Experimental |
2929
| Hygon DCU | `FLAGOS_ACCELERATOR=dcu` | CUDA boxing over the hipified DTK torch build (HIP kernels under the CUDA dispatch key) | Beta (including FP16/BF16 autocast and GradScaler) | Experimental (FlagTree HCU validated on `gfx936`; not in CI) | Experimental (RCCL via DTK; all_reduce/DDP measured on 2 cards, not in CI) | Beta (parity suite runs in CI) | Beta (Python dispatch only) | Beta |
3030
| Enflame GCU | `FLAGOS_ACCELERATOR=gcu` | Native `libtopsaten.so` operator backend, with CPU fallback for unrouted/int64/float64 ops | Beta (operator, RNG, factory, and AMP suites CI-guarded on S60) | Not validated | Not validated | Runtime only (TOPSPTI collects activities; no device events on a CPU-only Kineto build) | Experimental (Python dispatch, requires vendor Triton) | Beta |
@@ -107,13 +107,17 @@ still out of scope — `test_profiler_parity.py` is a diff against a torch-cuda
107107
only the CUDA-boxing platforms can run one — and remains excluded from CI (same file, lines
108108
112-117). The shared profiler *contract* does run, and the device side of it is live: MSPTI
109109
emits named kernel, runtime/API, flow, and (under the process-start preload above) memcpy
110-
activity, and the step was measured on 910 at `10 passed, 1 skipped, 1 xfailed`. The skip is
110+
activity, and the step was measured on 910 at `11 passed, 1 skipped`. The skip is
111111
`gpu_memset`, a correct-by-design consequence of routing `torch.zeros()` through
112112
`aclnnInplaceZero` rather than the allocator (see
113-
[`docs/architecture/profiler.md`](../architecture/profiler.md)). The xfail is device-time
114-
linkage: Ascend attributes each device event to the CPU op that follows its launch, so the
115-
launching operator reads zero self device time while the allocation op that follows absorbs
116-
it (issue #425). Distributed support recommends the FlagCX path (see
113+
[`docs/architecture/profiler.md`](../architecture/profiler.md)). The device-time
114+
linkage case is one of the eleven that assert: MSPTI used to stamp every launch with
115+
the id of the CPU op that started after it, which turned out to be this repository's
116+
defect rather than the vendor's — `popCorrelation` passed a null out-parameter, which
117+
CANN rejects without unwinding, so the external-correlation stack grew for the life of
118+
the process (issue #425). With the pop fixed, the launching
119+
operator owns its device events and the case asserts on this platform. Distributed support
120+
recommends the FlagCX path (see
117121
[`docs/architecture/distributed-flagcx.md`](../architecture/distributed-flagcx.md), lines
118122
204-208); the native HCCL fallback and the `flagos→npu` zero-copy view are architectural
119123
routing, not on-hardware-verified collectives (same file, lines 67-75).

‎tests/integration/profiler_support.py‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -146,14 +146,16 @@ def capabilities_for_platform(platform: str) -> ProfilerCapabilities:
146146
installs, nine of those ten pass; the tenth asserted a runtime ``cbid``
147147
argument that MSPTI has no field for, and is now its own row below.
148148
149-
What opening the row did *not* fix is device-time linkage: Ascend attributes
150-
each kernel to the CPU op that follows its launch, so the launching operator
151-
keeps ``self_device_time_total == 0.0`` and ``test_profiler_device_time_linkage``
152-
fails. That is issue #425, a tracer defect, and the case declares it with a
153-
platform-scoped ``xfail`` instead of a blanket one. The ``linkage`` row stays
154-
True rather than being turned off, so the case reports as an XFAIL -- a
155-
visible, releasable statement about a known defect -- instead of
156-
disappearing into a skip.
149+
Device-time linkage was the one row that did not open with them: MSPTI
150+
stamped every launch with the CPU op that started *after* it, so the
151+
launching operator kept ``self_device_time_total == 0.0``. That turned out
152+
to be this repository's defect rather than the vendor's -- ``popCorrelation``
153+
passed a null out-parameter, which CANN rejects without unwinding, leaving
154+
the external-correlation stack to grow for the life of the process (issue
155+
#425). With the pop fixed the launching operator owns its device events, so
156+
the ``linkage`` row below is True on its measured merits and
157+
``test_profiler_device_time_linkage`` runs as an ordinary case on the Ascend
158+
job instead of reporting as an XFAIL.
157159
"""
158160
device = True
159161
runtime = device

0 commit comments

Comments
 (0)