This is an archive of the discontinued LLVM Phabricator instance.

[WebAssembly] Simplify use of InputChunk within Symbols. NFC.
ClosedPublic

Authored by sbc100 on Jan 28 2018, 11:34 AM.

Details

Summary

Simplify the code so that we can use in InputChunk pointer
rather than an InputFunction + InputSegment depending
on the symbol type.

Use llvm dynamic casts when we need a particular subtype.

This change is useful for the upcoming gc-section support
which wants to deal with all input chunks in the same way.

Event Timeline

sbc100 created this revision.Jan 28 2018, 11:34 AM
sbc100 updated this revision to Diff 131714.Jan 28 2018, 11:37 AM
  • clang format
sbc100 edited the summary of this revision. (Show Details)Jan 28 2018, 11:39 AM
sbc100 retitled this revision from [WebAssemly] Associate symbol with InputChunk in which they are defined. NFC. to [WebAssemly] Simplify use of InputChunk within Symbols. NFC..
sbc100 edited the summary of this revision. (Show Details)
sbc100 added reviewers: ruiu, ncw.
This revision was not accepted when it landed; it landed in state Needs Review.Jan 28 2018, 11:59 AM
This revision was automatically updated to reflect the committed changes.

Sorry, I didn't mean to commit this without lgtm. I made a git-svn mistake. Please review and I will revert or update as needed.

sbc100 retitled this revision from [WebAssemly] Simplify use of InputChunk within Symbols. NFC. to [WebAssembly] Simplify use of InputChunk within Symbols. NFC..Jan 28 2018, 12:03 PM
ncw added a comment.EditedJan 29 2018, 5:18 AM

LGTM! I'm happy - I even added InputGlobal as a third type, which makes it even uglier, so this looks good. I even added a Symbol::getChunk method that's similar to yours (but not as tidy).

Rebasing my patches shouldn't be a problem. One thing I did play around with was trying to store a pointer to the Symbol inside the InputChunk - but I quickly abandoned that, it won't work (since some chunks don't have an associated symbol, and some chunks have several associated symbols, and symbols can be unassociated from chunks when weak symbols are overridden...). Just an FYI in case you're ever tempted to try the same thing! Symbol ➝ chunk is good, chunk ➝ symbol is bad.

ruiu added a comment.Feb 1 2018, 1:39 PM

LGTM

wasm/SymbolTable.cpp
124

nit: WasmSignature* Sig -> WasmSignature *Sig

wasm/Symbols.cpp
46–47

auto* -> auto *.