[COFF] Do not parse args twice if no rsp files exists

[takuto.ikuta]

Summary

This patch reduces link time of chromium's blink_core.dll in component build.
Total size of input argument in .directives become nearly 300MB in the build and no rsp file is used.
Speedup link by skipping duplicate parsing.

On my desktop machine, 4 times stats are like below. Improved around 15%.

This patch

TotalSeconds : 18.408538
TotalSeconds : 17.2996744
TotalSeconds : 17.1053862
TotalSeconds : 17.809777
avg: 17.6558439

master

TotalSeconds : 20.9290504
TotalSeconds : 19.9158213
TotalSeconds : 21.0643515
TotalSeconds : 20.8775831
avg: 20.696701575

[ruiu]

Thank you for doing this.

A command line string and .drctive section contents is pretty different. The former may have response files, but the latter shouldn't. The performance of parsing a command line string is not important, so we can always parse the same command line twice.

I'd define a new function, ArgParser::parseDirectives next to ArgParser::parse to parse directives section contents. That should make code cleaner.

[takuto.ikuta]

Thank you for review.
I made parseDirectives in ArgParser and extend parse() to receive flag for rsp file instead of copying.
How do you think?

[ruiu]

I still think you should split the function into two. The boolean flag doesn't look beautiful.

lld/COFF/DriverUtils.cpp

737–743

You don't need this for the directive section because the condition will never be true.

746

This is not appropriate for the directive section too.

[takuto.ikuta]

Sure. Made parseDirectives removing unnecessary codes. Thanks.

[ruiu]

lld/COFF/Driver.h

67–69

Defining two functions for one feature doesn't make much sense. This forwarder function doesn't do anything other than calling tokenize(). Please move that piece of code to the other function and merge the two.

[takuto.ikuta]

Merged functions.

[ruiu]

LGTM

[takuto.ikuta]

Thank you for review.

I don't have commit access.
Rui-san, can I ask you to merge this patch?

[takuto.ikuta]

Thank you!

[thakis]

(to keep things linked up, this broke building chromium, fixed by inglorion in r321512)

https://bugs.llvm.org/show_bug.cgi?id=35762)

lld/COFF/Driver.h

 };
 
 class ArgParser {
 public:
   // Concatenate LINK environment variable and given arguments and parse them.
   llvm::opt::InputArgList parseLINK(std::vector<const char *> Args);
 
   // Tokenizes a given string and then parses as command line options.
-  llvm::opt::InputArgList parse(StringRef S) { return parse(tokenize(S)); }
+  llvm::opt::InputArgList parse(StringRef S) {
+    return parse(tokenize(S), true);
+  }
+
+  // Tokenizes a given string and then parses as command line options in
+  // .drectve section.
+  llvm::opt::InputArgList parseDirectives(StringRef S) {
+    return parse(tokenize(S), false);
+  }
 
 private:
   // Parses command line options.
-  llvm::opt::InputArgList parse(llvm::ArrayRef<const char *> Args);
+  llvm::opt::InputArgList parse(llvm::ArrayRef<const char *> Args, bool ExpandRsp);
 
   std::vector<const char *> tokenize(StringRef S);
 
   COFFOptTable Table;
 };
 
 class LinkerDriver {
 public:
   void link(llvm::ArrayRef<const char *> Args);
 

lld/COFF/Driver.cpp

return Sym.startswith("@") || Sym.contains("@@") || Sym.startswith("?") ||
          (!Config->MinGW && Sym.contains('@'));
 }
 
 // Parses .drectve section contents and returns a list of files
 // specified by /defaultlib.
 void LinkerDriver::parseDirectives(StringRef S) {
   ArgParser Parser;
   // .drectve is always tokenized using Windows shell rules.
-  opt::InputArgList Args = Parser.parse(S);
+  opt::InputArgList Args = Parser.parseDirectives(S);
 
   for (auto *Arg : Args) {
     switch (Arg->getOption().getUnaliasedOption().getID()) {
     case OPT_aligncomm:
       parseAligncomm(Arg->getValue());
       break;
     case OPT_alternatename:
       parseAlternateName(Arg->getValue());

lld/COFF/DriverUtils.cpp

if (S == "windows")
       return cl::TokenizeWindowsCommandLine;
     return cl::TokenizeGNUCommandLine;
   }
   // The COFF linker always defaults to Windows quoting.
   return cl::TokenizeWindowsCommandLine;
 }
 
 // Parses a given list of options.
-opt::InputArgList ArgParser::parse(ArrayRef<const char *> Argv) {
+opt::InputArgList ArgParser::parse(ArrayRef<const char *> Argv, bool ExpandRsp) {
   // Make InputArgList from string vectors.
   unsigned MissingIndex;
   unsigned MissingCount;
   SmallVector<const char *, 256> Vec(Argv.data(), Argv.data() + Argv.size());
 
   // We need to get the quoting style for response files before parsing all
   // options so we parse here before and ignore all the options but
   // --rsp-quoting.
   opt::InputArgList Args = Table.ParseArgs(Vec, MissingIndex, MissingCount);
 
+  if (ExpandRsp) {
     // Expand response files (arguments in the form of @<filename>)
     // and then parse the argument again.
     cl::ExpandResponseFiles(Saver, getQuotingStyle(Args), Vec);
     Args = Table.ParseArgs(Vec, MissingIndex, MissingCount);
+  }
 
   // Print the real command line if response files are expanded.
   if (Args.hasArg(OPT_verbose) && Argv.size() != Vec.size()) {
     std::string Msg = "Command line:";
     for (const char *S : Vec)
       Msg += " " + std::string(S);
     message(Msg);
   }
 
   // Handle /WX early since it converts missing argument warnings to errors.
   errorHandler().FatalWarnings = Args.hasFlag(OPT_WX, OPT_WX_no, false);
 
   if (MissingCount)
     fatal(Twine(Args.getArgString(MissingIndex)) + ": missing argument");
   for (auto *Arg : Args.filtered(OPT_UNKNOWN))
     warn("ignoring unknown argument: " + Arg->getSpelling());
   return Args;
 }
 
 // link.exe has an interesting feature. If LINK or _LINK_ environment
 // variables exist, their contents are handled as command line strings.
 // So you can pass extra arguments using them.
 opt::InputArgList ArgParser::parseLINK(std::vector<const char *> Argv) {
   // Concatenate LINK env and command line arguments, and then parse them.
   if (Optional<std::string> S = Process::GetEnv("LINK")) {
     std::vector<const char *> V = tokenize(*S);
     Argv.insert(Argv.begin(), V.begin(), V.end());
   }
   if (Optional<std::string> S = Process::GetEnv("_LINK_")) {
     std::vector<const char *> V = tokenize(*S);
     Argv.insert(Argv.begin(), V.begin(), V.end());
   }
-  return parse(Argv);
+  return parse(Argv, true);
 }
 
 std::vector<const char *> ArgParser::tokenize(StringRef S) {
   SmallVector<const char *, 16> Tokens;
   cl::TokenizeWindowsCommandLine(S, Saver, Tokens);
   return std::vector<const char *>(Tokens.begin(), Tokens.end());
 }
 
 void printHelp(const char *Argv0) {
   COFFOptTable().PrintHelp(outs(), Argv0, "LLVM Linker", false);
 }
 
 } // namespace coff
 } // namespace lld