This is an archive of the discontinued LLVM Phabricator instance.

[Flang][Driver] Add support for AsmPrinter -mmlir options
ClosedPublic

Authored by psoni2628 on Jul 26 2022, 2:11 PM.

Details

Summary

This patch adds support for AsmPrinter -mmlir options to the Flang driver.

Diff Detail

Event Timeline

psoni2628 created this revision.Jul 26 2022, 2:11 PM
psoni2628 requested review of this revision.Jul 26 2022, 2:11 PM
psoni2628 set the repository for this revision to rG LLVM Github Monorepo.Jul 26 2022, 2:12 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 26 2022, 2:12 PM
psoni2628 edited the summary of this revision. (Show Details)Jul 26 2022, 2:12 PM

Thanks for doing this!

I think that at this point it would make sense to extend your test and to check for options from registerMLIRContextCLOptions and registerPassManagerCLOptions as well (one from each should be sufficient). Would you mind doing that? You'll need to rename the test file as well.

psoni2628 updated this revision to Diff 448080.Jul 27 2022, 9:42 AM
  • Add registerPassManagerCLOptions and registerMLIRContextCLOptions options to the test

Thanks for doing this!

I think that at this point it would make sense to extend your test and to check for options from registerMLIRContextCLOptions and registerPassManagerCLOptions as well (one from each should be sufficient). Would you mind doing that? You'll need to rename the test file as well.

Absolutely. Hope the new test file name is OK.

awarzynski accepted this revision.Jul 27 2022, 11:36 AM

LGTM!

flang/test/Driver/mmlir-opts.f90
7–10

[nit] You could add a comment so that it clear that e.g. --mlir-print-local-scope is from registerAsmPrinterCLOptions (same for other groups)

This revision is now accepted and ready to land.Jul 27 2022, 11:36 AM
  • Add comment about which option comes from where
flang/test/Driver/mmlir-opts.f90
7–10

Is the added comment clear?

awarzynski added inline comments.Jul 27 2022, 11:56 AM
flang/test/Driver/mmlir-opts.f90
11–13

It is clear, thank you!

Personally, I would do it more locally, but this was just a [nit] anyway, so feel free to ignore :)

psoni2628 marked 2 inline comments as done.
  • Fix the previously added comment to make it more local
psoni2628 added a comment.EditedJul 27 2022, 12:08 PM
This comment has been deleted.
flang/test/Driver/mmlir-opts.f90
11–13

Ah, I like this better. Done, thanks for your feedback!

This revision was landed with ongoing or failed builds.Jul 27 2022, 1:06 PM
This revision was automatically updated to reflect the committed changes.