[AtlantaPepsi]

Motivation

• NIC executor check against max message size
• fixing logical CU ID report
• inclusion of gfx1250-strict target
• Pingpong latency testing integration and presets

Technical Details

Test Plan

• Validate HSA_DISABLE_GFX12_STRICT=0 ./TransferBench output of gfx1250-strict target
• On gfx1250, gfx942 and gfx950 compare SHOW_ITERATIONS output CU ID against CU_MASK bits
• Latency Testing
  • p2p_latency preset: in/cross-domain, and AMD and Nvidia platform
  • Concurrent transfers + pingpongs run, single/multistream
  • validation of all memory types support

Test Result

p2p_latency example output

[Latency Related]
GPU_MEM_TYPE         =            0 : Using default GPU memory for flags (0=default, 1=fine-grained, 2=uncached, 3=managed)
NUM_GPU_DEVICES      =            8 : Using 8 GPUs
NUM_LAPS             =         1000 : Timing 1000 round trips per iteration
USE_REMOTE_READ      =            0 : Executors write to their partner's memory and poll their own

Pingpong round-trip latency per lap (us), each pair run by itself
[1000 laps] [default GPU memory flags] [remote write / local poll]
 PING\PONG     GPU 00     GPU 01     GPU 02     GPU 03     GPU 04     GPU 05     GPU 06     GPU 07
    GPU 00      0.550      1.973      4.093      4.504      4.155      4.501      4.092      4.504
    GPU 01      1.956      0.585      4.502      4.930      4.507      4.860      4.505      4.925
    GPU 02      4.079      4.501      0.548      1.993      4.095      4.503      4.067      4.503
    GPU 03      4.501      4.919      1.995      0.592      4.503      4.873      4.502      5.010
    GPU 04      4.111      4.501      4.050      4.501      0.578      1.973      4.111      4.501
    GPU 05      4.501      4.901      4.501      4.837      1.993      0.581      4.503      4.818
    GPU 06      4.060      4.501      4.034      4.500      4.088      4.501      0.562      1.954
    GPU 07      4.502      4.957      4.502      4.873      4.511      4.841      1.956      0.575

Concurrent transfer + pingpong hybrid run example output

./TransferBench cmdline 16M "6 16 (G0->G1->G1) (G1->G2->G2) (G2->G3->G3) (G3->G0->G0
) (N G0 G1 +1000 N G1 G0) (C0 G0 G1 +50 C0 G1 G0)"

Test 1:
-------------------┬--------------┬------------┬-------------------┬---------------------------
  Executor: GPU 00 │ 302.684 GB/s │   0.055 ms │    16777216 bytes │ 332.617 GB/s (sum)
-------------------┼--------------┼------------┼-------------------┼---------------------------
     Transfer 3    │ 332.617 GB/s │   0.050 ms │    16777216 bytes │ G3 -> G0:16 -> G0
     PingPong 4    │     1.046 us │   1.046 ms │         1000 laps │ N->G0->G1 <+> N->G1->G0
     PingPong 5    │     1.544 us │   0.077 ms │           50 laps │ C0->G0->G1 <+> C0->G1->G0
-------------------┼--------------┼------------┼-------------------┼---------------------------
  Executor: GPU 01 │ 331.512 GB/s │   0.051 ms │    16777216 bytes │ 355.359 GB/s (sum)
-------------------┼--------------┼------------┼-------------------┼---------------------------
     Transfer 0    │ 355.359 GB/s │   0.047 ms │    16777216 bytes │ G0 -> G1:16 -> G1
-------------------┼--------------┼------------┼-------------------┼---------------------------
  Executor: GPU 02 │ 345.778 GB/s │   0.049 ms │    16777216 bytes │ 350.870 GB/s (sum)
-------------------┼--------------┼------------┼-------------------┼---------------------------
     Transfer 1    │ 350.870 GB/s │   0.048 ms │    16777216 bytes │ G1 -> G2:16 -> G2
-------------------┼--------------┼------------┼-------------------┼---------------------------
  Executor: GPU 03 │ 358.855 GB/s │   0.047 ms │    16777216 bytes │ 364.532 GB/s (sum)
-------------------┼--------------┼------------┼-------------------┼---------------------------
     Transfer 2    │ 364.532 GB/s │   0.046 ms │    16777216 bytes │ G2 -> G3:16 -> G3
-------------------┼--------------┼------------┼-------------------┼---------------------------
   Aggregate (CPU) │  58.392 GB/s │   1.149 ms │    67108864 bytes │ Overhead 1.094 ms
-------------------┴--------------┴------------┴-------------------┴---------------------------

Submission Checklist

• Look over the contributing guidelines at https://github.com/ROCm/TheRock/blob/main/GOVERNANCE.md#pull-requests.

[Copilot]

Copilot review overview

🟡 Changes recommended

Unresolved critical and moderate correctness, portability, parsing, and build-target issues remain.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 4 · 4

Open (8)

• Restrict GetXccId CDNA5 instruction to CDNA5 targets · New
• Include pong subindex when enabling GFX subindices · New
• Emit advanced transfer fields before the pong token · New
• Align TDM device guard with supported architectures · New
• Avoid indexing stale or empty transfer results after failure · New
• Validate NIC message size for the selected port · New
• Fix pingpong timing scaling by subiterations · New
• Reject multi-device pingpong memory halves · New

What changed in this PR

Adds pingpong latency testing, NIC message-size validation, logical CU reporting fixes, and gfx1250-strict support.

Changes:

• Adds pingpong parsing, execution, timing, and latency presets.
• Adds NIC max_msg_sz validation and reporting.
• Updates GPU architecture and TDM handling.

File	Review summary
src/​header/​TransferBench.hpp	Critical and moderate issues in XCC handling, pingpong validation, NIC limits, subindex selection, dump parsing, and timing scaling.
src/​header/​tdmCopy.h	Critical architecture guard mismatch enables unsupported TDM targets.
src/​client/​Utilities.hpp	Reviewed result and topology utilities.
src/​client/​Topology.hpp	Reviewed NIC message-size reporting.
src/​client/​Presets/​Presets.hpp	Reviewed latency preset registration.
src/​client/​Presets/​Latency.hpp	Moderate issues with failed-run result handling and diagonal pair measurement.
src/​client/​EnvVars.hpp	Reviewed pingpong configuration variables.
src/​client/​Client.cpp	Reviewed pingpong transfer display updates.
docs/​install/​build_from_source.rst	Reviewed strict GPU target documentation.
CMakeLists.txt	Moderate issue: package build targets omit gfx1250-strict.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Copilot review overview

🟡 Changes recommended

Unresolved correctness issues remain in ping-pong execution, validation, parsing, and target gating.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 5 · 4

Open (9)

• Run unpinned pingpong parameters on only one XCC · New
• Align TDM device guard with supported architectures
• Emit advanced transfer fields before the pong token
• Include pong subindex when enabling GFX subindices
• Restrict GetXccId CDNA5 instruction to CDNA5 targets
• Reject multi-device pingpong memory halves
• Fix pingpong timing scaling by subiterations
• Validate NIC message size for the selected port
• Avoid indexing stale or empty transfer results after failure

[Copilot]

Copilot review overview

🟡 Changes recommended

Unresolved critical and moderate issues remain in pingpong execution, validation, result handling, and architecture gating.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 7 · 5

Open (12)

• Reject pingpong subindices below -1 · New
• Prevent persistent pingpong dependency cycles from hanging · New
• Run unpinned pingpong parameters on only one XCC
• Align TDM device guard with supported architectures
• Emit advanced transfer fields before the pong token
• Include pong subindex when enabling GFX subindices
• Restrict GetXccId CDNA5 instruction to CDNA5 targets
• Allow default numSubExecs for pingpong transfers · New
• Reject multi-device pingpong memory halves
• Fix pingpong timing scaling by subiterations
• Validate NIC message size for the selected port
• Avoid indexing stale or empty transfer results after failure

[Copilot]

Copilot review overview

🟡 Changes recommended

Unresolved correctness issues affect ping-pong execution, target guards, NIC validation, and package builds.

Review effort: Lite
Findings: 7 · 4

Open (11)

• Guard peer access setup to the executor's local MPI rank · New
• Prevent persistent pingpong dependency cycles from hanging
• Reject pingpong subindices below -1
• Run unpinned pingpong parameters on only one XCC
• Align TDM device guard with supported architectures
• Include pong subindex when enabling GFX subindices
• Restrict GetXccId CDNA5 instruction to CDNA5 targets
• Propagate RunTransfers failures from latency presets · New
• Allow default numSubExecs for pingpong transfers
• Reject multi-device pingpong memory halves
• Validate NIC message size for the selected port

Resolved since last review (3)

• Emit advanced transfer fields before the pong token
• Fix pingpong timing scaling by subiterations
• Avoid indexing stale or empty transfer results after failure

[Copilot]

🟡 Changes recommended

Unresolved critical and moderate issues remain in pingpong parsing and memory access, architecture guards, NIC validation, and latency failure propagation.

7 open findings

• Pingpong makes the GPU kernel dereference this destination pointer as a flag, but… · New
• In advanced transfer syntax, numSubExecs is never initialized because the parser only reads it in… · New
• Guard peer access setup to the executor's local MPI rank
• Prevent persistent pingpong dependency cycles from hanging
• Reject pingpong subindices below -1
• Include pong subindex when enabling GFX subindices
• Allow default numSubExecs for pingpong transfers

6 resolved since last review

• Run unpinned pingpong parameters on only one XCC
• Align TDM device guard with supported architectures
• Restrict GetXccId CDNA5 instruction to CDNA5 targets
• Propagate RunTransfers failures from latency presets
• Reject multi-device pingpong memory halves
• Validate NIC message size for the selected port

🧠 Review effort: Lite

---

Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

[Copilot]

🟡 Changes recommended

Unresolved critical and moderate correctness issues remain in ping-pong synchronization, target guards, validation, and error propagation.

8 open findings

• Ensure pingpong slot progression avoids parity-safe reuse · New
• In advanced transfer syntax, numSubExecs is never initialized because the parser only reads it in…
• Pingpong makes the GPU kernel dereference this destination pointer as a flag, but…
• Guard peer access setup to the executor's local MPI rank
• Prevent persistent pingpong dependency cycles from hanging
• Reject pingpong subindices below -1
• Include pong subindex when enabling GFX subindices
• Allow default numSubExecs for pingpong transfers

🧠 Review effort: Lite

---

Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

[Copilot]

🟡 Changes recommended

Unresolved critical and moderate correctness issues remain in pingpong execution, target handling, validation, and probe tooling.

10 open findings

• Zero-subexecutor path performs invalid result processing · New
• Ensure pingpong slot progression avoids parity-safe reuse
• In advanced transfer syntax, numSubExecs is never initialized because the parser only reads it in…
• Pingpong makes the GPU kernel dereference this destination pointer as a flag, but…
• Guard peer access setup to the executor's local MPI rank
• Prevent persistent pingpong dependency cycles from hanging
• Reject pingpong subindices below -1
• Include pong subindex when enabling GFX subindices
• Allow default numSubExecs for pingpong transfers
• Preset reference omits latency presets · New

🧠 Review effort: Lite

---

Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

[Copilot]

🟡 Changes recommended

Unresolved security-scan allowlist gaps and functional correctness issues remain.

12 open findings

• Global .lock allowlist hides credentials in lockfiles · New
• Generic secret allowlist suppresses real credential detections · New
• Zero-subexecutor path performs invalid result processing
• Ensure pingpong slot progression avoids parity-safe reuse
• In advanced transfer syntax, numSubExecs is never initialized because the parser only reads it in…
• Pingpong makes the GPU kernel dereference this destination pointer as a flag, but…
• Guard peer access setup to the executor's local MPI rank
• Prevent persistent pingpong dependency cycles from hanging
• Reject pingpong subindices below -1
• Include pong subindex when enabling GFX subindices
• Allow default numSubExecs for pingpong transfers
• Preset reference omits latency presets

Previously missed (3)

In code that hasn't changed since last review

tee pipeline aborts on SIGPIPE when truncated with head

.claude/​skills/​transferbench-debug/​examples/​topology-probe.sh:24

With set -o pipefail, truncating the tee pipeline with head can close the pipe while the binary is still writing; tee/the binary then exits on SIGPIPE and the script aborts before completing the probe. Use a reader that consumes the full stream (for example sed -n '1,60p') or capture first and truncate afterward.

Documented presets are not registered in presetFuncMap

.claude/​skills/​transferbench-run/​references/​presets.md:25

This reference presents these rows as runnable presets, but neither pcopy nor rwrite is registered in the current presetFuncMap (src/client/Presets/Presets.hpp:68-99), so following either command fails instead of launching a benchmark. Remove them or register the corresponding presets before documenting them here.

Packaging defaults omit gfx1250 GPU targets

CMakeLists.txt:224

Adding the target only to CMake's default list does not add it to the package build path: build_packages_local.sh passes its separate DEFAULT_GPU_TARGETS list to -DGPU_TARGETS, and that list still omits both gfx1250 and gfx1250-strict. Packages built without an explicit GPU_TARGETS override therefore will not contain the target advertised by this change; update the packaging defaults or explicitly document the scope.

🧠 Review effort: Lite

---

Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

[Copilot]

🔵 Needs a closer look

Unresolved correctness, documentation, packaging, and security-configuration issues remain.

28 open findings

• Pingpong-only path dereferences null timing parameter
• Skip pingpong resources during validation setup
• Generic secret allowlist suppresses real credential detections
• Global .lock allowlist hides credentials in lockfiles
• Zero-subexecutor path performs invalid result processing
• Ensure pingpong slot progression avoids parity-safe reuse
• In advanced transfer syntax, numSubExecs is never initialized because the parser only reads it in…
• Pingpong makes the GPU kernel dereference this destination pointer as a flag, but…
• Guard peer access setup to the executor's local MPI rank
• Prevent persistent pingpong dependency cycles from hanging
• Reject pingpong subindices below -1
• Include pong subindex when enabling GFX subindices
• Spin limit checked only every 1024 polls
• Diagnostic contains stale version 1.67 limitation
• Update packaged default GPU target architectures
• Sanity check passes expression where byte size is required
• Allow default numSubExecs for pingpong transfers
• Cmdline and dryrun arguments are reversed
• Quick dryrun uses invalid argument order
• Documented dryrun missing size argument

And 8 more that still need to be addressed.

🧠 Review effort: Lite

[Copilot]

🟡 Changes recommended

Unresolved correctness, packaging, documentation, and security-configuration issues remain, including a critical ping-pong capacity concern.

30 open findings

• Pingpong capacity warning can allow hangs or watchdog failures · New
• Pingpong-only path dereferences null timing parameter
• Skip pingpong resources during validation setup
• Generic secret allowlist suppresses real credential detections
• Global .lock allowlist hides credentials in lockfiles
• Zero-subexecutor path performs invalid result processing
• Ensure pingpong slot progression avoids parity-safe reuse
• In advanced transfer syntax, numSubExecs is never initialized because the parser only reads it in…
• Pingpong makes the GPU kernel dereference this destination pointer as a flag, but…
• Guard peer access setup to the executor's local MPI rank
• Prevent persistent pingpong dependency cycles from hanging
• Reject pingpong subindices below -1
• Include pong subindex when enabling GFX subindices
• Spin limit checked only every 1024 polls
• Diagnostic contains stale version 1.67 limitation
• Update packaged default GPU target architectures
• Sanity check passes expression where byte size is required
• Allow default numSubExecs for pingpong transfers
• Dryrun command omits required byte-size argument · New
• Cmdline and dryrun arguments are reversed

And 10 more that still need to be addressed.

🧠 Review effort: Lite

[Copilot]

🔵 Needs a closer look

Unresolved functional, packaging, CLI-example, and documentation issues remain.

30 open findings

• Pingpong capacity warning can allow hangs or watchdog failures
• Pingpong-only path dereferences null timing parameter
• Skip pingpong resources during validation setup
• Generic secret allowlist suppresses real credential detections
• Global .lock allowlist hides credentials in lockfiles
• Zero-subexecutor path performs invalid result processing
• Ensure pingpong slot progression avoids parity-safe reuse
• In advanced transfer syntax, numSubExecs is never initialized because the parser only reads it in…
• Pingpong makes the GPU kernel dereference this destination pointer as a flag, but…
• Guard peer access setup to the executor's local MPI rank
• Prevent persistent pingpong dependency cycles from hanging
• Reject pingpong subindices below -1
• Include pong subindex when enabling GFX subindices
• Spin limit checked only every 1024 polls
• Diagnostic contains stale version 1.67 limitation
• Update packaged default GPU target architectures
• Sanity check passes expression where byte size is required
• Allow default numSubExecs for pingpong transfers
• Dryrun command omits required byte-size argument
• Cmdline and dryrun arguments are reversed

And 10 more that still need to be addressed.

🧠 Review effort: Lite

[Copilot]

🟡 Changes recommended

Unresolved critical and moderate findings remain in target gating, pingpong execution, packaging, launcher behavior, and documentation/examples.

32 open findings

• GFX12 guard incorrectly enables CDNA5 XCC query on RDNA4 · New
• Pingpong capacity warning can allow hangs or watchdog failures
• Pingpong-only path dereferences null timing parameter
• Skip pingpong resources during validation setup
• Generic secret allowlist suppresses real credential detections
• Global .lock allowlist hides credentials in lockfiles
• Zero-subexecutor path performs invalid result processing
• Ensure pingpong slot progression avoids parity-safe reuse
• In advanced transfer syntax, numSubExecs is never initialized because the parser only reads it in…
• Pingpong makes the GPU kernel dereference this destination pointer as a flag, but…
• Guard peer access setup to the executor's local MPI rank
• Prevent persistent pingpong dependency cycles from hanging
• Reject pingpong subindices below -1
• Include pong subindex when enabling GFX subindices
• Spin limit checked only every 1024 polls
• Diagnostic contains stale version 1.67 limitation
• Update packaged default GPU target architectures
• Sanity check passes expression where byte size is required
• Allow default numSubExecs for pingpong transfers
• Dry-run command omits required byte-size argument · New

And 12 more that still need to be addressed.

🧠 Review effort: Lite

[Copilot]

🟡 Changes recommended

Fifteen final review comments remain unresolved, including moderate correctness, documentation, packaging, and security issues.

33 open issues

• GFX12 guard incorrectly enables CDNA5 XCC query on RDNA4
• Pingpong capacity warning can allow hangs or watchdog failures
• Pingpong-only path dereferences null timing parameter
• Skip pingpong resources during validation setup
• Generic secret allowlist suppresses real credential detections
• Global .lock allowlist hides credentials in lockfiles
• Zero-subexecutor path performs invalid result processing
• Ensure pingpong slot progression avoids parity-safe reuse
• In advanced transfer syntax, numSubExecs is never initialized because the parser only reads it in…
• Pingpong makes the GPU kernel dereference this destination pointer as a flag, but…
• Guard peer access setup to the executor's local MPI rank
• Prevent persistent pingpong dependency cycles from hanging
• Reject pingpong subindices below -1
• Include pong subindex when enabling GFX subindices
• Synchronize packaged GPU targets with CMake defaults · New
• Spin limit checked only every 1024 polls
• Diagnostic contains stale version 1.67 limitation
• Update packaged default GPU target architectures
• Sanity check passes expression where byte size is required
• Allow default numSubExecs for pingpong transfers

And 13 more that still need to be addressed.

_{Review effort: Lite}