[Driver][MSVC] Move lld specific flags before inputs

[HaohaiWen]

Summary

Check linker earlier so that lld specific flags can be append before
inputs. Just like position of other flags. The intention is to make
expanded cmd more consistent and elegent so that user can easily
look for inputs when using -###.

[mstorsjo]

Is this a NFC change, as a preparation for a separate change? In that case, please add an NFC tag to the subject - if not, please explain (and test) the expected behaviour change.

[HaohaiWen]

Add tests

[HaohaiWen]

In D154176#4465223, @mstorsjo wrote:

Is this a NFC change, as a preparation for a separate change? In that case, please add an NFC tag to the subject - if not, please explain (and test) the expected behaviour change.

The order of lld cmdargs would change from

lld "flags" "inputs" "/vfsoverlay:xxx"

to

lld "flags" "/vfsoverlay:xxx" "inputs"

[HaohaiWen]

Rebase

[mstorsjo]

Ok, thanks for clarifying. However I still don’t understand the “why” aspect here. You’re writing

so that lld specific flags can be append before inputs

Are you planning on adding such flags in a later patch, position dependent flags that need to be supplied before input files?

Or does the change in order of command line arguments have an effect on the behavior of the linker in this case?

[mstorsjo]

In D154176#4466186, @mstorsjo wrote:

Ok, thanks for clarifying. However I still don’t understand the “why” aspect here. You’re writing

so that lld specific flags can be append before inputs

Are you planning on adding such flags in a later patch, position dependent flags that need to be supplied before input files?

Or does the change in order of command line arguments have an effect on the behavior of the linker in this case?

Ok, on second thought, I guess that parsing the vfsoverlay option before the inputs makes it have an effect where it didn’t before. Is that right? (Most flags in lld aren’t very order dependent since it just checks for any occurrence of a flag anywhere among the arguments.)

[HaohaiWen]

In D154176#4466190, @mstorsjo wrote:

In D154176#4466186, @mstorsjo wrote:

Ok, thanks for clarifying. However I still don’t understand the “why” aspect here. You’re writing

so that lld specific flags can be append before inputs

Are you planning on adding such flags in a later patch, position dependent flags that need to be supplied before input files?

Or does the change in order of command line arguments have an effect on the behavior of the linker in this case?

Ok, on second thought, I guess that parsing the vfsoverlay option before the inputs makes it have an effect where it didn’t before. Is that right? (Most flags in lld aren’t very order dependent since it just checks for any occurrence of a flag anywhere among the arguments.)

I'm planing to add /dwodir to linker when user specify -gsplit-dwarf and using lld and lto.
The order does not matter.
I just want it looks more consistent and elegent since most flags are before inputs so that user can easily look for inputs when using -###.

[mstorsjo]

In D154176#4466230, @HaohaiWen wrote:

In D154176#4466190, @mstorsjo wrote:

In D154176#4466186, @mstorsjo wrote:

Ok, thanks for clarifying. However I still don’t understand the “why” aspect here. You’re writing

so that lld specific flags can be append before inputs

Are you planning on adding such flags in a later patch, position dependent flags that need to be supplied before input files?

Or does the change in order of command line arguments have an effect on the behavior of the linker in this case?

Ok, on second thought, I guess that parsing the vfsoverlay option before the inputs makes it have an effect where it didn’t before. Is that right? (Most flags in lld aren’t very order dependent since it just checks for any occurrence of a flag anywhere among the arguments.)

I'm planing to add /dwodir to linker when user specify -gsplit-dwarf and using lld and lto.
The order does not matter.

Ok, thanks for clarifying that - since I didn't quite understand how this change would be needed for that option.

I just want it looks more consistent and elegent since most flags are before inputs so that user can easily look for inputs when using -###.

Ok, that sounds reasonable. Please clarify that intent in the commit message, and this seems ok to me.

clang/lib/Driver/ToolChains/MSVC.cpp

void visualstudio::Linker::ConstructJob(Compilation &C, const JobAction &JA,
   }
 
   // Add compiler-rt lib in case if it was explicitly
   // specified as an argument for --rtlib option.
   if (!Args.hasArg(options::OPT_nostdlib)) {
     AddRunTimeLibs(TC, TC.getDriver(), CmdArgs, Args);
   }
 
+  StringRef Linker =
+      Args.getLastArgValue(options::OPT_fuse_ld_EQ, CLANG_DEFAULT_LINKER);
+  if (Linker.empty())
+    Linker = "link";
+  // We need to translate 'lld' into 'lld-link'.
+  else if (Linker.equals_insensitive("lld"))
+    Linker = "lld-link";
+
+  if (Linker == "lld-link")
+    for (Arg *A : Args.filtered(options::OPT_vfsoverlay))
+      CmdArgs.push_back(
+          Args.MakeArgString(std::string("/vfsoverlay:") + A->getValue()));
+
   // Add filenames, libraries, and other linker inputs.
   for (const auto &Input : Inputs) {
     if (Input.isFilename()) {
       CmdArgs.push_back(Input.getFilename());
       continue;
     }
 
     const Arg &A = Input.getInputArg();
void visualstudio::Linker::ConstructJob(Compilation &C, const JobAction &JA,
   }
 
   addHIPRuntimeLibArgs(TC, Args, CmdArgs);
 
   TC.addProfileRTLibs(Args, CmdArgs);
 
   std::vector<const char *> Environment;
 
-  // We need to special case some linker paths.  In the case of lld, we need to
-  // translate 'lld' into 'lld-link', and in the case of the regular msvc
+  // We need to special case some linker paths. In the case of the regular msvc
   // linker, we need to use a special search algorithm.
   llvm::SmallString<128> linkPath;
-  StringRef Linker
-    = Args.getLastArgValue(options::OPT_fuse_ld_EQ, CLANG_DEFAULT_LINKER);
-  if (Linker.empty())
-    Linker = "link";
-  if (Linker.equals_insensitive("lld"))
-    Linker = "lld-link";
-
-  if (Linker == "lld-link")
-    for (Arg *A : Args.filtered(options::OPT_vfsoverlay))
-      CmdArgs.push_back(
-          Args.MakeArgString(std::string("/vfsoverlay:") + A->getValue()));
-
   if (Linker.equals_insensitive("link")) {
     // If we're using the MSVC linker, it's not sufficient to just use link
     // from the program PATH, because other environments like GnuWin32 install
     // their own link.exe which may come first.
     linkPath = FindVisualStudioExecutable(TC, "link.exe");
 
     if (!TC.FoundMSVCInstall() && !canExecute(TC.getVFS(), linkPath)) {
       llvm::SmallString<128> ClPath;

clang/test/Driver/msvc-link.c

 // NOREPRO: "-out:msvc-link.exe"
 // NOREPRO: "-nologo"
 // NOREPRO-NOT: "-Brepro"
 
 // RUN: %clang_cl -fuse-ld=lld --vfsoverlay %s -### -- %s 2>&1 | FileCheck --check-prefix=VFSOVERLAY %s
 // VFSOVERLAY: -cc1"
 // VFSOVERLAY: "--vfsoverlay"
 // VFSOVERLAY: lld-link
-// VFSOVERLAY: "/vfsoverlay:
+// VFSOVERLAY: "/vfsoverlay:{{.*}}" "{{.*}}.obj"