[PATCH] D47162: [WebAssembly] Initial support for LTO

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added inline comments.


================
Comment at: test/wasm/lto/lto-start.ll:2
+; RUN: llvm-as %s -o %t.o
+; RUN: wasm-ld %t.o -o %t.wasm
+; RUN: obj2yaml %t.wasm | FileCheck %s
----------------
More tests please. I would expect to see a ThinLTO test as well as tests for the new flags.


================
Comment at: wasm/Config.h:32-50
+  bool SaveTemps;
   bool StripAll;
   bool StripDebug;
   bool StackFirst;
   uint32_t GlobalBase;
   uint32_t InitialMemory;
   uint32_t MaxMemory;
----------------
Out of these new options it looks like you are only initializing `--lto-partitions` and `--save-temps` during option parsing. You probably want to either initialize the remaining ones or remove them.


================
Comment at: wasm/Driver.cpp:281
+static Symbol *handleUndefined(StringRef Name) {
+  Symbol *S = Symtab->addUndefinedFunction(Name, 0, nullptr, nullptr);
+
----------------
pcc wrote:
> Should we implement `-u` in the same way as in the ELF linker? With this implementation it doesn't look like it will work correctly for undefined data and globals.
Please address.


================
Comment at: wasm/InputFiles.cpp:383
+  if (ObjSym.isExecutable())
+    return Symtab->addDefinedFunction(NameRef, 0, &F, nullptr);
+  return Symtab->addDefinedData(NameRef, 0, &F, nullptr, 0, 0);
----------------
pcc wrote:
> I think you also want to copy the symbol's binding here and below.
Please address.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D47162