[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 ↗

(On Diff #128188)

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

746 ↗

(On Diff #128188)

This is not appropriate for the directive section too.

[takuto.ikuta]

Sure. Made parseDirectives removing unnecessary codes. Thanks.

[ruiu]

lld/COFF/Driver.h

59–61 ↗

(On Diff #128189)

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/trunk/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)); }
 
+  // Tokenizes a given string and then parses as command line options in
+  // .drectve section.
+  llvm::opt::InputArgList parseDirectives(StringRef S);
+
 private:
   // Parses command line options.
   llvm::opt::InputArgList parse(llvm::ArrayRef<const char *> Args);
 
   std::vector<const char *> tokenize(StringRef S);
 
   COFFOptTable Table;
 };

lld/trunk/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/trunk/COFF/DriverUtils.cpp

opt::InputArgList ArgParser::parse(ArrayRef<const char *> Argv) {
 
   if (MissingCount)
     fatal(Twine(Args.getArgString(MissingIndex)) + ": missing argument");
   for (auto *Arg : Args.filtered(OPT_UNKNOWN))
     warn("ignoring unknown argument: " + Arg->getSpelling());
   return Args;
 }
 
+// Tokenizes and parses a given string as command line in .drective section.
+opt::InputArgList ArgParser::parseDirectives(StringRef S) {
+  // Make InputArgList from string vectors.
+  unsigned MissingIndex;
+  unsigned MissingCount;
+
+  opt::InputArgList Args =
+      Table.ParseArgs(tokenize(S), MissingIndex, MissingCount);
+
+  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());