[PATCH] D96001: [WebAssembly][lld] Preassign table number 0 to indirect function table for MVP inputs
Sam Clegg via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Feb 11 13:58:28 PST 2021
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
More information about the llvm-commits
mailing list