[PATCH] D94075: [lld][WebAssembly] Add support for handling table symbols

[Sam Clegg via Phabricator via llvm-commits]

sbc100 added a comment.

I suppose the synthetic table is there so it can one day be used for the indirect function table?  Should we wait until then to actually add it?

It would be good to add some test cases with multi-table and input and output.  Is there a fundamental reason we can't do that yet?

Otherwise this lgtm,  everything looks pretty straight forward.



================
Comment at: lld/wasm/InputFiles.cpp:415
+    tables.emplace_back(make<InputTable>(t, this));
+  }
+
----------------
Remove curlys


================
Comment at: lld/wasm/InputTable.h:46
+
+  bool live = false;
+
----------------
I don't if its worth have a common base class for `InputGlobal`, `InputEvent`, `InputTable` and `InputChunk`.  At least they all seem to share the `file` and `live` members.    Perhaps we can do that a followup?


================
Comment at: lld/wasm/SymbolTable.cpp:273
 
+DefinedTable *SymbolTable::addSyntheticTable(StringRef name, uint32_t flags,
+                                             InputTable *table) {
----------------
I don't see any callers for this.   Can we can add it later if/when its needed? 


================
Comment at: lld/wasm/SymbolTable.h:108
   std::vector<InputGlobal *> syntheticGlobals;
+  std::vector<InputTable *> syntheticTables;
 
----------------
Unneeded?


================
Comment at: lld/wasm/SyntheticSections.h:156
+    // unless it is imported, we need a table section.  FIXME: Treat
+    // __indirect_function_table as a normal symbol, and only residualize a
+    // table section as needed.
----------------
residualize?   I've not heard that expression before.    Is it like realize?  Or reify?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94075