This is an archive of the discontinued LLVM Phabricator instance.

[mlir] Prefix pass manager options with `mlir-`
ClosedPublic

Authored by awarzynski on Apr 11 2022, 4:19 AM.

Details

Summary

With this change, there's going to be a clear distinction between LLVM
and MLIR pass maanger options (e.g. -mlir-print-after-all vs
-print-after-all). This change is desirable from the point of view of
projects that depend on both LLVM and MLIR, e.g. Flang.

For consistency, all pass manager options in MLIR are prefixed with
mlir-, even options that don't have equivalents in LLVM .

Diff Detail

Event Timeline

awarzynski created this revision.Apr 11 2022, 4:19 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 11 2022, 4:19 AM
awarzynski requested review of this revision.Apr 11 2022, 4:19 AM

Fix -mlir-mlir-pass-statistics-display

rriddle accepted this revision.Apr 11 2022, 10:07 AM

Thanks!

mlir/docs/PassManagement.md
1143

-print-ir-failure

Does this need to be updated?

This revision is now accepted and ready to land.Apr 11 2022, 10:07 AM
awarzynski added inline comments.Apr 11 2022, 1:34 PM
mlir/docs/PassManagement.md
1143

Ah, it should be -print-ir-after-failure _before_ this change and -mlir-print-ir-after-failure _after_. I'll update this before merging.

rovka added a subscriber: rovka.Apr 12 2022, 1:04 AM

Just a thought: What do you think about adding an overload for llvm::cl::opt that also takes a prefix, and encourage people to use that for internal things? That way it will be easier for people to remember to add "mlir" to such options (and maybe we could make the same change in llvm, to add "llvm" as a prefix).

Anyway, I'm not objecting to doing things like this for now, it's just a suggestion (I guess people will just copy paste whatever's already in the file they're working on, so if all the options there have names like "mlir-blah", they'll stick to the pattern).

Just a thought: What do you think about adding an overload for llvm::cl::opt that also takes a prefix, and encourage people to use that for internal things?

In general, +1. I guess that there are two things here:

  1. the option spellings that are visible to the end users and to other sub-projects (i.e. pre-fixing globally visible options is good),
  2. what's the best way to implement this?

For now, let me address 1. and merge this as is :)

This revision was automatically updated to reflect the committed changes.