[review-only] feat(rocm): inter-stream synchronization deps on roctracer backend

[ajassani]

What you see today vs. with this PR

ROCm Kineto traces emit hipStreamWaitEvent with no information about what the wait is on. You see an opaque CPU-side timing row, and there's no way to reconstruct the producer stream / event from trace data alone.

 Today (upstream/main, roctracer backend):
 {
   "name": "hipStreamWaitEvent",
   "args": { "cid": 213, "correlation": 234 }
 }

+After this PR:
+{
+  "name": "hipStreamWaitEvent",
+  "args": {
+    "cid": 213, "correlation": 234,
+    "hip_sync_kind": "stream_wait_event",
+    "hip_event": "0x7fb5a8000",
+    "hip_stream": "0x7fb5a8400",
+    "wait_on_stream": "0x7fb5a8800",
+    "wait_on_hip_event_id": "0x7fb5a8000",
+    "wait_on_hip_event_record_corr_id": 187
+  }
+}

CUPTI emits the equivalent metadata for cudaStreamWaitEvent via CUPTI_ACTIVITY_KIND_SYNCHRONIZATION. This PR brings ROCm traces to functional parity.

What this PR does

Hooks five HIP sync-family APIs (hipEventRecord, hipStreamWaitEvent, hipEventSynchronize, hipStreamSynchronize, hipDeviceSynchronize) on the roctracer backend. Captures a per-process hipEvent_t -> sorted vector<{stream, correlationId}> map populated by hipEventRecord callbacks, and resolves producer-stream attribution at JSON-emission time. Roctracer-only in this PR; the equivalent on the rocprofiler-sdk backend will follow in a separate PR.

Why roctracer first

PyTorch's third_party/kineto submodule currently pins a pre-March-2026 SHA, so every PyTorch ROCm wheel today uses the roctracer backend (USE_ROCPROFILER_SDK=OFF by default). Landing this on roctracer first puts the feature in users' hands the moment PyTorch bumps its kineto pin to a SHA that includes this PR -- no other changes needed.

The shared activity-row types in RocLogger.h are designed to be backend-agnostic; the rocprofiler-sdk backend will reuse them when its implementation lands in a follow-up.

Field-by-field comparison with CUPTI

Trace consumers (Perfetto, kineto post-processors, Chakra/HolisticTraceAnalysis) need a parallel field set across CUDA and ROCm. The mapping:

Concept	CUPTI key (CUDA)	This PR (ROCm)	Notes
Sync-kind label	cuda_sync_kind	hip_sync_kind	enum: stream_wait_event | event_synchronize | stream_synchronize | device_synchronize
Source stream of a wait	wait_on_stream	wait_on_stream	exact match
Source corr-id of a wait	wait_on_cuda_event_record_corr_id	wait_on_hip_event_record_corr_id	s/cuda/hip
Source event handle	wait_on_cuda_event_id	wait_on_hip_event_id	s/cuda/hip
Event handle (producer)	event_id	hip_event	see "value type" note below
Stream the activity ran on	stream	hip_stream	see "value type" note below

The wait_on_* triplet is the primary consumer-facing parity surface -- those three fields are what trace analyzers follow to reconstruct inter-stream arrows. Field names match CUPTI exactly modulo the cuda -> hip substitution.

Note on field value types and naming

There is one deliberate divergence from CUPTI that's worth flagging:

• CUPTI assigns small monotonic integer IDs to CUDA event and stream handles internally and emits those in the trace ("event_id": 7, "stream": 2). The raw cudaEvent_t pointer never leaves CUPTI.
• roctracer delivers HIP API arguments verbatim. hipEvent_t and hipStream_t are opaque pointers in the HIP runtime ABI, and roctracer does not provide an equivalent ID-translation layer.

This PR emits the raw handles as hex strings ("hip_event": "0x7f...") rather than rolling our own integer-ID translation table. The reasons:

1. Cross-trace identity is preserved by correlation, not by the handle. Every HIP runtime call gets a unique monotonic correlation ID. The wait_on_hip_event_record_corr_id field links a wait directly to its producing hipEventRecord via these correlation IDs, which never collide. Consumers that need to follow producer/consumer arrows should key on correlation.
2. The g_eventMap resolution logic is collision-safe by construction. The map is keyed by handle but rewritten on every hipEventRecord, which matches HIP's own semantics (a wait sees the most recent record of the event). Even if a handle is destroyed and the pointer is later reused by a new event, the next hipEventRecord overwrites the stale entry before any wait can resolve against it.
3. The handle in the JSON is a debugging aid, useful when manually inspecting a trace; not intended as a stable cross-trace identifier.

The naming convention (hip_* prefix) was chosen to mirror CUPTI's cuda_sync_kind rather than CUPTI's unprefixed event_id / stream -- because the value type differs, an explicit backend prefix avoids surprising consumers who'd expect an integer.

How to review

Commits are ordered by reviewability, not feature flow:

Commit	Size	Start here if you want to look at...
feat: capture inter-stream synchronization deps on the roctracer backend	~270 LoC	The actual feature. g_eventMap design, recordEvent / resolveWait / clearEventMap, JSON specs in RoctracerActivity_inl.h, dispatch wiring in RocmActivityProfiler.cpp.
feat: add shared activity row types for inter-stream sync metadata	~54 LoC	Mechanical type definitions in RocLogger.h. The rocprofiler-sdk backend will reuse these in a follow-up PR.
test: cover inter-stream dependency emission on the roctracer backend	~440 LoC	5 new TEST_F blocks gated with #ifdef ROCTRACER_FALLBACK. Each mirrors a CUPTI test where one exists.
refactor: rename sync_type metadata field to hip_sync_kind	~8 LoC	Field-name cleanup to mirror CUPTI's cuda_sync_kind.
build: small fixes to let the roctracer backend build on ROCm 7+	~16 LoC	Build hygiene. Justified in the section below; safe to skip otherwise.

Build infrastructure: why one CMake + one init.cpp change

These two changes are coupled prerequisites and could draw "why are these in a roctracer feature PR?" without context. Short answer: without them, this PR cannot be CI-tested on modern ROCm.

• libkineto/CMakeLists.txt currently unconditionally forces USE_ROCPROFILER_SDK=ON on ROCm 6.4+. There's no caller opt-out -- passing -DUSE_ROCPROFILER_SDK=OFF is silently overridden, so the roctracer code path is unreachable on any modern ROCm install. The build commit changes this to if(NOT DEFINED USE_ROCPROFILER_SDK) so the caller can opt out explicitly.
• libkineto/src/init.cpp unconditionally calls RocprofLogger::ensureRegistered(), a rocprofsdk-only symbol. Once the CMake toggle is honored, the link fails with "undefined reference" on the roctracer build. The build commit guards the call with #ifndef ROCTRACER_FALLBACK. Roctracer's registration model differs (subscription via roctracer_open_pool at trace-start) and doesn't need an equivalent force-configure step.

The two are a pair: CMake change exposes a previously-unreachable code path, init.cpp change makes the resulting build link. Together they restore the ability to build the roctracer backend directly from upstream kineto on a modern ROCm system -- the surface upstream CI uses to validate this PR.

End-user behavior is unchanged:

• Default on ROCm 6.4+: still rocprofiler-sdk.
• Default on ROCm < 6.4: still roctracer.
• New: -DUSE_ROCPROFILER_SDK=OFF is now respected on ROCm 6.4+ instead of silently overridden.

Tests

5 new TEST_F blocks in libkineto/test/RocmActivityProfilerTest.cpp, gated with #ifdef ROCTRACER_FALLBACK so the rocprofsdk build is unaffected:

Test	Validates
InterStreamDependencyTest	Happy path: hipEventRecord -> hipStreamWaitEvent on same event resolves to producer correlationId.
StreamWaitEventFutureCorrelation	Vector + upper_bound on event-handle reuse: wait at corr=101 must resolve to record corr=100, not a future corr=200.
EventMapClearedOnReset	clearEventMap() runs in onResetTraceData -- stale records from prior session don't pollute the next.
EventSynchronizeResolvesProducer	hipEventSynchronize gets producer attribution AND always emits wait_on_hip_event_id. CUPTI parity for the event-sync side.
UnresolvedWaitStillEmitsEventId	wait_on_hip_event_id is emitted even when no producer match (event recorded before profiling started).

Validation status

The earlier (pre-restructure, 9-commit stacked) PR3 was validated on Shark26 (4x MI210, ROCm 7.2.3):

• L1: [ PASSED ] 5 tests against roctracer backend; [ PASSED ] 5 tests against rocprofiler-sdk (after PR2's L1 build fix).
• L2 (cross-backend trace diff): roctracer-PR3 vs rocprofsdk-patched produce identical sync metadata field set; 8/8 hipStreamWaitEvent resolved on both backends.
• L3 (DDP): train_ddp_crosscp.py world_size=4 -- 32/32 hipStreamWaitEvent resolved, 0 dangling arrows.

This standalone branch is being re-validated now. The diff vs. the prior version is small and mechanical (drop PR1+PR2 stacking, restructure into 5 commits including the hip_sync_kind rename, gate tests with #ifdef ROCTRACER_FALLBACK); a fresh L1 + L2 run on Shark26 is in progress and will be posted as a comment on this PR.

Out of scope / follow-up

• Inter-stream sync metadata on the rocprofiler-sdk backend -- reuses the shared types added in this PR; separate PR.
• HIP graph APIs (hipGraphLaunch, etc.) -- separate concern.
• Non-blocking query APIs (hipEventQuery, hipStreamQuery) -- not captured because they don't create sync dependencies.

---

Fork PR for self-review. Will be retargeted at pytorch/kineto:main when validation completes.

[ajassani]

Validation: standalone branch on Shark26 (4x MI210, ROCm 7.2.3)

Branch HEAD: b481aca (5 commits ahead of upstream/main).

L1: libkineto unit tests (USE_ROCPROFILER_SDK=OFF)

Build: cmake -DKINETO_BACKEND=rocm -DUSE_ROCPROFILER_SDK=OFF -DLIBKINETO_BUILD_TESTS=ON. Configures and builds cleanly with HIP 7.2.53211 + cmake 4.3.2 on the modern ROCm install -- the build-infra commit (CMake opt-out + init.cpp guard) is doing its job.

RocmActivityProfilerTest: [ PASSED ] 13 tests, 0 failed (96 ms total).

Pre-existing (8)	New in this PR (5)
SyncTrace	InterStreamDependencyTest
HtoDMemcpyUsesRuntimeStreamWhenAsyncQueueIsZero	StreamWaitEventFutureCorrelation
HtoDMemcpyKeepsNonzeroAsyncQueue	EventMapClearedOnReset
HtoDMemcpyStaysOnZeroWhenRuntimeStreamMapsToMultipleQueues	EventSynchronizeResolvesProducer
GpuNCCLCollectiveTest	UnresolvedWaitStillEmitsEventId
GpuUserAnnotationTest
SubActivityProfilers
JsonGPUIDSortTest

All 5 new tests parse the actual emitted trace JSON and assert on field presence + correlation linkage (hip_sync_kind, wait_on_stream, wait_on_hip_event_record_corr_id, etc.), so this also validates the sync_type -> hip_sync_kind rename.

L2: real PyTorch cross-stream workload

PyTorch 2.10.0a0 (HIP 7.2.53211) on 4x MI210. Workload: 5 iterations of matmul on stream0 -> event.record -> stream1.wait_event(ev) -> matmul on stream1 -> sync. Profiler enabled, trace exported via prof.export_chrome_trace.

Note: this PyTorch was built against the prior PR3 kineto (pre-rename, pre-restructure), so the trace shows sync_type -- the field name as it existed in PR3. This L2 confirms the feature works end-to-end in a real workload; the field rename to hip_sync_kind is L1-validated.

Trace counts:

• hipEventRecord: 5
• hipStreamWaitEvent: 5

Sample hipEventRecord args:

cid: 81
correlation: 18
hip_event:  0x45fe8230
hip_stream: 0x460d78e0

Sample hipStreamWaitEvent args:

cid: 348
correlation: 20
sync_type: stream_wait_event           <-- this becomes hip_sync_kind post-rename
hip_stream: 0x460ff9a0
hip_event:  0x45fe8230                 <-- same handle as the record
wait_on_hip_event_id: 0x45fe8230       <-- same handle
wait_on_stream: 0x460d78e0             <-- producer stream from the record
wait_on_hip_event_record_corr_id: 18   <-- matches the record's correlation=18

Cross-checks:

• wait_on_hip_event_record_corr_id=18 matches the producing hipEventRecord's correlation=18 -- the trace consumer can follow the link.
• wait_on_stream=0x460d78e0 matches the record's hip_stream=0x460d78e0 -- producer stream attribution is correct.
• hip_event is consistent across record (0x45fe8230) and wait (0x45fe8230).

Summary

Layer	Result
L1 -- libkineto unit tests on standalone branch	13/13 PASSED including the 5 new inter-stream tests and the rename
L2 -- real PyTorch cross-stream workload	All new fields emit correctly, correlation linkage verified, producer-stream attribution verified

The build-infra changes (CMake + init.cpp guard) work as intended -- the roctracer backend builds cleanly from upstream on ROCm 7+ via -DUSE_ROCPROFILER_SDK=OFF.

PR is ready for upstream review.

[ajassani]

Review feedback addressed

Thanks for the careful read. All three findings are real -- pushed three commits that fix them and added covering tests for the two correctness issues.

Commits

SHA	Fix	Lines
d038be8	<algorithm> include	+1
b7a4737	hipEventDestroy hook -> unrecordEvent evicts from g_eventMap	+89 / -0 (incl. new test)
09fb8ee	Defer producer resolution to buffer-flush time	+159 / -20 (incl. new test)

Detail per finding

1. Event-handle reuse can resolve to stale producers. The earlier reassurance was incomplete -- the reviewer is right that resolveWait is collision-safe ONLY when the reused handle gets re-recorded before any wait. Apps that destroy + recreate + wait-without-record (badly-written, but observable) would have seen stale producer links in the trace.

Hook HIP_API_ID_hipEventDestroy in RoctracerLogger::api_callback and call the new unrecordEvent(void*) which erases the entry from g_eventMap. Also emits a hipEventDestroy runtime row in the trace, matching CUPTI's cudaEventDestroy emission.

Side effects:

• Bounded map memory under destroy-heavy workloads (positive).
• +1 runtime row per destroy: in PyTorch's event-pool model this is dozens per process; sub-0.1% of typical trace size.

New test EventMapEvictsOnDestroy covers the exact scenario from the review: record at corr=10, destroy, wait at corr=30 (must not resolve), then re-record at corr=40 and verify the new wait at corr=50 attributes to the new producer.

2. Producer lookup done too early for out-of-order callback delivery. Real bug in multi-threaded HIP callers (PyTorch with autograd backward on a separate thread is the canonical case).

Moved the resolveWait() call out of the hipStreamWaitEvent and hipEventSynchronize cases of api_callback and into a new RoctracerLogger::resolvePendingSyncs() pass that runs from RocmActivityProfiler::processGpuActivities right before rows are emitted. By that point all hipEventRecord callbacks for the trace have been delivered, so the resolution sees the final g_eventMap state.

Two overloads of resolvePendingSyncs are exposed -- a no-arg form that operates on the singleton's rows_ (production), and an explicit-buffer form (used by unit tests via MockRocActivities, whose activities_ buffer is disjoint from the singleton). The mock's processActivities now calls the explicit form before iterating, so all existing tests exercise the new code path.

No change to the emitted JSON fields. Trace size and structure are identical; only the resolution timing moves.

New test StreamWaitEventCallbackArrivesBeforeRecord reverses the buffer order: the wait row is pushed BEFORE its producing record arrives, and the test asserts the resolved producer attribution appears in the final trace JSON (i.e. deferred resolution did its job).

3. Missing <algorithm> include. Added.

Validation

Re-ran L1 on Shark26 (4x MI210, ROCm 7.2.3, USE_ROCPROFILER_SDK=OFF). Build cleanly; all tests pass:

[==========] 15 tests from 1 test suite ran. (97 ms total)
[  PASSED  ] 15 tests.

Breakdown:

• 8 pre-existing tests (SyncTrace, HtoDMemcpy* x3, GpuNCCLCollectiveTest, GpuUserAnnotationTest, SubActivityProfilers, JsonGPUIDSortTest)
• 5 inter-stream-dep tests from earlier (InterStreamDependencyTest, StreamWaitEventFutureCorrelation, EventMapClearedOnReset, EventSynchronizeResolvesProducer, UnresolvedWaitStillEmitsEventId)
• 2 new tests from this round (StreamWaitEventCallbackArrivesBeforeRecord, EventMapEvictsOnDestroy)

Worth noting that ALL existing tests now route through resolvePendingSyncs on their way through the mock (the mock's processActivities calls it before iterating), so the deferred-resolution path is exercised broadly, not just by the new test.