Page MenuHomePhabricator

[NewPM] Print passes with params when using "opt -print-passes"
ClosedPublic

Authored by bjope on Jun 21 2021, 2:25 AM.

Details

Summary

Make sure we also print passes with params when using "opt -print-passes".

Diff Detail

Event Timeline

bjope created this revision.Jun 21 2021, 2:25 AM
bjope requested review of this revision.Jun 21 2021, 2:25 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 21 2021, 2:25 AM

could you add to the existing -print-passes test one loop pass and one function pass?

formatting

I'm worried that these will get out of sync with the actual implementation, but maybe that doesn't matter so much

another worry is that I believe some of these parsers accept multiple options, but that's not really mentioned in the semicolon separated list, WDYT?

bjope updated this revision to Diff 353466.Jun 21 2021, 12:58 PM

Formatting fixes + updated test case for print-passes.

bjope added a comment.Jun 21 2021, 1:18 PM

I'm worried that these will get out of sync with the actual implementation, but maybe that doesn't matter so much

Yes. I'm a bit worried as well. I considered to leave out the actual params in the printout, but wanted to make them visible.
One idea I had was to reference a static string (or a function returning the string) in PassBuilder.cpp instead of listing the params in the PassRegistry. That string/function could be placed next to the parser function, and maybe then it would be more easy to see that the string should be updated when changing the parser function.

Maybe the ideal solution would be to make the parameter parsing a bit more generic, and base it on options listed in the PassRegistery definition. But that is also a bit more work.

As you say, maybe it doesn't matter that much if it goes out of sync. Someone will eventually notice and fix it. At least if someone will base some kind of fuzzy testing on parsing of the "-print-passes" output.

another worry is that I believe some of these parsers accept multiple options, but that's not really mentioned in the semicolon separated list, WDYT?

I do not have any good ideas on how to document the rules for that in the output.

I used semicolon as separator as that is how params are separated when using multiple options. But I do not think that it is obvious that all those are more or less optional. Neither does it say which options that are default, etc. I just wanted to somehow list all options (including the ones with "no-" prefix) to give the user some hints on what the parser accepts. The details are still in the code implementing the parsers, unfortunately.

This revision is now accepted and ready to land.Jun 21 2021, 4:21 PM
This revision was landed with ongoing or failed builds.Jun 22 2021, 12:20 AM
This revision was automatically updated to reflect the committed changes.