[PATCH] D96001: [WebAssembly][lld] Preassign table number 0 to indirect function table for MVP inputs

[Andy Wingo via Phabricator via llvm-commits]

wingo marked 2 inline comments as done.
wingo added inline comments.


================
Comment at: lld/wasm/Driver.cpp:804
+  if (existing && existing->hasPreassignedTableNumberZero())
+    sym->preassignTableNumberZero();
   return sym;
----------------
sbc100 wrote:
> Can these two lines be moved into `SymbolTable::addSyntheticTable` (which already has a check for an existing symbol).   Then you don't need to pass the extra `existing` arg here, and you don't need to extra `existingTable` table below?
> 
> hmm.. maybe that would make addSyntheticTable a little less tidy, but it seems like logical place... either that or replaceSymbol itself. 
> 
> In general this notion of preassigned index seems to complicate things in a way that I'm not a fan of.    I wish we could find a simpler way to force the indirect table to always have index 0.
Right, the whole notion of symbols with preassigned addresses (table numbers in this case) is... unsatisfying.  But it does seem to be essential to the constraints we're operating under.  I thought that keeping the bits that much about with preassigned addresses local to the parts that deal in tables (and specifically the indirect function table) would limit the "taint".

Probably replaceSymbol is not the place for this to go, as other kinds of symbols rightly don't have a notion of preassigned addresses.

Incidentally, looking again, I don't think the code as it is works, because of the placement new in replaceSymbol :/  Will fix, I guess in addSyntheticTable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96001