[PATCH] D91637: [WebAssembly] Only emit indirect function table import if needed

Sam Clegg via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 19 00:16:37 PST 2020


sbc100 added subscribers: tlively, aardappel.
sbc100 added a comment.

In D91637#2404799 <https://reviews.llvm.org/D91637#2404799>, @wingo wrote:

> In D91637#2400474 <https://reviews.llvm.org/D91637#2400474>, @sbc100 wrote:
>
>> What I was hoping for here is minimal change to the object writer to remove the table from object files that don't reference it.  This seem separable from the question of whether the indirection function table should be first class MC symbol .. although maybe I'm taking this CL splitting thing to far.
>
> Just backing up a bit for context -- the end goal to add support to wasm-ld for linking object files that reference tables; pruning needless `__indirect_function_table` imports is a "side quest".  To link tables, I need relocs for all table references written to object files, and linker symtab entries for all tables.  Do you agree that a first-class `MCSymbol` is a part of that final pipeline?  If so, I think we're on the good path here, and that using a different mechanism would just require more testing when we put the MCSymbol mechanism in place.  Anyway, just a gentle argument that it seems to me that this is about the right patch size, though whether we have the symbol creation in MCContext is another question of course.

Yes, I was thinking we could just land the side quest (I like that term!) quickly without too much discussion.   If you want to include the MCSymbol creation in addtion then I'm not averse to that.. but as you say, that means we have to figure if the target-independent MCContext is the right place for the two new function (`getOrCreateWasmFunctionTableSymbol`).  My gut tells me it not, but I'm also no an expert in these areas... @tlively @aardappel might have some idea where the right place for this kind of thing is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91637



More information about the llvm-commits mailing list