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

[Sam Clegg via Phabricator via llvm-commits]

sbc100 added a comment.

we when we comes to assigning indexes we do something like this:



================
Comment at: lld/wasm/Symbols.h:382
+  void preassignTableNumberZero();
+  bool hasPreassignedTableNumberZero() const;
+
----------------
I feel a little bad for pushing back more on the this change, but it still feels a little more invasive than it could be.

Rather than adding these two methods to the table class what do you think about this appoach:

- Add a global config variable that gets set if we have MVP objects in the link.   We could call this  something like`bool legacyFunctionTable = false`.   Or alternatively "forceFunctioTableToBeZero"?  Or "functionTableMustBeZero"?   I kind of like the idea of a bool here because it clearly indicates that are two different modes.
- We already track the global symbol `indirectFunctionTable`.
- Then, we when we comes to assigning indexes we do something like this:

```
   if (condig->legacyFunctionTable) {
        placeIndirectFunctionTableFirst()   <-- here we could report errors if the sorting is impossible
   }
```

This sorting and error reporting could be done in the `TableSection::assignIndexes()`.

Hopefully this should avoid adding the preassignTableNumberZero/hasPreassignedTableNumberZero methods and should avoid extra logic to too much extra logic `addTable` method.  

WDYT?

(I guess it would also need to happen in `ImportSection::assignIndexes`...)

These are mostly cosmetic changes, that actual behaviour LGTM.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96001