We're working through the HIP benchmarks against a TheRock-based ROCm install and have noticed a small inconsistency I'd like to discuss before drafting PRs.
The vast majority of benchmarks expose a top-level Makefile so that make, make clean, and make run work directly from src/<benchmark>/. A small subset doesn't — their actual Makefile (or CMakeLists.txt) lives in a subdirectory:
| Benchmark |
Build entry point |
daphne-hip |
src/points2image/Makefile |
hpl-hip |
src/hpl-2.3/Makefile |
local-ht-hip |
src/Makefile |
logic-resim-hip |
Simulation/Makefile |
mf-sgd-hip |
src/Makefile |
miniFE-hip |
src/Makefile |
si-hip |
CMakeLists.txt (only) |
snicit-hip |
bin/Makefile |
tgvnn-hip |
src/Makefile |
That makes them awkward to drive from a generic build script — every consumer has to special-case the subdirectory per benchmark, or maintain a local wrapper-Makefile workaround. We're currently doing the latter, but it'd be nice to align with the rest of the repo.
Proposal
Add a small wrapper Makefile at the benchmark root for each of these 9. Each is ~10 lines, no functional changes — just delegates all, clean, and run to the existing inner build. Example for local-ht-hip:
HIP_ARCH ?= gfx906
.PHONY: all clean run
all:
$(MAKE) -C src HIP_ARCH=$(HIP_ARCH)
clean:
$(MAKE) -C src clean
run:
$(MAKE) -C src run
si-hip would invoke cmake instead of $(MAKE) -C (it has no inner Makefile), but follows the same shape: thin top-level wrapper, build artifacts stay where the inner build puts them, no behavior change for anyone who was already invoking the inner build directly.
Ask
Would you be open to a PR (or per-benchmark PRs — your call on granularity) adding these wrappers? Happy to handle review feedback on the wrapper template; we'd just like to confirm the direction is welcome before drafting the patches.
We're working through the HIP benchmarks against a TheRock-based ROCm install and have noticed a small inconsistency I'd like to discuss before drafting PRs.
The vast majority of benchmarks expose a top-level
Makefileso thatmake,make clean, andmake runwork directly fromsrc/<benchmark>/. A small subset doesn't — their actual Makefile (orCMakeLists.txt) lives in a subdirectory:daphne-hipsrc/points2image/Makefilehpl-hipsrc/hpl-2.3/Makefilelocal-ht-hipsrc/Makefilelogic-resim-hipSimulation/Makefilemf-sgd-hipsrc/MakefileminiFE-hipsrc/Makefilesi-hipCMakeLists.txt(only)snicit-hipbin/Makefiletgvnn-hipsrc/MakefileThat makes them awkward to drive from a generic build script — every consumer has to special-case the subdirectory per benchmark, or maintain a local wrapper-Makefile workaround. We're currently doing the latter, but it'd be nice to align with the rest of the repo.
Proposal
Add a small wrapper
Makefileat the benchmark root for each of these 9. Each is ~10 lines, no functional changes — just delegatesall,clean, andrunto the existing inner build. Example forlocal-ht-hip:si-hipwould invokecmakeinstead of$(MAKE) -C(it has no inner Makefile), but follows the same shape: thin top-level wrapper, build artifacts stay where the inner build puts them, no behavior change for anyone who was already invoking the inner build directly.Ask
Would you be open to a PR (or per-benchmark PRs — your call on granularity) adding these wrappers? Happy to handle review feedback on the wrapper template; we'd just like to confirm the direction is welcome before drafting the patches.