Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions mlir/tools/rocmlir-tuning-driver/CacheFlush.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,17 @@ class CacheFlushState {
}
}

LogicalResult prepare(bool useLastLevelCacheSize) {
std::lock_guard<std::mutex> lock(stateMutex);
if (failed(allocCacheFlushBuffer(useLastLevelCacheSize)))
return failure();
#if defined(__HIP_PLATFORM_AMD__)
if (failed(buildFlushInstructionCacheKernel()))
return failure();
#endif
return success();
}

LogicalResult flushCache(hipStream_t stream, bool useLastLevelCacheSize) {
std::lock_guard<std::mutex> lock(stateMutex);
if (failed(allocCacheFlushBuffer(useLastLevelCacheSize)))
Expand Down Expand Up @@ -311,6 +322,10 @@ CacheFlushState &getState() {

} // namespace

LogicalResult prepareCacheFlushArtifacts(bool useLastLevelCacheSize) {
return getState().prepare(useLastLevelCacheSize);
}

LogicalResult flushCache(hipStream_t stream, bool useLastLevelCacheSize) {
return getState().flushCache(stream, useLastLevelCacheSize);
}
Expand Down
11 changes: 11 additions & 0 deletions mlir/tools/rocmlir-tuning-driver/CacheFlush.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,17 @@

namespace rocmlir::tuningdriver {

/// \brief Builds the artifacts that the flush helpers below would otherwise
/// create lazily on their first call: the hiprtc-compiled instruction-cache
/// invalidation kernel and the cache-sized flush buffer.
///
/// Both are one-time, host-side and expensive (a runtime compile plus a large
/// ``hipMalloc``). Callers that time the flush helpers must build them up
/// front, or that setup cost is attributed to the first timed iteration.
/// \param useLastLevelCacheSize Sizing for the flush buffer; see flushCache.
/// \return success() if the artifacts are ready, failure() otherwise.
mlir::LogicalResult prepareCacheFlushArtifacts(bool useLastLevelCacheSize);

/// \brief Flushes the cache by performing a memory write operation.
/// \param stream The HIP stream to use for the flush operation.
/// \param useLastLevelCacheSize When true, size the flush buffer to the
Expand Down
10 changes: 10 additions & 0 deletions mlir/tools/rocmlir-tuning-driver/rocmlir-tuning-driver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -750,6 +750,16 @@ benchmarkKernels(ArrayRef<std::string> binaries,
return failure();
}
} else {
// Build these before the estimate below starts timing: they are created
// lazily at a one-time host-side cost of order 100 ms (hiprtc compile plus
// a cache-sized hipMalloc), and since the estimate records its start event
// on an empty stream, that stall would land in its elapsed time. An
// inflated estimate collapses every iteration count derived from it to its
// floor, leaving the benchmark ranking configs on a few samples at idle
// clocks.
if (failed(prepareCacheFlushArtifacts(params.flushLastLevelCache)))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This eager prepare only covers the --triton-benchmark-mode branch, but the legacy default path has the same defect. In measureSmallKernel the CPU timer starts at line 470 and the first flushInstructionCache/flushCache happens at lines 475-478, i.e. inside the timed region — so on the very first config of a tuning run the hiprtc compile plus the (up to 256 MiB with --flush-last-level-cache) hipMalloc is charged to smallKernelCpuMs. Since legacy mode is the default and tuning mode is exactly the case where benchmarkMode is false and the flushes do run, that first perf config gets a badly inflated time and can be ranked out even when it is the best one. Suggest hoisting the call above the if (!params.tritonBenchmarkMode) at line 670 so both paths get the artifacts built up front; if you want to avoid the large allocation in the legacy path that never flushes, gate it on params.tritonBenchmarkMode || params.benchmarkConfig.empty() instead of duplicating the call. (Checklist: Major — "DRY/YAGNI/KISS violations: redundant code" / incomplete fix across equivalent code paths.)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dhernandez0 I agree with this one, please take a look

return failure();

// Estimate the per-launch runtime so we can size warmup/benchmark iteration
// counts from the requested time budgets (Triton do_bench style). We time a
// handful of launches (flushing caches between them) using a single event
Expand Down
39 changes: 34 additions & 5 deletions mlir/utils/performance/perfRunner.py
Original file line number Diff line number Diff line change
Expand Up @@ -328,16 +328,43 @@ def create_paths(config_file_path, mlir_build_dir_path) -> Paths:


# utility functions.
def is_rocclr_internal_kernel(row) -> bool:
"""Whether a rocprof CSV row describes a HIP-runtime-internal kernel.

The HIP runtime dispatches its own kernels onto the same queue as ours, and
``rocprofv3 --kernel-trace`` records every dispatch it sees. ROCclr services
a small unpinned ``hipMemcpy`` with the ``__amd_rocclr_copyBuffer`` blit
shader rather than the SDMA engine, so the host harness's parameter copies
show up next to the kernel under test; ``__amd_rocclr_initHeap``,
``_scheduler`` and friends can appear the same way. Their cost is a fixed
few microseconds, which is noise for a large kernel but can dominate a small
one, so exclude the whole family from timing and metric aggregation.
"""
# rocprofv3 names the kernel column differently per report: the ``--stats``
# summary uses ``Name``, counter collection uses ``Kernel_Name``.
for column in ('Name', 'Kernel_Name'):
name = row.get(column)
if name is not None:
return name.strip('"').startswith('__amd_rocclr_')
return False


def get_nanoseconds(filename):
if not os.path.exists(filename):
return np.nan
with open(filename, 'r') as csv_file:
reader = csv.DictReader(csv_file, delimiter=',')
result = 0
for row in reader:
result += int(float(row['AverageNs']))
# Summing over rows is intentional: a single benchmarked op can dispatch
# several kernels (for example a strided backward-data convolution,
# which emits one kernel per filter slice), and the op's runtime is
# their total.
durations = [
int(float(row['AverageNs'])) for row in reader if not is_rocclr_internal_kernel(row)
]
csv_file.close()
return result
if not durations:
return np.nan
return sum(durations)


def get_profiler_output_path(arch: str, base_out_path):
Expand Down Expand Up @@ -371,9 +398,11 @@ def get_bank_conflict(filename):

result = []
for row in reader:
if row['Counter_Name'] == 'LDSBankConflict':
if row['Counter_Name'] == 'LDSBankConflict' and not is_rocclr_internal_kernel(row):
result.append(float(row['Counter_Value']))
csv_file.close()
if not result:
return np.nan
result_average = sum(result) / len(result)
return result_average

Expand Down
69 changes: 58 additions & 11 deletions mlir/utils/performance/tests/test_perfRunner.py
Original file line number Diff line number Diff line change
Expand Up @@ -283,24 +283,71 @@ def test_mixed_nchw_nhwc_config_is_skipped(self):
"N01GC")) is None


def _write_csv(tmp_path, contents):
path = tmp_path / "results.csv"
path.write_text(contents)
return str(path)


class TestGetNanoseconds:
"""Tests for get_nanoseconds (reads CSV from rocprof)."""

def test_missing_file_returns_nan(self):
ns = perfRunner.get_nanoseconds("/nonexistent/path.csv")
assert math.isnan(ns)

def test_valid_csv(self):
with tempfile.NamedTemporaryFile(mode="w", suffix=".csv", delete=False, newline="") as f:
f.write("KernelName,AverageNs,SomeOther\n")
f.write("kern1,1000,0\n")
f.write("kern2,2000,0\n")
path = f.name
try:
ns = perfRunner.get_nanoseconds(path)
assert ns == 3000
finally:
os.unlink(path)
def test_valid_csv(self, tmp_path):
# Several kernels for one op (e.g. a strided backward-data
# convolution, which emits one kernel per filter slice) sum.
path = _write_csv(tmp_path, "Name,AverageNs,SomeOther\n"
"kern1,1000,0\n"
"kern2,2000,0\n")
assert perfRunner.get_nanoseconds(path) == 3000

def test_rocclr_internal_kernels_excluded(self, tmp_path):
# The HIP runtime's blit shader for the harness's hipMemcpy calls lands
# in the same trace and must not be charged to the kernel under test.
path = _write_csv(
tmp_path, "Name,AverageNs,SomeOther\n"
"kern1,1000,0\n"
"__amd_rocclr_copyBuffer,2800,0\n"
"__amd_rocclr_initHeap,500,0\n")
assert perfRunner.get_nanoseconds(path) == 1000

def test_only_internal_kernels_returns_nan(self, tmp_path):
# No kernel of ours ran, so there is no time to report -- NaN rather
# than 0, which would otherwise read as infinite TFlops.
path = _write_csv(tmp_path, "Name,AverageNs,SomeOther\n"
"__amd_rocclr_copyBuffer,2800,0\n")
assert math.isnan(perfRunner.get_nanoseconds(path))

def test_missing_name_column_sums_all_rows(self, tmp_path):
path = _write_csv(tmp_path, "AverageNs\n"
"1000\n"
"2000\n")
assert perfRunner.get_nanoseconds(path) == 3000


class TestGetBankConflict:
"""Tests for get_bank_conflict (reads rocprof's counter-collection CSV)."""

HEADER = "Kernel_Name,Counter_Name,Counter_Value\n"

def test_missing_file_returns_nan_string(self):
assert perfRunner.get_bank_conflict("/nonexistent/path.csv") == "NaN"

def test_averages_over_our_dispatches_only(self, tmp_path):
# The blit shader reports 0% and would otherwise dilute the average.
path = _write_csv(
tmp_path, self.HEADER + "kern1,LDSBankConflict,40.0\n"
"kern1,LDSBankConflict,60.0\n"
"__amd_rocclr_copyBuffer,LDSBankConflict,0.0\n"
"__amd_rocclr_copyBuffer,LDSBankConflict,0.0\n")
assert perfRunner.get_bank_conflict(path) == 50.0

def test_no_matching_rows_returns_nan(self, tmp_path):
path = _write_csv(tmp_path, self.HEADER + "kern1,SomeOtherCounter,7.0\n")
assert math.isnan(perfRunner.get_bank_conflict(path))


class TestGetProfilerOutputPath:
Expand Down
Loading