Enhancement: Add rocprofv2 trace support for AMD GPUs by shcho · Pull Request #817 · microsoft/superbenchmark

Shenghsun Cho (shcho) · 2026-05-19T18:17:40Z

Description
Extend trace generation to support AMD GPUs using rocprofv2

runner.py: Add SB_ENABLE_ROCPROF/SB_ROCPROF_TRACE_DIR env vars to enable rocprofv2 profiling (--hip-trace --kernel-trace --plugin json) in local, torch.distributed, and mpi modes
pytorch_base.py: Extend GPU guard to support ROCm (torch.version.hip) so PyTorch profiler works on AMD GPUs

- runner.py: Add SB_ENABLE_ROCPROF/SB_ROCPROF_TRACE_DIR env vars to enable rocprofv2 profiling (--hip-trace --kernel-trace --plugin json) in local, torch.distributed, and mpi modes - pytorch_base.py: Extend GPU guard to support ROCm (torch.version.hip) so PyTorch profiler works on AMD GPUs

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds ROCm profiling support alongside existing Nsight Systems tracing and updates model benchmark GPU detection to include AMD (HIP) builds.

Changes:

Add rocprofv2 command injection (gated by env vars) for local/distributed/mpi runner modes.
Introduce ROCm trace directory env var support (SB_ROCPROF_TRACE_DIR).
Expand PyTorch GPU check to treat HIP builds as GPU-capable.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File	Description
superbench/runner/runner.py	Adds optional rocprofv2 profiling prefixes/trace commands controlled by environment variables.
superbench/benchmarks/model_benchmarks/pytorch_base.py	Updates GPU detection to include ROCm/HIP PyTorch builds.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Address PR review: wrap all interpolated path/name segments in shlex.quote() to prevent command injection or broken commands when paths contain whitespace or shell metacharacters. Applied to both nsys and rocprofv2 trace commands across all three execution modes (local, torch.distributed, mpi).

Address PR review: the variable holds either an nsys or rocprofv2 prefix, so rename to trace_prefix to avoid implying Nsight-only behavior.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

Address PR review: torch.version.hip (and .cuda) may not exist on all PyTorch builds/versions, raising AttributeError. Use getattr with a None default to make the check compatible across all PyTorch builds.

Address PR review: add the conventional -- end-of-options separator after rocprofv2 flags so that benchmark command arguments starting with - are not misinterpreted as rocprofv2 options. Applied to all three execution modes (local, torch.distributed, mpi).

Shenghsun Cho (shcho) · 2026-07-07T18:53:49Z

@microsoft-github-policy-service agree company="Microsoft"

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.

Address PR review: shlex.quote() wraps values with spaces or shell metacharacters in single quotes, which would terminate the outer bash -lc '...' quoting used to dispatch commands. Add a helper that runs shlex.quote() and then replaces each ' with '\'' so the quoted value is safe to embed inside the outer single-quoted command string. Applied to all six nsys/rocprofv2 trace output paths.

Address PR review: previously SB_ENABLE_ROCPROF was silently ignored whenever SB_ENABLE_NSYS was also set, producing no traces and no diagnostic. Log a warning documenting that nsys takes precedence and explicitly disable rocprofv2 in that case.

Address PR review: clarify that ProfilerActivity.CUDA and event.cuda_time are aliases that cover HIP kernels on ROCm builds of PyTorch, so the same profiler code path works for both NVIDIA and AMD GPUs. Verified end-to-end on MI300x with ROCm 6.3.4.

Address PR review: the nsys/rocprofv2 selection + path quoting logic was copy-pasted across all three mode branches (local, torch.distributed, mpi). Extract __build_trace_command() so future changes to profiler flags, quoting, or additional tools live in a single place.

Apply yapf column-limit reflows and mark the _quote_for_bash_lc docstring as raw (D301) since it contains backslash escapes.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

rocprofv2 does not recognize the conventional -- separator and errors with 'Wrong option "--", Please use the following options:'. Drop the separator; rocprofv2 treats the first non-option token as the target executable, matching the syntax originally validated on MI300x. Add a comment noting the incompatibility so it is not reintroduced.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

superbench/benchmarks/model_benchmarks/pytorch_base.py:594

Grammar/capitalization in the comment: use “an NVIDIA” (not “a Nvidia”) for correct article usage and consistent vendor capitalization.

        # Check if this is a Nvidia or AMD GPU

Hongtao Zhang (polarG) · 2026-09-01T19:19:10Z

+    terminate the outer single-quoted context. Escape any single quotes as ``'\''`` so the value survives
+    both quoting layers.
+    """
+    return shlex.quote(value).replace("'", "'\\''")


[NON-BLOCKING] Add regression coverage for nested-shell quoting

This helper is the fix for the prior nested-shell quoting defect, but the existing runner tests never enable either profiler and therefore do not exercise it. Please add focused cases for spaces, apostrophes, command-substitution text, and semicolons, covering both the Docker bash -lc and skip-Docker bash -c command shapes.

Copilot AI review requested due to automatic review settings May 19, 2026 18:17

Copilot AI reviewed May 19, 2026

View reviewed changes

Comment thread superbench/runner/runner.py Outdated

Comment thread superbench/runner/runner.py

Comment thread superbench/runner/runner.py Outdated

Comment thread superbench/runner/runner.py Outdated

Comment thread superbench/benchmarks/model_benchmarks/pytorch_base.py Outdated

Shenghsun Cho added 2 commits May 19, 2026 18:46

Rename nsys_prefix to trace_prefix for tool-agnostic naming

a9485ef

Address PR review: the variable holds either an nsys or rocprofv2 prefix, so rename to trace_prefix to avoid implying Nsight-only behavior.

Shenghsun Cho (shcho) marked this pull request as ready for review May 20, 2026 02:35

Shenghsun Cho (shcho) requested a review from a team as a code owner May 20, 2026 02:35

Copilot AI review requested due to automatic review settings May 20, 2026 02:35

Copilot AI reviewed May 20, 2026

View reviewed changes

Comment thread superbench/benchmarks/model_benchmarks/pytorch_base.py Outdated

Comment thread superbench/runner/runner.py Outdated

Comment thread superbench/runner/runner.py

Comment thread superbench/runner/runner.py Outdated

Shenghsun Cho added 2 commits July 7, 2026 18:19

Use getattr for safe torch.version.cuda/hip access in pytorch_base.py

8a22f3e

Address PR review: torch.version.hip (and .cuda) may not exist on all PyTorch builds/versions, raising AttributeError. Use getattr with a None default to make the check compatible across all PyTorch builds.

Copilot AI review requested due to automatic review settings July 21, 2026 20:10

Copilot started reviewing on behalf of Hongtao Zhang (polarG) July 21, 2026 20:11 View session

Copilot AI reviewed Jul 21, 2026

View reviewed changes

Hongtao Zhang (polarG) requested changes Jul 21, 2026

View reviewed changes

gusui-msft requested review from gusui-msft and Hongtao Zhang (polarG) July 22, 2026 18:08

Shenghsun Cho added 5 commits July 22, 2026 21:25

Fix yapf formatting and flake8 D301 in runner.py / pytorch_base.py

b4b5e54

Apply yapf column-limit reflows and mark the _quote_for_bash_lc docstring as raw (D301) since it contains backslash escapes.

Copilot AI review requested due to automatic review settings July 22, 2026 22:51

Shenghsun Cho (shcho) force-pushed the dev/choshenghsun/reliaguard-tracegen-mi300x branch from a81f9be to b4b5e54 Compare July 22, 2026 22:51

Copilot started reviewing on behalf of Shenghsun Cho (shcho) July 22, 2026 22:52 View session

Copilot AI reviewed Jul 22, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 28, 2026 16:09

Copilot started reviewing on behalf of Shenghsun Cho (shcho) July 28, 2026 16:09 View session

Copilot AI reviewed Jul 28, 2026

View reviewed changes

gusui-msft approved these changes Aug 6, 2026

View reviewed changes

Hongtao Zhang (polarG) approved these changes Sep 1, 2026

View reviewed changes

Uh oh!

Conversation

Shenghsun Cho (shcho) commented May 19, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Shenghsun Cho (shcho) commented Jul 7, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Hongtao Zhang (polarG) Sep 1, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants