[PATCH] D103004: [llvm-mc] Add -M to replace -riscv-no-aliases and -riscv-arch-reg-names

[Fangrui Song via Phabricator via llvm-commits]

MaskRay marked an inline comment as done.
MaskRay added inline comments.


================
Comment at: llvm/include/llvm/MC/MCInstPrinter.h:58
+  /// True if we prefer aliases (e.g. nop) to raw mnemonics.
+  bool PrintAliases = true;
+
----------------
tmatheson wrote:
> It would be better if we could avoid ending up with two variables that mean essentially the same thing. I prefer the name `NoAliases` because it matches both command line flags better, and doesn't have the logical inversion that `PrintAliases` does. Can they both be made to use the same storage with `cl::location`?
No. RISCV is at odds here. I will use this for aarch64 as well: D103005.

The `cl::opt` global variable is inelegant and should be avoided. Say we want to have MCInstPrinter, now due to the singleton nature of a `cl::opt` variable, we cannot really make their `PrintAliases` states different.

Regarding the name, The positively named `PrintAliases` avoid negation, instead of the other way around.
If we add something `-M aliases` then it should become clearer `NoAliases` is not a good name.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103004/new/

https://reviews.llvm.org/D103004