Add Logic for Using TDM and Async Loads and Stores - #322
Conversation
…control TDM kernel execution
Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # src/header/TransferBench.hpp
| ROCM_PATH ?= /opt/rocm | ||
| CUDA_PATH ?= /usr/local/cuda | ||
| MPI_PATH ?= /usr/local/openmpi | ||
| # pip ROCm wheels ship bin/amdclang++ but often omit bin/amdllvm (which that stub execs). |
There was a problem hiding this comment.
Logic for running building and running this in a venv on WSL
|
Discussed with @AtlantaPepsi and @gilbertlee-amd. The plan is to merge as is. There's some cleanup that needs to be performed however before the candidate branch can be released:
Regarding async loads/stores, the current code is just proof of concept. The waitcnt placement and unrolling isn't optimized. Feel free to remove if you think it's best. TBH, I think this code could be rolled into the existing TransferBench kernels. I was thinking of working on that (actually what I started on before moving to TDM). TDM feels sufficiently different where this likely isn't warranted. I concur with Gilbert on that one. |
| @@ -0,0 +1,355 @@ | |||
| /* | |||
There was a problem hiding this comment.
This file is a standard header but was missing from my installation of ROCm, not sure why. It can probably be removed. This is standard in CLR:
There was a problem hiding this comment.
Pull request overview
This PR adds two new GPU executor variants intended to improve copy performance on gfx1250 by using (1) TDM-based tensor load/store operations and (2) async global load/store-to-LDS memory ops, along with CLI/help/env-var wiring and a small Makefile improvement for ROCm wheel layouts.
Changes:
- Add new executor types
EXE_GPU_ASYNC_TENSORandEXE_GPU_ASYNC_MEMOPSwith corresponding dispatch, validation, topology naming, and client stringification. - Introduce TDM configuration (
TdmOptions,TDM_*env vars) and implement new HIP kernels for the TDM tensor path and async load/store path. - Improve HIP compiler discovery in the Makefile for ROCm installations where
bin/amdllvmis absent.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
src/header/TransferBench.hpp |
Adds new executor types, TDM options, and implements/dispatches async GPU kernels (TDM + async LD/ST). |
src/header/tdm.h |
Adds gfx1250 TDM register layout/bitfield definitions used by the TDM kernel path. |
src/client/Utilities.hpp |
Extends executor-to-string mapping for the new executor types. |
src/client/Presets/Help.hpp |
Updates help text to document new executor letters and examples. |
src/client/EnvVars.hpp |
Adds env vars for TDM kernel tuning and wires them into ConfigOptions. |
Makefile |
Adjusts HIP compiler selection to support ROCm wheel layouts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Copy from global memory to LDS | ||
| for (size_t i = 0; i < elementsToProcess; i += elements_per_dwordx4_load_store) { | ||
| asyncLoadX4(srcPtr + i, shmemPtr + i); | ||
| __builtin_amdgcn_s_wait_asynccnt(0); | ||
| } | ||
|
|
||
| // Write back from LDS to global | ||
| for (size_t i = 0; i < elementsToProcess; i += elements_per_dwordx4_load_store){ | ||
| asyncStoreX4(shmemPtr + i, dstPtr + i); | ||
| __builtin_amdgcn_s_wait_asynccnt(0); | ||
| } |
There was a problem hiding this comment.
async l/s will undergo further changes
| if (rss.numBytes % sizeof(float) != 0) | ||
| return {ERR_FATAL, "Async tensor executor (TDM): numBytes (%zu) must be a multiple of %zu", rss.numBytes, sizeof(float)}; |
There was a problem hiding this comment.
this will be elevated into TransfersHaveErrors guard
| int maxShmem = 0; | ||
| ERR_CHECK(hipDeviceGetAttribute(&maxShmem, hipDeviceAttributeMaxSharedMemoryPerBlock, exeIndex)); | ||
| maxShmem = std::min<int>(maxShmem, cfg.tdm.maxLDSBytes); | ||
| int kWavesPerWorkgroup = threads / warpSize; | ||
| int pipelineDepth = usePipelinedTensorOps ? 2 : 1; | ||
| int numFloatsPerTile = std::max<int>(1, maxShmem / (sizeof(float) * kWavesPerWorkgroup * pipelineDepth)); | ||
| unsigned int shmemBytes = numFloatsPerTile * kWavesPerWorkgroup * sizeof(float) * pipelineDepth; |
There was a problem hiding this comment.
to be further validated in next PR
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* 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>
* 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>
Motivation
Run fast on gfx1250
Technical Details
Adds simple copy kernels using TDM and async/loads and stores
Test Plan
Code has correctness testing, but I need to add more corner case testing
Test Result
Tests pass on a hardware simulator for both variants:
./TransferBench cmdline 1024K "1 4 G0 T0 G0"
./TransferBench cmdline 1024K "1 4 G0 L0 G0"
Submission Checklist