Split gpu passes into seperate pipelines by pfultz2 · Pull Request #4904 · ROCm/AMDMIGraphX

pfultz2 · 2026-05-21T20:01:45Z

Motivation

This will make it easier to use to change the passes needed for different compilation modes.

Technical Details

None of the passes are changed here. In the future we could look at moving some of the passes around to put into another pipeline that makes more sense.

Changelog Category

Add a CHANGELOG.md entry for any option other than Not Applicable

- Added: New functionality.
- Changed: Changes to existing functionality.
- Removed: Functionality or support that has been removed. (Compared to a previous release)
- Optimized: Component performance that has been optimized or improved.
- Resolved Issues: Known issues from a previous version that have been resolved.
- Not Applicable: This PR is not to be included in the changelog.

Copilot

Pull request overview

This PR refactors the GPU target compilation pass list into several named sub-pipelines, making it easier to adjust the pass sequence for different compilation modes without changing the underlying passes.

Changes:

Introduces a local pipeline_factory to group GPU passes into logical pipelines (dynamic shapes, required, optimize/rewrite, fusion, backend).
Reassembles the final pass vector by concatenating these pipelines (preserving the existing pass order).

+    std::vector<std::vector<pass>> pipelines = {
+        p.dynamic_shapes_pipeline(),
+        p.required_pipeline(),
+        p.optimize_rewrite_pipeline(),
+        p.fusion_pipeline(),
+        p.backend_pipeline(),
    };
-    // clang-format on
+
+    std::vector<pass> passes;
+    std::copy(pipelines.begin(), pipelines.end(), join_back_inserter(passes));
+    return passes;


TedThemistokleous

I like the idea of this PR as it provides a cleaner mental model of how passes for GPU interact and gives us flexibility to play with ordering. Certainly good from a context what fits where.

My comments are more about

Grouping - Quantization ops themselves in a separate pass - Allows us to toggle later
Outstanding fusion passes (fuse_horizontal) outside of the fusion_pipeline() pass
General question about dead code elimination (DCE) with respect to passes or back to back. - Possible to optimize back
Might be a nitpick - Naming of required_pipeline - we should make this "core" if that's or key piece to optimize a model.

Feel free to comment inline here or in github comments.

TedThemistokleous · 2026-05-21T20:16:00Z

+                        fp8_ocp_to_fnuz{}),
+            enable_pass(not gpu::gfx_has_fp8ocp_intrinsics() and gpu::gfx_has_fp8fnuz_intrinsics(),
+                        dead_code_elimination{}),
+            simplify_qdq{.use_mx_quant = gpu::gfx_has_mx_intrinsics()},


Do we want to maybe split out the blocks for quantitation here too?

rewrite_quantization, simplify_qdq, fp8 related stuff can be bundled

TedThemistokleous · 2026-05-21T20:17:58Z

+            dead_code_elimination{},
+            prefuse_ops{get_context()},
+            dead_code_elimination{},
+            dead_code_elimination{},


I know this in the older change-set but this looks odd to me. Is there a reason we're doing this twice vs interleave after every pass?

I think we moved a pass and didnt remove the extra DCE.

TedThemistokleous · 2026-05-21T20:20:01Z

+
+    std::vector<std::vector<pass>> pipelines = {
+        p.dynamic_shapes_pipeline(),
+        p.required_pipeline(),


Name seems odd compared to the others. Can we call this maybe "core pipeline"

TedThemistokleous · 2026-05-21T20:24:08Z

+            optimize_module{},
+            layout_convolution{.channels_last = enabled(MIGRAPHX_ENABLE_NHWC{})},
+            dead_code_elimination{},
+            enable_pass(disabled(MIGRAPHX_ENABLE_FULL_DYNAMIC{}), fuse_horizontal{}),


Is there any way we can put this in the fusion_pipeline pipeline then? Does ordering matter for this one?

It shouldn't go in the fusion pipeline. And ordering does matter for this,

TedThemistokleous

Approving this since the intent is to not change the passes here.

kahmed10

I think we should still mention this in the changelog somewhere because this grouping could be referenced for future versions.

We could look into having the ability/API for disabling passes within a pipeline in the future for finer-grained control.

Otherwise LGTM

pfultz2 · 2026-05-22T14:36:06Z

I think we should still mention this in the changelog somewhere because this grouping could be referenced for future versions.

I dont think that makes sense at all, and there is no selection in the changelog for no functionality changes.

We could look into having the ability/API for disabling passes within a pipeline in the future for finer-grained control.

We could add to the changelog when we expose those things to the API as that will add new functionality, but its very unlikely we would, and I am strongly opposed to exposing this to the API. That is the whole reason for adding the compilation modes.

codecov · 2026-05-22T17:01:30Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

Additional details and impacted files

@@           Coverage Diff            @@
##           develop    #4904   +/-   ##
========================================
  Coverage    92.66%   92.66%           
========================================
  Files          588      588           
  Lines        30412    30412           
========================================
  Hits         28180    28180           
  Misses        2232     2232

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

kahmed10 · 2026-05-22T18:18:18Z

I think we should still mention this in the changelog somewhere because this grouping could be referenced for future versions.

I dont think that makes sense at all, and there is no selection in the changelog for no functionality changes.

We could look into having the ability/API for disabling passes within a pipeline in the future for finer-grained control.

We could add to the changelog when we expose those things to the API as that will add new functionality, but its very unlikely we would, and I am strongly opposed to exposing this to the API. That is the whole reason for adding the compilation modes.

Ok that's fine. I thought we wanted the pipelines to be configurable. But if we are just using it as a way to support different compilation modes, that's fine too.

pfultz2 added 2 commits May 21, 2026 14:58

Seperate passes into seperate pipelines

9d44d32

Format

be0973f

pfultz2 requested a review from causten as a code owner May 21, 2026 20:01

Copilot AI review requested due to automatic review settings May 21, 2026 20:01

Copilot started reviewing on behalf of pfultz2 May 21, 2026 20:01 View session

pfultz2 requested review from TedThemistokleous, kahmed10 and pnikolic-amd May 21, 2026 20:04

Copilot AI reviewed May 21, 2026

View reviewed changes

TedThemistokleous reviewed May 21, 2026

View reviewed changes

TedThemistokleous approved these changes May 21, 2026

View reviewed changes

kahmed10 requested changes May 21, 2026

View reviewed changes

pfultz2 added 2 commits May 21, 2026 17:52

Make const method

e432b59

Suppress cppcheck warning

72c2461

kahmed10 approved these changes May 22, 2026

View reviewed changes

pnikolic-amd approved these changes May 25, 2026

View reviewed changes

pfultz2 and others added 2 commits May 25, 2026 13:41

Put the right suppression

b39b2d8

Merge branch 'develop' into pipeline-passes

29aa7f0

pfultz2 merged commit a476c52 into develop May 26, 2026
38 checks passed

pfultz2 deleted the pipeline-passes branch May 26, 2026 13:22

Conversation

pfultz2 commented May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Technical Details

Changelog Category

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

TedThemistokleous left a comment

Choose a reason for hiding this comment

Uh oh!

TedThemistokleous May 21, 2026

Choose a reason for hiding this comment

Uh oh!

TedThemistokleous May 21, 2026

Choose a reason for hiding this comment

Uh oh!

pfultz2 May 21, 2026

Choose a reason for hiding this comment

Uh oh!

TedThemistokleous May 21, 2026

Choose a reason for hiding this comment

Uh oh!

TedThemistokleous May 21, 2026

Choose a reason for hiding this comment

Uh oh!

pfultz2 May 21, 2026

Choose a reason for hiding this comment

Uh oh!

TedThemistokleous left a comment

Choose a reason for hiding this comment

Uh oh!

kahmed10 left a comment

Choose a reason for hiding this comment

Uh oh!

pfultz2 commented May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

kahmed10 commented May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

pfultz2 commented May 21, 2026 •

edited

Loading

pfultz2 commented May 22, 2026 •

edited

Loading

codecov Bot commented May 22, 2026 •

edited

Loading

kahmed10 commented May 22, 2026 •

edited

Loading