[PATCH] D91870: [WebAssembly] Add support for table linking to wasm-ld

[Andy Wingo via Phabricator via llvm-commits]

wingo marked 4 inline comments as done.
wingo added a comment.

New patch addresses micro-feedback, but still thinking if a patch split is possible.  Probably also want to land a precursor also to ensure that the compiler produces __indirect_function_table imports for object files with call_indirect but no TABLE_INDEX relocs.



================
Comment at: lld/wasm/Symbols.cpp:19
 #include "lld/Common/ErrorHandler.h"
+#include "lld/Common/Memory.h"
 #include "lld/Common/Strings.h"
----------------
sbc100 wrote:
> Why this new include?
We can't fit a WasmTableType in a TableSymbol without growing the symbol size, so instead TableSymbol has a `WasmTableType*`, with the precondition that the symbol's lifespan is a subset of the `WasmTableType`'s lifespan.

But in `TableSymbol::setLimits`, we can't just fiddle the limits in place, because they are embedded in a `WasmTableType` that doesn't belong to the symbol; we need to allocate a fresh table type in the arena.  So `Memory.h` allow us to allocate a new `WasmTableType` using `make<>`.

Did I get the memory management right?  Feedback very welcome.


================
Comment at: lld/wasm/Writer.cpp:787
+    symtab->indirectFunctionTableSymbol->setLimits(limits);
+  }
+
----------------
sbc100 wrote:
> This last seems a little out of place since its not assigning indexes.     Would this make more sense alongside wherever `out.elemSec` is populated once we know its complete?
Sure.  I tried initially putting it in `TableSection::finalizeContents` but of course this symbol might be imported, so I instead arranged to call it just after populating elemSec.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91870/new/

https://reviews.llvm.org/D91870