[PATCH] D90058: [ms] [llvm-ml] Introduce command-line compatibility for ml.exe and ml64.exe

[Eric Astor via Phabricator via llvm-commits]

epastor added a comment.

In D90058#2371375 <https://reviews.llvm.org/D90058#2371375>, @thakis wrote:

> Nice!
>
> Do tests still run after the test file renaming to .asm? llvm/test/lit.cfg.py has `config.suffixes = ['.ll', '.c', '.test', '.txt', '.s', '.mir', '.yaml']` which includes .test but not .asm. I would expect that you need to edit llvm/test/tools/llvm-ml/lit.local.cfg to have `config.suffixes.add('.asm,')` for the tests to keep running.

... they do now. Thanks for catching this! (They still pass.)



================
Comment at: llvm/tools/llvm-ml/Opts.td:6
+class LLVMJoinedOrSeparate<string name> : JoinedOrSeparate<["-", "/"], name>;
+class LLVMSeparate<string name> : Separate<["-", "/"], name>;
+
----------------
thakis wrote:
> Do we need the non-ml-compat options for anything? If so, why support '/' prefixes for this? Why not the more usual '--' here? (see also lld/ELF/Options.td for some color on this.)
> 
> What's with the '.' syntax?
We do need it for now - it lets users select things like output filetype. (We might be able to replace that with the listing options once those are implemented, and generally I plan on working to converge towards ML.EXE's interface.) I've switched to supporting only "-" and "--" here; I'm keeping "-" available for partial compatibility with llvm-mc.

(And the "." was a typo.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90058