This is an archive of the discontinued LLVM Phabricator instance.

[lld][WebAssembly] Add support for handling table symbols
ClosedPublic

Authored by wingo on Jan 5 2021, 3:14 AM.

Details

Summary

This commit adds table symbol support in a partial way, while still
including some special cases for the __indirect_function_table symbol.
No change in tests.

Diff Detail

Event Timeline

wingo created this revision.Jan 5 2021, 3:14 AM
wingo requested review of this revision.Jan 5 2021, 3:14 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 5 2021, 3:14 AM
wingo added 1 blocking reviewer(s): sbc100.Jan 5 2021, 3:16 AM

Gentle ping to @sbc100 :)

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.

lld/wasm/InputFiles.cpp
416

Remove curlys

lld/wasm/InputTable.h
47

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?

lld/wasm/SymbolTable.cpp
273

I don't see any callers for this. Can we can add it later if/when its needed?

lld/wasm/SymbolTable.h
106

Unneeded?

lld/wasm/SyntheticSections.h
156

residualize? I've not heard that expression before. Is it like realize? Or reify?

This change does point to perhaps some refactoring that we should do at some point to try to remove some the boiler place that is common among symbol types, and input elements, as well as how the symbol are added to the symbol table.

wingo added a comment.Jan 14 2021, 1:43 AM

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?

Unfortunately things are still broken with table imports: imported table numbers start at 0, whereas at this point in the patch set there are still a few things that expect the __indirect_function_table to be 0, even if it's not imported.

wingo marked 5 inline comments as done.Jan 14 2021, 1:51 AM
wingo added inline comments.
lld/wasm/InputTable.h
47

Sure, I can take a look in a followup.

lld/wasm/SyntheticSections.h
156

I guess it's a term more from middle-ends; like a pass that does function inlining is a source-to-source transformation, and you call the output program the residual program; you residualize code to the output. Maybe it's more used in the FP world; see e.g. usage in Waddell and Dybvig's "Fast and effective procedure inlining". But that paper is 20 years old and maybe it's not a good term in this context. I'll reword.

wingo updated this revision to Diff 316592.Jan 14 2021, 1:55 AM
wingo marked 2 inline comments as done.

Address feedback

wingo added a comment.Jan 14 2021, 2:18 AM

Pushing, given the earlier LGTM, let me know if I am misinterpreting things. Cheers & thanks for the review :)

This revision was not accepted when it landed; it landed in state Needs Review.Jan 14 2021, 2:20 AM
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.