[lld-macho] Replace LC_LINKER_OPTION parsing

[keith]

Summary

This removes the tablegen based parsing of LC_LINKER_OPTION since it can
only actually contain a very small number of potential arguments. In our
project with tablegen this took 5 seconds before.

This replaces https://reviews.llvm.org/D113075

[int3]

lld/MachO/Driver.cpp

399–400

Can we include in the comment an explanation of why we're not using OptTable? Thanks!

[int3]

lld/MachO/Driver.cpp

410–411

wait a minute... shouldn't this be continue, or the next line changed to else if?

I'd thought we would have tests to catch this :/

412–413

[keith]

lld/MachO/Driver.cpp

410–411

Since this parses a single option at a time shouldn't breaking be ok? I guess I'm not sure if the format intends to support -lfoo -lbar in a single LC_LINKER_OPTION or not?

[keith]

Update for feedback

lld/MachO/Driver.cpp

412–413

awesome thanks!

[int3]

lld/MachO/Driver.cpp

410–411

Oh I see... we have one LC_LINKER_OPTION per -lfoo and -framework Foo.

Doesn't that mean the for loop is redundant, since we'll only have 1 or 2 elements in argv?

[keith]

Remove loop

[keith]

lld/MachO/Driver.cpp

410–411

So I wasn't sure about that given the way argv is setup and explicitly defined as a size of 4. Pushed an option without that which still passes the current tests.

[int3]

lgtm!

lld/MachO/Driver.cpp

if (auto *dylibFile = dyn_cast_or_null<DylibFile>(
       }
     }
     return;
   }
   error("framework not found for -framework " + name);
 }
 
 // Parses LC_LINKER_OPTION contents, which can add additional command line
-// flags.
+// flags. This directly parses the flags instead of using the standard argument
+// parser to improve performance.
 void macho::parseLCLinkerOption(InputFile *f, unsigned argc, StringRef data) {
-  SmallVector<const char *, 4> argv;
+  SmallVector<StringRef, 4> argv;
   size_t offset = 0;
   for (unsigned i = 0; i < argc && offset < data.size(); ++i) {
     argv.push_back(data.data() + offset);
     offset += strlen(data.data() + offset) + 1;
   }
   if (argv.size() != argc || offset > data.size())
     fatal(toString(f) + ": invalid LC_LINKER_OPTION");
 
-  MachOOptTable table;
-  unsigned missingIndex, missingCount;
-  InputArgList args = table.ParseArgs(argv, missingIndex, missingCount);
-  if (missingCount)
-    fatal(Twine(args.getArgString(missingIndex)) + ": missing argument");
-  for (const Arg *arg : args.filtered(OPT_UNKNOWN))
-    error("unknown argument: " + arg->getAsString(args));
-
-  for (const Arg *arg : args) {
-    switch (arg->getOption().getID()) {
-    case OPT_l: {
-      StringRef name = arg->getValue();
-      ForceLoad forceLoadArchive =
-          config->forceLoadSwift && name.startswith("swift") ? ForceLoad::Yes
-                                                             : ForceLoad::No;
-      addLibrary(name, /*isNeeded=*/false, /*isWeak=*/false,
-                 /*isReexport=*/false, /*isExplicit=*/false, forceLoadArchive);
-      break;
-    }
-    case OPT_framework:
-      addFramework(arg->getValue(), /*isNeeded=*/false, /*isWeak=*/false,
-                   /*isReexport=*/false, /*isExplicit=*/false, ForceLoad::No);
-      break;
-    default:
-      error(arg->getSpelling() + " is not allowed in LC_LINKER_OPTION");
-    }
+  unsigned i = 0;
+  StringRef arg = argv[i];
+  if (arg.consume_front("-l")) {
+        config->forceLoadSwift && arg.startswith("swift") ? ForceLoad::Yes
+    addLibrary(arg, /*isNeeded=*/false, /*isWeak=*/false,
+  } else if (arg == "-framework") {
+    StringRef name = argv[++i];
+    addFramework(name, /*isNeeded=*/false, /*isWeak=*/false,
+  } else {
+    error(arg + " is not allowed in LC_LINKER_OPTION");
   }
 }
 
 static void addFileList(StringRef path) {
   Optional<MemoryBufferRef> buffer = readFile(path);
   if (!buffer)
     return;
   MemoryBufferRef mbref = *buffer;