[llvm-ml] Add support for the -o flag

[ayzhao]

Summary

Many LLVM tools, such as clang-cl, that accept the flag /Fo to specify
output files are also tolerant of the -o flag to do the same thing. To
maintain parity with these other tools, llvm-ml should also support -o.

As with clang-cl, if both /Fo and -o are specified, then /Fo takes
precedence[0].

[0]: https://github.com/llvm/llvm-project/blob/175b9af484f483c3423ab2f78db5de7e25b64c31/clang/lib/Driver/ToolChains/Clang.cpp#L987-L992

[epastor]

llvm/test/tools/llvm-ml/output_flag.asm

9–12

Since we expect the same output in all cases, I don't think we need three different prefixes?

Also, we should probably test that you actually get no output if outputting to /dev/null.

[ayzhao]

Reduce the number of test prefixes and check output to /dev/null

[hans]

For clang-cl we support -o because MSVC supports it (although in the VS 2019 version I have it causes a warning saying it's deprecated). Does MSVC's ml.exe or ml64.exe support -o?

[ayzhao]

Append a newline to the end of output_flag.asm

[ayzhao]

In D123173#3433575, @hans wrote:

For clang-cl we support -o because MSVC supports it (although in the VS 2019 version I have it causes a warning saying it's deprecated). Does MSVC's ml.exe or ml64.exe support -o?

ml64.exe doesn't support -o, but I think there's still some value in adding it to llvm-ml

1. It keeps command line flags consistent with other LLVM tools
2. When cross-compiling from *nix to Windows, having *nix-style flags can be beneficial as Windows style flags beginning with a forward slash look like file paths. I believe that this would make things less buggy.

[epastor]

I'm fine with this and want to remove my request for changes - but please make sure Hans approves as well.

[hans]

In D123173#3433751, @ayzhao wrote:

In D123173#3433575, @hans wrote:

For clang-cl we support -o because MSVC supports it (although in the VS 2019 version I have it causes a warning saying it's deprecated). Does MSVC's ml.exe or ml64.exe support -o?

ml64.exe doesn't support -o, but I think there's still some value in adding it to llvm-ml

1. It keeps command line flags consistent with other LLVM tools
2. When cross-compiling from *nix to Windows, having *nix-style flags can be beneficial as Windows style flags beginning with a forward slash look like file paths. I believe that this would make things less buggy.

I thought the purpose of llvm-ml was to act as a drop-in replacement of ml64.exe, in which case that's the command-line interface it should have. If we try make it support both ml64.exe's options and GNU as-inspired options at the same time, I worry we might en up with a bad interface.

Other llvm tools generally use a Unix-style interface, where the -o option is common, but this is not common for MSVC's tools. For clang-cl, we only added it because MSVC supports it and there was code that relied on it (see discussion in https://github.com/llvm/llvm-project/issues/21268). What's the use case for llvm-ml supporting -o?

As for using the forward slashes in cross-compiles from Unix to Windows, ml64.exe (like the other MSVC tools) also accept the flags with dashes, i.e. -Fo should work.

I guess the summary is that unless there's a compelling use case that needs llvm-ml to support -o, we should stick to the flags that ml64.exe uses.

[ayzhao]

In D123173#3434264, @hans wrote:

In D123173#3433751, @ayzhao wrote:

In D123173#3433575, @hans wrote:

For clang-cl we support -o because MSVC supports it (although in the VS 2019 version I have it causes a warning saying it's deprecated). Does MSVC's ml.exe or ml64.exe support -o?

ml64.exe doesn't support -o, but I think there's still some value in adding it to llvm-ml

1. It keeps command line flags consistent with other LLVM tools
2. When cross-compiling from *nix to Windows, having *nix-style flags can be beneficial as Windows style flags beginning with a forward slash look like file paths. I believe that this would make things less buggy.

I thought the purpose of llvm-ml was to act as a drop-in replacement of ml64.exe, in which case that's the command-line interface it should have. If we try make it support both ml64.exe's options and GNU as-inspired options at the same time, I worry we might en up with a bad interface.

Other llvm tools generally use a Unix-style interface, where the -o option is common, but this is not common for MSVC's tools. For clang-cl, we only added it because MSVC supports it and there was code that relied on it (see discussion in https://github.com/llvm/llvm-project/issues/21268). What's the use case for llvm-ml supporting -o?

As for using the forward slashes in cross-compiles from Unix to Windows, ml64.exe (like the other MSVC tools) also accept the flags with dashes, i.e. -Fo should work.

I guess the summary is that unless there's a compelling use case that needs llvm-ml to support -o, we should stick to the flags that ml64.exe uses.

I'll abandon this patch given that -Fo exists.

llvm/test/tools/llvm-ml/output_flag.asm

+; RUN: llvm-ml --filetype=s %s /Fo - | FileCheck %s --check-prefix=HAS_OUTPUT
+; RUN: llvm-ml --filetype=s %s -o - | FileCheck %s --check-prefix=HAS_OUTPUT
+; /Fo takes precedence over -o, so we should still expect to see output
+; RUN: llvm-ml --filetype=s %s -o /dev/null /Fo - | FileCheck %s --check-prefix=HAS_OUTPUT
+
+; RUN: llvm-ml --filetype=s %s /Fo /dev/null | FileCheck %s --allow-empty --check-prefix=NO_OUTPUT
+; RUN: llvm-ml --filetype=s %s -o /dev/null | FileCheck %s --allow-empty --check-prefix=NO_OUTPUT
+
+.code
+
+t1:
+; HAS_OUTPUT: t1:
+; NO_OUTPUT-NOT: t1:
+
+END

llvm/tools/llvm-ml/Opts.td

 def as_lex : LLVMFlag<"as-lex">,
              HelpText<"Lex tokens from a .asm file without assembling">;
 def fatal_warnings : LLVMFlag<"fatal-warnings">,
                      HelpText<"Treat warnings as errors">;
 def filetype : LLVMJoined<"filetype=">, Values<"obj,s,null">,
                HelpText<"Emit a file with the given type">;
 def output_att_asm : LLVMFlag<"output-att-asm">,
                      HelpText<"Use ATT syntax for output assembly">;
+def output_file_llvm : LLVMJoinedOrSeparate<"o">,
+                       HelpText<"Names the output file">;
 def show_encoding : LLVMFlag<"show-encoding">,
                     HelpText<"Show instruction encodings in output assembly">;
 def show_inst : LLVMFlag<"show-inst">,
                 HelpText<"Show internal instruction representation in output "
                          "assembly">;
 def show_inst_operands : LLVMFlag<"show-inst-operands">,
                          HelpText<"Show instructions operands as parsed">;
 def print_imm_hex : LLVMFlag<"print-imm-hex">,

llvm/tools/llvm-ml/llvm-ml.cpp

int main(int Argc, char **Argv) {
   StringRef FileType = InputArgs.getLastArgValue(OPT_filetype, "obj");
   SmallString<255> DefaultOutputFilename;
   if (InputArgs.hasArg(OPT_as_lex)) {
     DefaultOutputFilename = "-";
   } else {
     DefaultOutputFilename = InputFilename;
     sys::path::replace_extension(DefaultOutputFilename, FileType);
   }
-  const StringRef OutputFilename =
-      InputArgs.getLastArgValue(OPT_output_file, DefaultOutputFilename);
+  StringRef OutputFilename;
+  if (Arg *OutputFile = InputArgs.getLastArg(OPT_output_file))
+    OutputFilename = OutputFile->getValue();
+  else if (Arg *OutputFile = InputArgs.getLastArg(OPT_output_file_llvm))
+    OutputFilename = OutputFile->getValue();
+  else
+    OutputFilename = DefaultOutputFilename;
   std::unique_ptr<ToolOutputFile> Out = GetOutputStream(OutputFilename);
   if (!Out)
     return 1;
 
   std::unique_ptr<buffer_ostream> BOS;
   raw_pwrite_stream *OS = &Out->os();
   std::unique_ptr<MCStreamer> Str;