TransferBench v1.70.00 by AtlantaPepsi · Pull Request #355 · ROCm/TransferBench

AtlantaPepsi · 2026-09-01T22:22:47Z

Motivation

Release of TransferBench v1.70.00

Technical Details

See CHANGELOG

* Put in stubs * Add kernel and fix up * Finalize tunability of threads per block and number of blocks. * Implement pipelining * Add debugging logic * Fix segfault * Disable debug prints * Add missing timing information * Add TDM_BLOCK_SIZE, TDM_MAX_LDS_BYTES, and TDM_PIPELINED env vars to control TDM kernel execution * Not functioning but proof of concept async load/store implementation * Fix async load store kernel MVP * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Tim <43156029+AtlantaPepsi@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot

🟡 Changes recommended

There are a few concrete correctness/build issues (missing <limits> include, a broken format string, and unchecked shift bounds) that should be fixed before merging.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Release bump to TransferBench v1.70.00, introducing a new GPU Tensor Data Mover (TDM) execution path plus related runtime/validation/topology improvements and updated docs/build metadata.

Changes:

Add TDM executor (EXE_GPU_TDM, transfer letter T) with new tdmCopy.h backend and a tdmsweep preset for performance exploration.
Improve correctness/operability: optional on-device validation (VALIDATE_ON_DEVICE), persistent thread pools to reduce per-iteration thread creation, and more robust socket communicator behavior.
Refine topology/NUMA handling (logical↔physical CPU NUMA mapping, multi-PCIe-domain NIC proximity) and update release/versioning/docs.

File summaries

File	Description
src/header/TransferBench.hpp	Core library updates: new TDM executor, thread pools, NUMA remapping, on-device validation, NIC proximity changes, socket robustness, version bump.
src/header/tdmCopy.h	New header implementing device-side TDM copy APIs for AMD gfx1250 and NVIDIA sm_90+.
src/client/Utilities.hpp	Adds helper to verify GPU-count homogeneity across ranks; adds TDM label; flushes output.
src/client/Topology.hpp	Aligns CPU NUMA remapping with core library and supports multiple “closest NICs” per GPU.
src/client/Presets/WallClock.hpp	Uses new GPU-count homogeneity check; updates XCC query call.
src/client/Presets/TdmSweep.hpp	New `tdmsweep` preset to sweep TDM knobs and report best bandwidth.
src/client/Presets/SmokeTest.hpp	Adds failure logging to a temp file with more detailed error capture.
src/client/Presets/Rings.hpp	Adds optional TDM executor path and mutual exclusion with DMA option.
src/client/Presets/Presets.hpp	Registers the new `tdmsweep` preset.
src/client/Presets/PodPeerToPeer.hpp	Adds optional TDM executor path and mutual exclusion with DMA option.
src/client/Presets/PodAllToAll.hpp	Adds optional TDM executor path and mutual exclusion with DMA option.
src/client/Presets/PeerToPeer.hpp	Adds optional TDM executor path and mutual exclusion with DMA option.
src/client/Presets/Help.hpp	Updates help text for the new TDM executor.
src/client/Presets/GfxSweep.hpp	Minor env var print ordering tweak.
src/client/Presets/EmptyKernel.hpp	Uses new GPU-count homogeneity check.
src/client/Presets/AllToAll.hpp	Adds optional TDM executor path and mutual exclusion with DMA option.
src/client/EnvVars.hpp	Adds sweep bounds, TDM env vars, on-device validation env var; updates config mapping/help output.
src/client/Client.cpp	Uses sweep bounds to control auto-generated byte sweep.
Makefile	Improves HIPCC selection for ROCm wheel layouts.
docs/reference/environment-variables.rst	Documents `VALIDATE_ON_DEVICE`.
docs/conceptual/transferbench-data-validation.rst	Documents on-device validation option semantics.
CMakeLists.txt	Version bump to 1.70.00.
CHANGELOG.md	Adds v1.70.00 release notes.

Review details

Suppressed comments (1)

src/client/EnvVars.hpp:414

USE_HIP_EVENTS is listed twice in the environment variable help output; the first line is outdated and should be removed to avoid confusion.

    printf(" USE_HIP_EVENTS      - Use HIP events for GFX executor timing\n");
    printf(" USE_HIP_EVENTS      - Use HIP events for GFX/DMA/TDM executor timing (0=CPU wall-clock)\n");

Files reviewed: 22/23 changed files
Comments generated: 6
Review effort level: Lite

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

Copilot

🔵 Needs a closer look

It introduces broad, high-impact changes to the core execution/validation/runtime topology logic that warrant careful human validation beyond a narrow automated review.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

src/client/EnvVars.hpp:417

The environment variable help text lists USE_HIP_EVENTS twice (with conflicting descriptions) and the TDM_BLOCK_ORDER description omits the supported value 2=random (see validation in the library and presets). This can mislead users.

This issue also appears on line 563 of the same file.

src/client/EnvVars.hpp:564

TDM_BLOCK_ORDER reporting treats any non-zero value as "Interleaved", but the codebase also supports 2=random. This makes the displayed configuration incorrect when users select random ordering.

    Print("TDM_BLOCK_ORDER", tdmBlockOrder,
          "TDM Thread block ordering: %s", tdmBlockOrder == 0 ? "Sequential" : "Interleaved");

Files reviewed: 22/23 changed files
Comments generated: 2
Review effort level: Lite

Copilot

🟡 Changes recommended

There are verified build/help-output issues in the updated code (e.g., missing header for getpid() and duplicated/incorrect env-var help text) that should be fixed before release.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

src/client/Presets/SmokeTest.hpp:25

getpid() is used later in this header but the required declaration header isn’t included here, which can cause a compile error depending on transitive includes. Include <unistd.h> explicitly before calling getpid().

#include <set>
#include <cstdio>
#include <ctime>

src/client/EnvVars.hpp:564

TDM_BLOCK_ORDER now supports more than two values (0/1/2), but the displayed description only distinguishes 0 vs non-0. This makes TDM_BLOCK_ORDER=2 show up as “Interleaved” even though it means “Random”.

    Print("TDM_BLOCK_ORDER", tdmBlockOrder,
          "TDM Thread block ordering: %s", tdmBlockOrder == 0 ? "Sequential" : "Interleaved");

Files reviewed: 22/23 changed files
Comments generated: 1
Review effort level: Lite

… re-enable

…ns on some presets

Emergency push to support the new amd-smi 27 fabric layout. Preserve source compatibility with both nested pre-27 amd-smi and flattened 27+ amd-smi fabric payload declarations. Co-authored-by: Cursor <cursoragent@cursor.com>

…ins (candidate-1.70) (#353) * Map all equally-close NICs per GPU instead of picking a single winner Replace single-winner NIC selection with all equally-close NICs. Previously, when multiple NICs tied as closest to a GPU, only the least-used NIC (tracked by assignedCount) was recorded, so systems where each GPU has 2 NICs at equal PCIe distance reported only 1 NIC per GPU instead of 2 -- closestNicsToGpu is a topology map, not a traffic assignment, so recording all equally-close NICs is the correct semantic. Apply the same fix to the reverse mapping: the bus-ID-distance fallback now collects all GPUs at minimum distance rather than stopping at the first one found. * Show all closest NICs per GPU in topology display Use GetClosestNicsToGpu (vector) instead of GetClosestNicToGpu (int) in both the NIC table and GPU table so all equally-close NICs are shown as a comma/space-separated list (e.g. "0,1") instead of a single index, matching the topology map populated by the previous commit. Also fix the NIC table loop: the old code called GetClosestNicsToGpu inside a per-NIC outer loop, making numNics*numGpus calls. Build an inverse GPU-per-NIC map up front (numGpus calls) and look up the result directly in the print loop instead. * Use PCIe domain distance instead of bus-ID distance for proximity tiebreak Rename ExtractBusNumber/GetBusIdDistance to ExtractDomain/GetDomainDistance. Bus numbers are firmware-assigned and do not reliably track physical closeness within a PCIe domain, so they are no longer used to discriminate among candidates that already share the same LCA depth or that fall back to the no-tree-match path -- doing so could break ties between NICs that are genuinely equidistant from a GPU. Distance is now computed from the PCIe domain field, which only matters as a deterministic tiebreak across domains. Also fix GetNearestDevicesInTree to treat an unparseable address (-1) as the largest possible distance rather than using -1 directly in the comparison, so it never incorrectly outranks a candidate with a real distance while still remaining eligible as a last resort. The GPU-to-NIC fallback loop now reads distances from ibvAddressList instead of the raw device list, so it stays consistent with the PCIe-tree path above and never maps a GPU to a NIC without an active port. --------- Co-authored-by: paklui <paklui@users.noreply.github.com>

…ays validating

… of on host

U is uncached GPU memory; H is unpinned host memory.

Copilot

🟡 Changes recommended

A few verified issues in the updated code/docs (incorrect subexecutor warning scope, and inconsistent/duplicated env-var help text) should be corrected before approving the release.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

src/client/EnvVars.hpp:417

The environment-variable help prints USE_HIP_EVENTS twice (one outdated line), and TDM_BLOCK_ORDER help omits the supported "2=random" option.

    printf(" TDM_BLOCK_ORDER     - How blocks for TDM transfers are ordered. 0=sequential, 1=interleaved\n");
    printf(" TDM_BLOCK_SIZE      - # of threads per threadblock for TDM (async tensor) kernels (Must be multiple of 32)\n");
    printf(" TDM_LDS_BYTES       - Amount of LDS bytes to allocate per workgroup for TDM kernels (0 = device max; K/M/G suffixes accepted)\n");
    printf(" USE_HIP_EVENTS      - Use HIP events for GFX executor timing\n");
    printf(" USE_HIP_EVENTS      - Use HIP events for GFX/DMA/TDM executor timing (0=CPU wall-clock)\n");

src/client/EnvVars.hpp:564

TDM_BLOCK_ORDER supports 0/1/2 (sequential/interleaved/random), but the DisplayEnvVars output currently treats any non-zero value as "Interleaved", which is incorrect for the random setting.

    Print("TDM_BLOCK_ORDER", tdmBlockOrder,
          "TDM Thread block ordering: %s", tdmBlockOrder == 0 ? "Sequential" : "Interleaved");

Files reviewed: 23/24 changed files
Comments generated: 2
Review effort level: Lite

Copilot

🟡 Changes recommended

There are a few concrete, fixable issues in the updated code/docs (notably missing <limits> include usage robustness and a duplicated env-var help line) that should be corrected before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

src/client/EnvVars.hpp:417

The environment-variable usage text prints USE_HIP_EVENTS twice with conflicting descriptions, which will confuse users and drift from the actual behavior (it now applies to GFX/DMA/TDM).

    printf(" USE_HIP_EVENTS      - Use HIP events for GFX executor timing\n");
    printf(" USE_HIP_EVENTS      - Use HIP events for GFX/DMA/TDM executor timing (0=CPU wall-clock)\n");

Files reviewed: 23/24 changed files
Comments generated: 1
Review effort level: Lite

gilbertlee-amd and others added 7 commits July 6, 2026 15:36

Fix for using byte offsets with DMA executors (#330)

f8064da

Dynamic Loading IB verbs (#253)

9eede61

TDM productionization in TB

d7be156

temporary fixes

33d0bf7

gate runtime check for absent TDM header

d2aff82

Add ability to set min/max power of 2 to sweep

082ac2f

AtlantaPepsi requested a review from a team as a code owner September 1, 2026 22:22

Copilot AI lite review requested due to automatic review settings September 1, 2026 22:22

AtlantaPepsi requested a review from a team as a code owner September 1, 2026 22:22

Copilot started reviewing on behalf of AtlantaPepsi September 1, 2026 22:23 View session

Copilot AI reviewed Sep 1, 2026

View reviewed changes

Comment thread src/header/TransferBench.hpp

Comment thread src/client/Client.cpp

Comment thread src/client/EnvVars.hpp Outdated

Comment thread src/client/Presets/Help.hpp Outdated

Comment thread src/header/TransferBench.hpp

Comment thread src/header/TransferBench.hpp Outdated

Copilot AI review requested due to automatic review settings September 2, 2026 01:04

Copilot started reviewing on behalf of AtlantaPepsi September 2, 2026 01:05 View session

Copilot AI reviewed Sep 2, 2026

View reviewed changes

Comment thread src/client/Client.cpp

Comment thread src/client/Presets/SmokeTest.hpp

Copilot AI review requested due to automatic review settings September 2, 2026 01:19

AtlantaPepsi force-pushed the candidate-1.70-develop branch from c4c5851 to 4675f07 Compare September 2, 2026 01:19

Copilot started reviewing on behalf of AtlantaPepsi September 2, 2026 01:19 View session

Copilot AI reviewed Sep 2, 2026

View reviewed changes

Comment thread src/client/EnvVars.hpp

AtlantaPepsi and others added 11 commits September 1, 2026 21:01

TDM cleanup and enhancement + presets

34914e9

TMA support for nvidia platform

d226047

Temporary workaround in Broadcast

adb7f6f

Hiding CPU NUMA nodes with no cores by default. TB_SHOW_ALL_NUMA=1 to…

bf5a50d

… re-enable

Switching to persistent threadpools per Executor to minimize overheads

efd6bfa

Updating default unroll on GFX1250 to 32, remove temp barrier

8099bdc

Add TB_SEND_USLEEP for diagnosing socket stability, loosen restrictio…

c9d5b5e

…ns on some presets

TransferBench v1.69.01 (#347)

1ce40c6

Emergency push to support the new amd-smi 27 fabric layout. Preserve source compatibility with both nested pre-27 amd-smi and flattened 27+ amd-smi fabric payload declarations. Co-authored-by: Cursor <cursoragent@cursor.com>

Adding ppodId printing to verbose mode

fa5ca1c

Adding guard before ibv_free_device_list

d5d1972

gilbertlee-amd and others added 6 commits September 1, 2026 21:01

Adjusting how hwids are collected for gfx1250

0dcec94

Adding additional logging on fail for smoketest preset

2137392

Removing warn from fail reporting for smoketest, clearing data if alw…

484dd2e

…ays validating

Adding more comprehensive mismatch logging

e0a5c3e

Adding VALIDATE_ON_DEVICE to allow for validation done on GPU instead…

aecc993

… of on host

Removal of 2D copy

b225f43

AtlantaPepsi force-pushed the candidate-1.70-develop branch 2 times, most recently from 76d0aa7 to 84aedc9 Compare September 2, 2026 02:20

Fix memory type symbols in configuration-file docs (#351)

6f0c164

U is uncached GPU memory; H is unpinned host memory.

Copilot AI review requested due to automatic review settings September 2, 2026 04:34

AtlantaPepsi force-pushed the candidate-1.70-develop branch from 84aedc9 to 6e5e2b4 Compare September 2, 2026 04:34

Copilot started reviewing on behalf of AtlantaPepsi September 2, 2026 04:35 View session

Copilot AI reviewed Sep 2, 2026

View reviewed changes

Comment thread src/header/TransferBench.hpp Outdated

Comment thread src/header/TransferBench.hpp Outdated

Merge branch 'develop' into candidate-1.70-develop

f2a2b1f

Copilot AI review requested due to automatic review settings September 2, 2026 04:42

AtlantaPepsi force-pushed the candidate-1.70-develop branch from 6e5e2b4 to f2a2b1f Compare September 2, 2026 04:42

Copilot started reviewing on behalf of AtlantaPepsi September 2, 2026 04:43 View session

Copilot AI reviewed Sep 2, 2026

View reviewed changes

Comment thread src/header/TransferBench.hpp

gilbertlee-amd approved these changes Sep 2, 2026

View reviewed changes

AtlantaPepsi merged commit 17da7c1 into develop Sep 2, 2026
9 of 10 checks passed

AtlantaPepsi deleted the candidate-1.70-develop branch September 3, 2026 03:42

Navigation Menu

Uh oh!

TransferBench v1.70.00 - #355

TransferBench v1.70.00#355
AtlantaPepsi merged 26 commits into
developfrom
candidate-1.70-develop

AtlantaPepsi commented Sep 1, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Uh oh!

Conversation

AtlantaPepsi commented Sep 1, 2026

Motivation

Technical Details

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (1)

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

🔵 Needs a closer look

Suppressed comments (2)

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (2)

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (2)

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (1)

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants