Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Opt passes= doesn't work if the first one is a function pass #81128

Closed
JonChesterfield opened this issue Feb 8, 2024 · 4 comments
Closed

Opt passes= doesn't work if the first one is a function pass #81128

JonChesterfield opened this issue Feb 8, 2024 · 4 comments
Labels
question A question, not bug report. Check out https://llvm.org/docs/GettingInvolved.html instead! tools:opt

Comments

@JonChesterfield
Copy link
Collaborator

OK:
opt --passes=inline,instcombine whatever.ll -S

Fails:
opt --passes=instcombine,inline whatever.ll -S

unknown function pass 'inline'

I think if the first pass named is a module pass, putting several in sequence works as expected. If the first is a function pass, it fails to find the later ones. Discovered writing tests for other passes.

This seems unlikely to be intended behaviour, though I can't find documentation for passes= so maybe it was. It's not helpful behaviour.

Ideally one should be able to string a long sequence of comma separated passes together and have them all run, without opt failing to find some passes on some permutations.

@fhahn
Copy link
Contributor

fhahn commented Feb 8, 2024

I think this works as expected. Without looking into the details here, I think the first case works, because the first pass is a module pass, it defaults to module passes. It then automatically can create a function pass adapter for the module to run on all functions in the module.

This doesn't work for the second example, because now the first pass is a function pass and we don't have an adaptor that runs a module pass from a function pass.

You can explicitly for a specific scope as in opt -passes='function(instcombine),module(inline)'.

@aeubanks
Copy link
Contributor

aeubanks commented Feb 8, 2024

this is documented at https://llvm.org/docs/NewPassManager.html#invoking-opt

@EugeneZelenko EugeneZelenko added question A question, not bug report. Check out https://llvm.org/docs/GettingInvolved.html instead! tools:opt and removed llvm:optimizations labels Feb 8, 2024
@JonChesterfield
Copy link
Collaborator Author

JonChesterfield commented Feb 20, 2024

I think "expected" is a stretch, particularly given the diagnostic feedback if you fail to guess that passes= is not a comma separated list of passes despite appearances.

I will concede it is as documented (though not as far as I can tell under opt -help) and I can't tell what the author means by nested.

Passes have a single type - module, function, loop etc. They run in left to right order. Provided we don't reuse the same name for multiple different passes (and even if we did), I think where these nested/wrapper things need to go is completely determined by a list of pass names.

Slightly extending the existing "implicit creation" logic seems likely to make passes= a comma separated list without needing any of the foo(...) annotation.

Does something force this obscure interface or is it a matter of documenting something instead of fixing it?

@JonChesterfield
Copy link
Collaborator Author

I've just run into this again where this time I remembered that one must write function and module in special places but still failed to guess the proper placement of parentheses and commas.

I still think opt should cope with a comma separated list of passes to run, and run them left to right, but two comments saying the current behaviour is as desired means I'm going to close the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question A question, not bug report. Check out https://llvm.org/docs/GettingInvolved.html instead! tools:opt
Projects
None yet
Development

No branches or pull requests

4 participants