Repository navigation
Submodules fix for SDK CI jobs - #17
Merged
Merged
Conversation
ammallya
pushed a commit
that referenced
this pull request
Aug 6, 2025
- Porting from ROCm/omnitrace#411 - Improve OMPT support - Add OpenMP target example to testing - Update Timemory submodule to use ROCm/Timemory rather than NERSC/Timemory - Update `actions/upload-artifacts` to v4 - Standardize the `cmake_minimum_required` to 3.18.4 across workflows, project, and examples - Updated Ubuntu 20.04 workflows [ROCm/rocprofiler-systems commit: 7dce592]
ammallya
pushed a commit
that referenced
this pull request
Aug 6, 2025
- Porting from ROCm/omnitrace#411 - Improve OMPT support - Add OpenMP target example to testing - Update Timemory submodule to use ROCm/Timemory rather than NERSC/Timemory - Update `actions/upload-artifacts` to v4 - Standardize the `cmake_minimum_required` to 3.18.4 across workflows, project, and examples - Updated Ubuntu 20.04 workflows [ROCm/rocprofiler-systems commit: 9da7365]
ammallya
pushed a commit
that referenced
this pull request
Aug 7, 2025
* Adding tool pc sampling support Fixing merge issue tool support on SDKupdates link amd-comgr Sanitizer failure fix fix format Addressing review comments misc fix Adding dispatch id to the CSV output AddingCHANGELOG [ROCProfV3][PC Sampling] Initial ROCProfV3 PC sampling tests for JSON and CSV formats (#17) ROCProfV3 initial tests for JSON and CSV output. Simple kernels that simplify the verification of samples to instruction decoding has been introduced. removing option to enable pc sampling explicitly Adding documentation no pc-sampling option in tests anymore Addressing review comments Updating docs an option for choosing whether all units must be sampled try ignoring PC sampling tests (#36) * run pc-sampling tests on MI2xx runners * use v_fmac_f32 instead of s_nop 0 in tests * fixing docs [ROCm/rocprofiler-sdk commit: 50b185b]
systems-assistant Bot
pushed a commit
that referenced
this pull request
Aug 10, 2025
## Associated JIRA ticket number/Github issue number [SWDEV-461087](https://ontrack-internal.amd.com/browse/SWDEV-461087) ## What type of PR is this? (check all applicable) - [ ] Refactor - [ ] Feature - [ ] Bug Fix - [ ] Optimization - [ ] Documentation Update - [ ] Continuous Integration ## What were the changes? <!-- Please give a short summary of the change. --> ## Why are these changes needed? <!-- Please explain the motivation behind the change and why this solves the given problem. --> ## Updated CHANGELOG? <!-- Needed for Release updates for a ROCm release. --> - [ ] Yes - [ ] No, Does not apply to this PR. ## Added/Updated documentation? - [ ] Yes - [ ] No, Does not apply to this PR. ## Additional Checks - [ ] I have added tests relevant to the introduced functionality, and the unit tests are passing locally. - [ ] Any dependent changes have been merged.
jayhawk-commits
pushed a commit
that referenced
this pull request
Aug 18, 2025
[ROCm/hipother commit: 2366ff9]
ammallya
pushed a commit
that referenced
this pull request
Jan 21, 2026
update the link for MPI/UCX compilation
ammallya
pushed a commit
that referenced
this pull request
Jan 21, 2026
update the link for MPI/UCX compilation [ROCm/rocshmem commit: ee6b4ad]
ammallya
pushed a commit
that referenced
this pull request
Apr 9, 2026
ammallya
pushed a commit
that referenced
this pull request
Apr 9, 2026
[ROCm/hipFile commit: a89c605]
7 of 9 tasks
ammarwa
added a commit
that referenced
this pull request
Apr 19, 2026
…ride path, skip subdirs in config copy Finding #5: Add smoke_test field to _Dep; opencode registry gets smoke_test=["--version"]. check_tool_available binary branch now runs the smoke command — OSError, non-zero exit, or timeout each return (False, detail). Finding #6: PERFXPERT_OPENCODE_PATH pointing to a nonexistent file now raises FileNotFoundError immediately instead of silently falling back to bundled/PATH lookup. Finding #9: _prepare_runtime_config_dir guards `if not f.is_file(): continue` before read_bytes() to avoid crashing on subdirectories. Finding #17: opencode_launcher module docstring clarifies opencode is a system dependency (Phase 8 deliverable), not bundled in this release. Tests: new smoke-test OSError/non-zero/timeout tests in test_tooldep; test_resolve_binary_raises_when_override_missing replaces the old fallback test; test_prepare_runtime_config_dir_skips_subdirectories added. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ammarwa
added a commit
that referenced
this pull request
Apr 23, 2026
… polish + contributing docs (#5174) ## Summary Proven-optimizations catalog, nightly benchmark harness, contributing guides + RFC process + architecture refresh, `perfxpert doctor` polish, Phase 7.1 `ai_analysis/` module deletion, and the convergence-review fixes that establish real legacy-vs-agentic parity. This is the last PR in the original retirement series; subsequent PRs (#5179, #5180) add the bundled opencode fork and the docs-audit toolchain. Stacks on #5173. Next in stack: #5179. ## What lands ### Phase 7.1 — `perfxpert/ai_analysis/` module removed Commit `cb03e82c36` deletes the entire `ai_analysis/` module — **21 files, ~11.7 KLOC**. All execution now routes through `perfxpert/agents/`; the dispatcher and the `PERFXPERT_LEGACY` env toggle are gone. This completes the retirement started in #5173: there is no longer a "legacy path" to opt back into. ### Proven-optimizations catalog (10 canonical case studies) - `knowledge/proven_optimizations.yaml`: vgpr_reduction, lds_tiling, mfma_enablement, memory_coalescing, fast_math, hip_stream_overlap, kernel_fusion, device_sync_removal, warp_primitives, cache_blocking - Dual-schema fixture integration: synthetic `duration_ns` + real rocprofv3 UUID-suffixed `(end_ns - start_ns)` tables - `tests/fixtures/*.db`: 5 fixtures re-profiled with `--pmc` on real MI300X hardware ### Nightly benchmark harness + orchestration ### Contributing guides + RFC process + architecture refresh ### `perfxpert doctor` polish (8 checks incl. opencode bundled + LLM providers) ## Review findings addressed (29 findings, 5 fix batches) ### Batch 1 — Gate cascade fidelity (3 commits) - `gate-2-sol-signature`: `_run_sol_gate` took wrong keyword; unit tests mocked the function so the TypeError went undetected. Two-tier check now: hard cap on speedup ratio (>50× rejected arch-independent) + optional absolute FLOPS check via `sol.sanity_check`. 13 new un-mocked integration tests (`TestRunSolGateUnmocked`). - `gate-4-hot-kernel-key-unit`: `regression.compare_runs` returns `per_kernel_deltas` (list of dicts with `delta_pct` as a **fraction**, not percent), not `hot_kernels`. Old code iterated a non-existent key so Gate 4 silently treated all regressions as OK. Fraction↔percent conversion corrected. - `gate-5-skip-verdict`: verdict strings no longer misleading on skip (`candidate_binary=None` path). ### Batch 2 — Schemas, validators, framework invariants (6 findings) - #4: Pydantic cross-field validators on `GateVerdictModel` (status/failing_gate coupling), `CorrectnessOutput` (verdict/action), confidence clamps `Field(..., ge=0.0, le=1.0)`, typed `Literal` for provider - #12: `Agent` dataclass `frozen=True`; `__post_init__` validates layer in (0,1,2) - #7: `_parse_headers` raises `ValueError` on malformed JSON instead of silent `{}` - #10, #18: tasks module logs resolved store path; stale READ_ONLY docstrings rewritten - #17: README clarifies opencode is system dependency; doctor warns when PERFXPERT_TASK_ROOT unset ### Batch 3 — External tool verifier + fence alignment (6 findings) - N28: anthropic/openai SDK imports routed through `require_tool()` with install-hint on missing - N29: all 8 agent fence slices aligned to actual agent tool allowlists - #5, #6, #9, #17: smoke-test opencode binary (not just presence), hard-fail on bad `PERFXPERT_OPENCODE_PATH` override, skip subdirs in config copy - `tooldep`: user-home path search + `PERFXPERT_ROCPROF_TRACE_DECODER_PATH` env support ### Batch 4 — Classifier + data-provenance fidelity (2 findings) - **#1**: classifier no longer swallows DB errors silently (was returning empty metrics → `data_insufficient`; now logs + propagates error markers) - **#26 (priority user-flagged)**: confidence now weighted by `evidence_strength × match_ratio` instead of raw rule-count normalization — partial-PMC classifications no longer over-confident. New `test_partial_pmc_confidence` covers trace-only vs full-PMC paths. ### Batch 5 — Tests coverage gaps (9 findings) - #20: gate cascade short-circuit coverage (Gates 2/3/4) - #21: `_TraceAnalysisAdapter` shim delegation tests - #22: trace-only `memcpy_pct` path in `classify_from_metrics` - #23: `anthropic.APITimeoutError` normalization path - #24: Root agent lambda stub guard (`test_root.py::test_lambda_stubs_never_invoked`) - #25: MCP tool coverage across 5 representative tools - #27: flaky `time.sleep` → `proc.wait(timeout=5)` - #14, #15, #16: stale comments + docstring rewrites (api.py, __main__.py, recommendation.py) ## Parity story **Problem found during review**: parity test wiring was toggling a dead flag (`PERFXPERT_USE_AGENTS`) after the retirement PR flipped the default — both legs were running the agentic path. "Parity" passed vacuously. **Fixes applied (Decision A+C):** - **A**: Parity runner now toggles the operative legacy opt-out so legacy and agentic paths are genuinely distinct; Tier-1 trace-only fixtures `xfail`ed until re-profiled with `--pmc`. - **C**: Re-profiled all 5 fixtures (`baseline`, `memory_bound`, `compute_bound`, `latency_bound`, `regressed`) on real MI300X with `rocprofv3 --pmc`: - `compute_bound`: VALU-bound kernel (not MFMA, to legitimately match Compute-Bound classification) - `memory_bound` / `baseline`: 128 MB H2D+D2H loop - `latency_bound` / `regressed`: 6,000 tiny kernels - `_build_analysis_result` returns `api_overhead` (not `latency`) for `overhead_pct > 25%` (legacy heuristic fix) - E2E + dashboard tests updated for evidence-weighted classification **Result**: `parity_snapshots/_aggregate.json` → **100% agreement on PMC fixtures** (10/10 bottleneck + rec_technique signals). `rec_type` signal skipped where the agentic path can't populate without an LLM (documented asymmetric None). ## Convergence review round (2026-04-18) - **Brand scrub**: zero-tolerance removal of the prior codename across code, tests, patches, env-var names, and documentation. - **CLI flag plumbing**: all 10 CLI flags (`--prompt`, `--llm`, `--llm-model`, `--att-dir`, `--pc-sampling`, `--source-dir`, `--format`, `-d/-o`) now wired end-to-end via an `analysis_options` side-channel on `Context`. - **PMC shard UNION** across multiple `.db` files uses column-intersection and emits a `RuntimeWarning` listing dropped columns. - **FallbackProvider** has a typed exception taxonomy (`LLMRateLimitError`, `LLMAuthenticationError`, `LLMTransientError`, `LLMFatalError`). Auth/fatal errors surface immediately; only transient + rate-limit cascade. The provider is wired into `build_session`; `PERFXPERT_LLM_FALLBACK_CHAIN` now has runtime effect. ## Test plan - [x] 1036 pass / 3 skip / 0 fail from `experimental/python/perfxpert/` - [x] Core suites — 100% pass: `tests/test_agents + test_providers + test_runtime + test_tools` - [x] Parity ship-gate — 100%: `tests/test_parity/` (12 passed, 5 rec_type skips documented) - [x] Gate cascade — 100%: 28 tests including 13 new un-mocked SOL integration tests (Batch 1 recovery) - [x] E2E brain: `perfxpert-code run -m openai/gpt-4o-mini "..."` → MCP → Python → `81.7 TFLOPS` + AMD footer - [x] `perfxpert doctor`: all 8 checks green (34 MCP tools at this PR boundary; 43 after #5179+#5180; opencode 1.4.11 bundled; 3/5 LLM providers configured) - [x] No legacy opt-out path remains (Phase 7.1 removed the dispatcher and the `PERFXPERT_LEGACY` toggle) - [ ] Live MI300X nightly run (requires hardware; nightly workflow will refresh baseline) - [ ] Full interactive TUI (requires TTY; E2E can only prove banner-emitted) --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
mradosav-amd
added a commit
that referenced
this pull request
Apr 30, 2026
…ks, real_perfetto_sink Add three standalone integration test binaries that exercise production code paths requiring the full rocprofiler-systems static library: - real_overflow_trigger_test.cpp: smoke tests for perf_event_open-backed OverflowTriggerPolicy (default-constructed state, stop/start no-ops, dtor, type properties). Separate binary because perf.cpp symbols live in rocprofiler-systems-object-library which the unit test binary must not link. - sampling_service_production_hooks_test.cpp: verifies shutdown_production_wiring clears tl_sampler_state_vp, tl_offload_vp, tl_logical_tid (AC-11, NFR-TS-2). Uses configure_settings(false) via GTest Environment to avoid recursive_init_error. - real_perfetto_sink_test.cpp: verifies empty-sample guard and perfetto-disabled early-return (ROCPROFSYS_TRACE=false default) for both emit_timer and emit_overflow. All three link rocprofiler-systems::librocprofiler-systems-static for the full symbol set (thread_info, tracing, perf, TLS state). Added rocprofiler-systems- interface-library PUBLIC to the sampling OBJECT library to supply timemory include paths needed by real_overflow_trigger.hpp → library/perf.hpp → core/exception.hpp. Results: 882/882 unit tests PASS, 4/4 overflow-trigger PASS, 4/4 perfetto-sink PASS, 1/1 production-hooks PASS. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
bgopesh
added a commit
that referenced
this pull request
May 18, 2026
- loader_win.cpp (C2): use exclusive upper bound for SizeOfImage so the [start, last) range matches _in_address_range's half-open semantics (addr >= start && addr < last); was off-by-one with last = start + SizeOfImage - 1. - loader_win.cpp (C3): skip entries where SizeOfImage == 0 (forwarder DLLs / synthetic MEM_IMAGE regions) before computing the range; without the guard last wraps to UINTPTR_MAX, causing every address to appear in-range and breaking secure-mode. - loader_win.cpp (C5): guard both EnumProcessModulesEx iteration loops with `if(modules[i] == nullptr) continue` so snapshot-race null slots are skipped rather than passed to GetProcAddress / GetModuleInformation. - rocprofiler_register.cpp (#19): wrap `#define GNU_SOURCE 1` in `#if !defined(_WIN32)` -- the macro is a Linux/glibc extension and is a no-op (or noise) on Windows. - tests/common/mock_export.hpp (new, #17): extract ROCP_REG_TEST_MOCK_EXPORT into a standalone header with no dependencies; each mock library .hpp now includes this instead of redefining the macro locally, eliminating the 6-way duplication. ctest 62/62 pass on Windows (MSVC + Ninja); Linux behaviour unchanged. Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
powderluv
added a commit
to powderluv/rocm-systems
that referenced
this pull request
Jun 24, 2026
… -> MES schedules+dispatches on Windows The gfx1201 MES could not service queues on any of the three OSes (macOS ROCm#17, Linux ROCm#21 ~14-dispatch ceiling). On the Windows WDDM/VFIO-passthrough setup the host doorbell write never reaches the GPU's doorbell monitor at all (proven by an isolation probe: the direct compute HQD only ever advanced via the MMIO CP_HQD_PQ_WPTR poke / WPTR_POLL, never the doorbell). So drive the MES by MMIO-poking CP_HQD_PQ_WPTR instead of relying on the doorbell, gated by ROCR_WINDOWS_MES_MMIO_WPTR: - SubmitMesApiFrameOnRing: after writing the scheduler/KIQ ring + the in-mem wptr, SelectHqd(me=3,pipe,0) + write CP_HQD_PQ_WPTR_LO/HI = new_wptr before the doorbell ring -> the MES scheduler services its API ring. - SubmitDirectQueue (mes_backed): same poke on the MES-mapped compute HQD (me=1, DirectQueuePipe/Hqd(queue_index)) -> the mapped queue executes. Plus an optional ROCR_WINDOWS_MES_WPTR_POLL diagnostic (CP_PQ_WPTR_POLL_CNTL) and WPTR_POLL/CNTL readbacks in DumpMesRingState. Both flags default OFF, so macOS/Linux behavior is byte-for-byte unchanged. Validated on Windows gfx1201 hardware (TheRock userspace_driver/wddm_lite harness over this lite:: code): single MES NOP+fence PASS; mesmulti = 1360 back-to-back MES-backed dispatches with NO ceiling (the 1024-dword ring wraps 21x) -> blows past the ~14 ceiling of ROCm#21; meskern = a real fill_kernel through the MES-backed queue -> out[0..63]=0xDEADBEEF. The MMIO-poke (not WPTR_POLL, not the IH ring) is the fix. Likely transferable to the macOS ROCm#17 / Linux ROCm#21 MES ceiling. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> (cherry picked from commit 9dc021c6add13152544bd6dd2645f88bad7dce16)
prasanna-amd
added a commit
that referenced
this pull request
Aug 26, 2026
…ntegration Add monotonic generation counter per pool slot to detect stale callbacks from reclaimed collectives, resolving the 0==0 race in pool reclaim and all callback-lifetime concerns (threads #4, #6, #8, #9). Add atomic ref-count to acclCommContext so acclPluginFinalize defers free() until all outstanding callbacks have drained (thread #2). Wire accl plugin into the parent RCCL CMake build (thread #20), add CXX_VISIBILITY_PRESET/VISIBILITY_INLINES_HIDDEN (thread #1), and bump ACCL_MAX_CHANNELS from 64 to 256 (thread #11). Fix acclShouldFinalize: nChannels==0 collectives (onerank path) finalize immediately on coll stop; guard channelId against nChannels (thread #10, #12). Free leaked ProxyOp when coll's proxyOpIndices is full (thread #13). Rename firstCpuStart to firstKernelCpuStartUs for clarity (thread #14). Fix warmup off-by-one (sn <= warmup → sn < warmup, thread #7), use try/finally for output file handle (thread #15), fix test fixtures to start at coll_sn=0 (thread #16), remove stale ACCL_PROFILER_WARMUP_ITERS docs (thread #17), sync bottleneck categories with classify() (thread #18), add NVIDIA Apache-2.0 header to accl_shim.h (thread #19). Add tests: ProxyStep lifecycle, channel bounds, kernel timing assertions (threads #3, #12, #21).
1 task done
1 task done
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
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.
No description provided.