[llvm-ml] Add support for the .S extension

[ayzhao]

Summary

Even though MASM files typically have the .asm extension, there are some
use cases [0] where they have the .S extension. MSVC ml assembles such
files with no problems, so llvm-ml should as well.

Additionally, fix the implementation of the /Ta flag and add a test for
it.

[0]: https://crrev.com/c/3668287

[ayzhao]

fix commit message

[ayzhao]

add newline at end of file

[epastor]

llvm/test/tools/llvm-ml/invalid_file_extension.blah

2

While we're at it, mind testing the /Ta option as well, which is supposed to allow assembly of any file regardless of extension?

I think you can just add another RUN line here using /Ta, and use a different CHECK suffix to distinguish the results.

[ayzhao]

Fix the /Ta flag and add a test for it.

[ayzhao]

llvm/test/tools/llvm-ml/invalid_file_extension.blah

2

Done.

Incidentally, this exposed a bug in the implementation of /Ta, which I fixed.

[epastor]

llvm/test/tools/llvm-ml/invalid_file_extension.blah

2

Thank you!

[hans]

It appears to me that MASM doesn't care much about the file extension at all:

C:\src\tmp>type a.blah
.code

main proc
  ret
main endp

END
C:\src\tmp>ml64 a.blah /link /entry:main
Microsoft (R) Macro Assembler (x64) Version 14.28.29336.0
Copyright (C) Microsoft Corporation.  All rights reserved.

 Assembling: a.blah
Microsoft (R) Incremental Linker Version 14.28.29336.0
Copyright (C) Microsoft Corporation.  All rights reserved.

/OUT:a.exe
a.obj
/entry:main

The fact that /Ta exists suggest maybe it did care at some point, but that may have changed.

[epastor]

In D126425#3545711, @hans wrote:

It appears to me that MASM doesn't care much about the file extension at all:

[test results omitted]

The fact that /Ta exists suggest maybe it did care at some point, but that may have changed.

Thanks for doing the testing! Sounds like something we should match them on, then... /Ta will still be useful when on systems where a filename could start with a /, so we shouldn't make it a no-op, but otherwise let's remove the extension restriction.

(Assuming this still holds for ml.exe, as well as ml64.exe... they do have different behavior sometimes.)

[ayzhao]

In D126425#3547306, @epastor wrote:

In D126425#3545711, @hans wrote:

It appears to me that MASM doesn't care much about the file extension at all:

[test results omitted]

The fact that /Ta exists suggest maybe it did care at some point, but that may have changed.

Thanks for doing the testing! Sounds like something we should match them on, then... /Ta will still be useful when on systems where a filename could start with a /, so we shouldn't make it a no-op, but otherwise let's remove the extension restriction.

(Assuming this still holds for ml.exe, as well as ml64.exe... they do have different behavior sometimes.)

D126931 removes all file extension restrictions.

llvm/test/tools/llvm-ml/invalid_file_extension.blah

+; RUN: not llvm-ml %s /Fo /dev/null 2>&1 | FileCheck %s --check-prefixes=CHECK-INVALID
+; RUN: llvm-ml /Ta %s -m64 -filetype=s /Fo - | FileCheck %s --check-prefixes=CHECK-TA
+
+; CHECK-INVALID: error: invalid option '{{.*}}invalid_file_extension.blah'
+
+.code
+foo:
+; CHECK-TA: foo:

llvm/test/tools/llvm-ml/lit.local.cfg

 if not ('X86' in config.root.targets):
     # We need support for X86.
     config.unsupported = True
 
 config.suffixes.add('.asm')
+config.suffixes.add('.S')
+config.suffixes.add('.blah')

llvm/test/tools/llvm-ml/valid_file_extension.S

+; RUN: llvm-ml -m64 -filetype=s %s /Fo - | FileCheck %s
+
+.code
+foo:
+; CHECK: foo:

llvm/tools/llvm-ml/Opts.td

 class UnsupportedJoinedOrSeparate<string name> :
         JoinedOrSeparate<["/", "-"], name>, Group<unsupported_Group>;
 class UnsupportedSeparate<string name> : Separate<["/", "-"], name>,
                                          Group<unsupported_Group>;
 
 def bitness : LLVMJoined<"m">, Values<"32,64">,
               HelpText<"Target platform (x86 or x86-64)">;
 def as_lex : LLVMFlag<"as-lex">,
-             HelpText<"Lex tokens from a .asm file without assembling">;
+             HelpText<"Lex tokens from an .asm or .S file without assembling">;
 def debug : LLVMFlag<"debug">, Flags<[HelpHidden]>,
             HelpText<"Enable debug output">;
 def debug_only : LLVMCommaJoined<"debug-only=">, Flags<[HelpHidden]>,
                  HelpText<"Enable a specific type of debug output (comma "
                           "separated list of types)">;
 def fatal_warnings : LLVMFlag<"fatal-warnings">,
                      HelpText<"Treat warnings as errors">;
 def filetype : LLVMJoined<"filetype=">, Values<"obj,s,null">,
def include_path : MLJoinedOrSeparate<"I">,
                    HelpText<"Sets path for include files">;
 def safeseh : MLFlag<"safeseh">,
               HelpText<"Mark resulting object files as either containing no "
                        "exception handlers or containing exception handlers "
                        "that are all declared with .SAFESEH. Only available in "
                        "32-bit.">;
 def assembly_file : MLJoinedOrSeparate<"Ta">,
                     HelpText<"Assemble source file with name not ending with "
-                             "the .asm extension">;
+                             "the .asm or the .S extension">;
 def error_on_warning : MLFlag<"WX">, Alias<fatal_warnings>;
 def parse_only : MLFlag<"Zs">, HelpText<"Run a syntax-check only">,
                  Alias<filetype>, AliasArgs<["null"]>;
 def ignore_include_envvar : MLFlag<"X">,
                             HelpText<"Ignore the INCLUDE environment variable">;
 
 def tiny_model_support : UnsupportedFlag<"AT">, HelpText<"">;
 def alternate_linker : UnsupportedJoined<"Bl">, HelpText<"">;

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

int main(int Argc, char **Argv) {
   unsigned MissingArgIndex, MissingArgCount;
   ArrayRef<const char *> ArgsArr = makeArrayRef(Argv + 1, Argc - 1);
   opt::InputArgList InputArgs =
       T.ParseArgs(ArgsArr, MissingArgIndex, MissingArgCount);
 
   std::string InputFilename;
   for (auto *Arg : InputArgs.filtered(OPT_INPUT)) {
     std::string ArgString = Arg->getAsString(InputArgs);
-    if (ArgString == "-" || StringRef(ArgString).endswith(".asm")) {
+    StringRef ArgStringRef(ArgString);
+    if (ArgString == "-" || ArgStringRef.endswith(".asm") ||
+        ArgStringRef.endswith(".S")) {
       if (!InputFilename.empty()) {
         WithColor::warning(errs(), ProgName)
             << "does not support multiple assembly files in one command; "
             << "ignoring '" << InputFilename << "'\n";
       }
       InputFilename = ArgString;
     } else {
       std::string Diag;
for (auto *Arg : InputArgs.filtered(OPT_INPUT)) {
     }
   }
   for (auto *Arg : InputArgs.filtered(OPT_assembly_file)) {
     if (!InputFilename.empty()) {
       WithColor::warning(errs(), ProgName)
           << "does not support multiple assembly files in one command; "
           << "ignoring '" << InputFilename << "'\n";
     }
-    InputFilename = Arg->getAsString(InputArgs);
+    InputFilename = Arg->getValue();
   }
 
   for (auto *Arg : InputArgs.filtered(OPT_unsupported_Group)) {
     WithColor::warning(errs(), ProgName)
         << "ignoring unsupported '" << Arg->getOption().getName()
         << "' option\n";
   }