[PATCH] D90058: [ms] [llvm-ml] Introduce command-line compatibility for ml.exe and ml64.exe
Eric Astor via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Nov 5 11:25:39 PST 2020
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
More information about the llvm-commits
mailing list