GEMM+GEMM and CONV+GEMM support in script for handling new configs #2053

dorde-antic · 2025-10-23T10:34:35Z

Motivation

Last version of the script didn't support GEMM+GEMM and CONV+GEMM configs and didn't support incomplete configs.

Resolves https://github.com/ROCm/rocMLIR-internal/issues/2084

Technical Details

Doesn't use perfRunner's fromCommandLine since we sometimes obtain incomplete configurations.
Introduces support for GEMM+GEMM and CONV+GEMM configs.

Test Plan

Locally

Test Result

✅

Submission Checklist

Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.

dhernandez0 · 2025-10-23T10:43:36Z

mlir/utils/performance/handleNewConfigs.py

                        help="Path to the file containing existing convolution configurations")
    parser.add_argument("--gemm", type=str, default=None,
                        help="Path to the file containing existing GEMM configurations")
+    parser.add_argument("--gemmgemm", type=str, default=None,


let's keep it consistent with rocmlir-gen, gemm_gemm

do we want to change tier1-gemmgemm-configs to tier1-gemm_gemm-configs as well (same for conv_gemm file)? @dhernandez0

dhernandez0 · 2025-10-23T10:43:46Z

mlir/utils/performance/handleNewConfigs.py

                        help="Path to the file containing existing GEMM configurations")
+    parser.add_argument("--gemmgemm", type=str, default=None,
+                        help="Path to the file containing existing GEMM_GEMM configurations")
+    parser.add_argument("--convgemm", type=str, default=None,


dhernandez0 · 2025-10-23T10:46:14Z

mlir/utils/performance/handleNewConfigs.py


    return configs

 def detectConfigType(config) -> Optional[str]:


can we return Operation here? (perfCommonUtils.py)

dhernandez0 · 2025-10-23T10:48:45Z

mlir/utils/performance/handleNewConfigs.py

    return configs

 def detectConfigType(config) -> Optional[str]:
    """Detect config type: returns 'conv', 'gemm', or 'attention'."""


update or just don't specify conv, gemm, ...

Copilot

Pull Request Overview

This PR extends the configuration handling script to support GEMM+GEMM and CONV+GEMM configurations, and simplifies the deduplication logic by removing canonical form conversions that required complete configurations.

Key changes:

Added detection and handling for GEMM+GEMM and CONV+GEMM configuration types
Removed canonicalization logic that depended on fromCommandLine, enabling support for incomplete configurations
Switched from error to warning when encountering unrecognized config types

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

mlir/utils/performance/handleNewConfigs.py

Copilot

Pull Request Overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

mlir/utils/performance/handleNewConfigs.py

Co-authored-by: Copilot <[email protected]>

Copilot

Pull Request Overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-10-29T12:29:55Z

mlir/utils/performance/handleNewConfigs.py

    existing_conv = load_existing_configs(conv_configs)
    existing_gemm = load_existing_configs(gemm_configs)
+    existing_gemm_gemm = load_existing_configs(gemm_gemm_configs)
+    existing_conv_gemm = load_existing_configs(conv_gemm_configs)


Variable name mismatch: the function receives 'conv_gemm_config' (singular) from resolve_paths on line 172, but attempts to use 'conv_gemm_configs' (plural) here. This will cause a NameError at runtime.

Copilot · 2025-10-29T12:29:55Z

mlir/utils/performance/handleNewConfigs.py

    _append_configs(conv_configs, new_conv)
    _append_configs(gemm_configs, new_gemm)
+    _append_configs(gemm_gemm_configs, new_gemm_gemm)
+    _append_configs(conv_gemm_configs, new_conv_gemm)


Variable name mismatch: attempting to use 'conv_gemm_configs' (plural), but the variable defined on line 172 is 'conv_gemm_config' (singular). This will cause a NameError at runtime.

Copilot

Pull Request Overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (2)

mlir/utils/performance/handleNewConfigs.py:39

These global variables ARCH, CHIP, and NUM_CU are defined but no longer used after removing the canonicalization functions. Consider removing these unused global variables and their corresponding imports if they serve no other purpose in the module.

ARCH = get_arch()
CHIP = get_chip()
NUM_CU = get_num_cu(CHIP)

mlir/utils/performance/handleNewConfigs.py:49

The function read_non_empty_lines is no longer called anywhere in the code after refactoring the main loop to directly read from the file. Consider removing this unused function.

def read_non_empty_lines(path: str) -> list[str]:
    if not os.path.exists(path):
        print(f"Error: {path} does not exist")
        sys.exit(-1)
    with open(path, "r") as f:
        return [line.strip() for line in f if line.strip() and not line.strip().startswith("#")]

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

mlir/utils/performance/handleNewConfigs.py

Copilot · 2025-10-29T14:20:58Z

mlir/utils/performance/handleNewConfigs.py

+                    new_conv_gemm.append(config)
+                    existing_conv_gemm.add(config)
+            else:
+                print(f"Warning: Could not determine config type for: {config}")


The message severity changed from 'Error' to 'Warning', but the handling remains the same (just logs and continues). The previous version would print an error and continue, which was effectively a warning. While 'Warning' is more accurate, consider whether unrecognized configs should cause the script to fail (return non-zero exit code) or at least be tracked and reported in the final summary to ensure visibility of potential issues.

@copilot open a new pull request to apply changes based on this feedback

Copilot · 2025-10-29T14:40:56Z

@dorde-antic I've opened a new pull request, #2075, to work on those changes. Once the pull request is ready, I'll request review from you.

* Initial plan * Track and report unrecognized configs with non-zero exit code Co-authored-by: dorde-antic <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: dorde-antic <[email protected]>

Update handleNewConfigs script

25855b7

dorde-antic requested a review from causten as a code owner October 23, 2025 10:34

dorde-antic requested review from dhernandez0, mirza-halilcevic and umangyadav October 23, 2025 10:37

dhernandez0 reviewed Oct 23, 2025

View reviewed changes

umangyadav requested a review from Copilot October 23, 2025 13:08

Copilot AI reviewed Oct 23, 2025

View reviewed changes

mlir/utils/performance/handleNewConfigs.py Show resolved Hide resolved

mlir/utils/performance/handleNewConfigs.py Show resolved Hide resolved

address comments, use Operation class

2474aad

dorde-antic requested a review from dhernandez0 October 23, 2025 21:33

dorde-antic added 3 commits October 23, 2025 21:38

revert gemm_gemmConfigs and conv_gemmConfigs

4c1ab4c

Merge branch 'develop' into handleNewConfigs-script-update

c02a686

Merge branch 'develop' into handleNewConfigs-script-update

3b832d5

dorde-antic requested a review from Copilot October 29, 2025 12:16

Copilot AI reviewed Oct 29, 2025

View reviewed changes

mlir/utils/performance/handleNewConfigs.py Outdated Show resolved Hide resolved

Update mlir/utils/performance/handleNewConfigs.py

1e31c6c

Co-authored-by: Copilot <[email protected]>

dorde-antic requested a review from Copilot October 29, 2025 12:23

Copilot AI reviewed Oct 29, 2025

View reviewed changes

Update handleNewConfigs.py

6d15e79

dorde-antic requested a review from Copilot October 29, 2025 14:14

Copilot AI reviewed Oct 29, 2025

View reviewed changes

Copilot AI mentioned this pull request Oct 29, 2025

Track and fail on unrecognized configs in handleNewConfigs.py #2075

Merged

Copilot AI and others added 2 commits October 29, 2025 15:56

Merge branch 'develop' into handleNewConfigs-script-update

ad8508d


		return configs

		def detectConfigType(config) -> Optional[str]:

GEMM+GEMM and CONV+GEMM support in script for handling new configs #2053

Are you sure you want to change the base?

GEMM+GEMM and CONV+GEMM support in script for handling new configs #2053

Conversation

dorde-antic commented Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Technical Details

Test Plan

Test Result

Submission Checklist

Uh oh!

dhernandez0 Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

dorde-antic Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

dhernandez0 Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

dhernandez0 Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

dhernandez0 Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

dorde-antic Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Oct 29, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dorde-antic commented Oct 23, 2025 •

edited

Loading