Skip to content

feat(veir-opt): teach optimizer about collections of passes#1060

Open
regehr wants to merge 9 commits into
mainfrom
regehr/optimization-groups
Open

feat(veir-opt): teach optimizer about collections of passes#1060
regehr wants to merge 9 commits into
mainfrom
regehr/optimization-groups

Conversation

@regehr

@regehr regehr commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

I would like the optimizer to understand about common groups of passes that are used together to achieve some effect, so we don't have to have different versions of this sitting around in different parts of the repo.

I think two things are up for debate here:

  • what are the actual passes we want to invoke for "-O" and also for the RISC-V backend? I've taken some guesses and am open to feedback
  • do we want this option to compose with other pass options, so that we can build larger pipelines? for now I've gone with "no" to keep things simple

@github-actions github-actions Bot left a comment

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.

VeIR Benchmarks

Details
Benchmark suite Current: 964b643 Previous: 1971543 Ratio
add-fold-worklist/create 1839000 ns (± 68678) 2264000 ns (± 89652) 0.81
add-fold-worklist/rewrite 3672000 ns (± 73387) 4027000 ns (± 79991) 0.91
add-fold-worklist-local/create 1862000 ns (± 128621) 2225000 ns (± 106345) 0.84
add-fold-worklist-local/rewrite 3494000 ns (± 116613) 3716000 ns (± 61682) 0.94
add-zero-worklist/create 1852000 ns (± 14018) 2226000 ns (± 51046) 0.83
add-zero-worklist/rewrite 2304000 ns (± 31196) 2600000 ns (± 51267) 0.89
add-zero-reuse-worklist/create 1563000 ns (± 22554) 1889000 ns (± 90843) 0.83
add-zero-reuse-worklist/rewrite 1950000 ns (± 36074) 2127000 ns (± 49138) 0.92
mul-two-worklist/create 1849000 ns (± 15758) 2209000 ns (± 62608) 0.84
mul-two-worklist/rewrite 5092000 ns (± 50251) 5620000 ns (± 248024) 0.91
add-fold-forwards/create 1865000 ns (± 14839) 2279000 ns (± 74547) 0.82
add-fold-forwards/rewrite 2702000 ns (± 32445) 3063000 ns (± 51863) 0.88
add-zero-forwards/create 1842000 ns (± 87634) 2163000 ns (± 99039) 0.85
add-zero-forwards/rewrite 1741000 ns (± 28726) 1940000 ns (± 16682) 0.90
add-zero-reuse-forwards/create 1544000 ns (± 9290) 1902000 ns (± 49987) 0.81
add-zero-reuse-forwards/rewrite 1387000 ns (± 12268) 1574000 ns (± 33890) 0.88
mul-two-forwards/create 1987000 ns (± 80341) 2239000 ns (± 86176) 0.89
mul-two-forwards/rewrite 3476000 ns (± 103840) 3723000 ns (± 102624) 0.93
add-zero-reuse-first/create 1545500 ns (± 91981) 1837000 ns (± 97092) 0.84
add-zero-reuse-first/rewrite 10000 ns (± 2289) 8000 ns (± 1821) 1.25
add-zero-lots-of-reuse-first/create 1608000 ns (± 74869) 1924000 ns (± 83197) 0.84
add-zero-lots-of-reuse-first/rewrite 781500 ns (± 24897) 801000 ns (± 13405) 0.98

This comment was automatically generated by workflow using github-action-benchmark.

@tobiasgrosser tobiasgrosser left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I love it. Let's see what @luisacicolini @math-fehr and @nchappe say.

@math-fehr

Copy link
Copy Markdown
Collaborator

I'm not a big fan of this particular design, because as you said you cannot compose it with other groups.
Maybe it would be better to just create passes that are collection of passes, so that we can compose everything? Also, it should be fairly easy to implement

@regehr

regehr commented Jul 15, 2026

Copy link
Copy Markdown
Collaborator Author

I'm not a big fan of this particular design, because as you said you cannot compose it with other groups. Maybe it would be better to just create passes that are collection of passes, so that we can compose everything? Also, it should be fairly easy to implement

take another look please @math-fehr ?

@math-fehr

Copy link
Copy Markdown
Collaborator

Sorry, what I meant is having both -p and -pgrp in the same option (like -p).
So you would have -p=canonicalize,O,canonicalize.
I think it would make this more readable and easy to understand?

@regehr

regehr commented Jul 15, 2026

Copy link
Copy Markdown
Collaborator Author

@math-fehr ok look one more time please!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants