[PATCH] D101695: [llvm-objdump] Add -M {att,intel} as user-facing alternatives to --x86-asm-syntax={att,intel}

[Hongtao Yu via Phabricator via llvm-commits]

hoy accepted this revision.
hoy added a comment.

Current change LGTM. More thoughts may be needed on retiring the `--x86-asm-syntax=` options which may break downstream usage.



================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:2480
+  llvm::erase_if(DisassemblerOptions, [&](StringRef V) {
+    if (V == "att") {
+      LLVMArgs.push_back("--x86-asm-syntax=att");
----------------
MaskRay wrote:
> hoy wrote:
> > MaskRay wrote:
> > > hoy wrote:
> > > > MaskRay wrote:
> > > > > MaskRay wrote:
> > > > > > hoy wrote:
> > > > > > > thakis wrote:
> > > > > > > > equals_lower() for better objdump compat?
> > > > > > > Can this be made an alias switch of `OBJDUMP_x86_asm_syntax_att`?
> > > > > > I think GNU objdump uses startswith for case-sensitive "intel" and "att" and ignores unrecognized values.
> > > > > > 
> > > > > > I don't think anyone wants to use "intel*" (other than the unimplemented "intel-mnenomic" so I'll just use rigid `==` here.
> > > > > > Can this be made an alias switch of OBJDUMP_x86_asm_syntax_att?
> > > > > 
> > > > > No. `llvm-objdump --x86-asm-syntax` will retire.
> > > > Or the other way around? The `AsmSyntax` assignments looks a bit redundant. It's fine if you are going to remove the `OBJDUMP_x86_asm` cases later.
> > > No, it is needed to avoid duplicate `--x86-asm-syntax`.
> > > 
> > I'm confused. What do you mean by "--x86-asm-syntax will retire."? Are `OBJDUMP_x86_asm_syntax_att` and `OBJDUMP_x86_asm_syntax_intel` going away?
> `llvm-objdump --x86-asm-syntax=` will go away.
> 
> `x86-asm-syntax` is an internal cl::opt option, not intended to be exposed. `-M intel` will work by setting the internal `x86-asm-syntax` option.
Thanks for the clarification. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101695