Skip to content
Open
Changes from 7 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
59 changes: 47 additions & 12 deletions mlir/utils/performance/perfRunner.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,30 @@ def inverse_filter_layouts(filter_layout):
return "".join(map[char] for char in filter_layout)


# Map rocMLIR-specific layout names to MIOpenDriver layout names (NCHW, NHWC).
# MIOpenDriver does not accept rocMLIR layout names (e.g. GNC01, NGC01).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Add comment that this is "best effort mapping" and may not result in fair comparison.

Add comment that it is doing "channel first" mapping to NCHW and "channel last" to "NHWC"

ROCMLIR_TO_MIOPEN_LAYOUT = {

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.

we could simplify this by first converting 0 -> H and 1 -> W.

@dhernandez0 dhernandez0 Mar 26, 2026

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.

also, I see we just drop "G", we can do that if it's not a group conv (G=1), if it is, we can't. Are we checking that somewhere?

@dhernandez0 dhernandez0 Mar 26, 2026

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.

also input and output have "K" as well, we aren't converting that here? it's probably easier to keep separate dicts for input, filter and output.

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.

Should we emit an error (or at least a warning) if we are dropping G?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Did you run nightly reports to see if it runs into any errors or not ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

'GNC01': 'NCHW',
'NGC01': 'NCHW',
'NC0G1': 'NCHW',
'G0NC1': 'NCHW',
'01NGC': 'NHWC',
'N01GC': 'NHWC',
'NCHW': 'NCHW',
'NHWC': 'NHWC',
}


def conv_commandline_to_miopen_layouts(commandline):
"""Return a copy of commandline with -f, -I, -O layout values translated to MIOpen names."""
result = list(commandline)
for i in range(len(result)):
if result[i] in ('-f', '-I', '-O') and i + 1 < len(result):
layout = result[i + 1]
result[i + 1] = ROCMLIR_TO_MIOPEN_LAYOUT.get(layout, layout)
return result
Comment on lines +98 to +148

Copilot AI Apr 24, 2026

Copy link

Choose a reason for hiding this comment

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

New layout translation logic (_rocmlir_layout_to_miopen / conv_commandline_to_miopen_layouts) isn’t covered by the existing mlir/utils/performance/tests/test_perfRunner.py suite (which already tests layout helpers). Adding unit tests for representative layouts (e.g. GNC01, NGC01, grouped conv cases, and pass-through on unknown) would help prevent regressions and validate the heuristic.

Copilot uses AI. Check for mistakes.


@dataclass
class MLIRPaths:
rocmlir_gen_path: str
Expand Down Expand Up @@ -694,20 +718,31 @@ def benchmark_external(cls, commandline, paths: Paths, arch, num_cu, num_chiplet
if os.path.exists(get_profiler_output_path(arch, BENCHMARKING_METRICS_FILE_NAME)):
os.remove(get_profiler_output_path(arch, BENCHMARKING_METRICS_FILE_NAME))
config = cls.from_command_line(commandline, arch, num_cu, num_chiplets)
miopen_driver_cmd = [MIOPENDRIVER, *commandline, '-V', '0', '-t', '1']
print("Running MIOpen Benchmark: ", ' '.join(commandline))
# Configs use rocMLIR layout names; MIOpenDriver expects NCHW/NHWC.
miopen_commandline = conv_commandline_to_miopen_layouts(commandline)
miopen_driver_cmd = [MIOPENDRIVER, *miopen_commandline, '-V', '0', '-t', '1']
print("Running MIOpen Benchmark: ", ' '.join(miopen_driver_cmd))
# invoke MIOpenDriver.
outs, noerr = run_pipeline([miopen_driver_cmd])
nanoseconds = np.nan
if noerr:
# convert bytes to str
outs = outs.decode('utf-8')
# Extract Elapsed time in ms from the output of MIOpenDriver
# Use regular expression to match the contents between
# "Elasped: " (note the space at the end) and "ms"
elapsed_time_in_ms = ELAPSED_TIME_RE.search(outs).group(1)
nanoseconds = float(elapsed_time_in_ms) * 1.0e6

if not noerr:
err_msg = outs.decode('utf-8') if isinstance(outs, bytes) else str(outs)
raise RuntimeError("MIOpen benchmark failed. CI must fail on MIOpen errors.\n"
"Failing command: " + ' '.join(miopen_driver_cmd) + "\n"
"Error: " + err_msg)

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.

The yapf formatter requires this raise RuntimeError(...) to have the first argument on the same line as the opening parenthesis, with continuation lines aligned beneath it. The original multi-line style (opening paren on its own line) fails the yapf --diff check in CI.

Fixed format:

raise RuntimeError("MIOpen benchmark failed. CI must fail on MIOpen errors.
"
                   "Failing command: " + ' '.join(miopen_driver_cmd) + "
"
                   "Error: " + err_msg)

Comment on lines +756 to +760

Copilot AI Apr 24, 2026

Copy link

Choose a reason for hiding this comment

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

In the error path (noerr == False), the raised RuntimeError uses outs as the error message, but run_pipeline() returns only stdout; stderr is consumed inside run_pipeline() and only printed. This often makes the exception’s "Error:" section empty/unhelpful. Consider updating run_pipeline() to return stderr (or merged stdout+stderr) so callers can include it, or at least include p.stderr output in outs when returning False.

Copilot uses AI. Check for mistakes.
# convert bytes to str
outs = outs.decode('utf-8')
# Extract Elapsed time in ms from the output of MIOpenDriver
# Use regular expression to match the contents between
# "Elapsed: " (note the space at the end) and "ms"
match = ELAPSED_TIME_RE.search(outs)
if not match:
raise RuntimeError(

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.

yapf formatting violation — the CI py-checks job failed here.

The raise RuntimeError(...) block has the opening parenthesis on its own line with indented string arguments, but yapf requires the first argument to start on the same line as the function call. Please reformat to:

raise RuntimeError("Failed to parse elapsed time from MIOpenDriver output.
"
                   "Failing command: " + ' '.join(miopen_driver_cmd) + "
"
                   "Output:
" + outs)

Or run yapf -i mlir/utils/performance/perfRunner.py to auto-fix it.

"Failed to parse elapsed time from MIOpenDriver output.\n"
"Failing command: " + ' '.join(miopen_driver_cmd) + "\n"
"Output:\n" + outs
)
elapsed_time_in_ms = match.group(1)
nanoseconds = float(elapsed_time_in_ms) * 1.0e6
return config.table_entry(nanoseconds)


Expand Down
Loading