This is an archive of the discontinued LLVM Phabricator instance.

[WebAssembly] Remove InputChunk from Symbol baseclass. NFC.
ClosedPublic

Authored by sbc100 on Feb 19 2018, 4:41 PM.

Details

Summary

Instead include InputFuction and InputSegment directly
in the subclasses that use them (DefinedFunction and
DefinedGlobal).

Diff Detail

Repository
rL LLVM

Event Timeline

sbc100 created this revision.Feb 19 2018, 4:41 PM
sbc100 retitled this revision from [WebAssembly] Remove InputChunk from Symbol baseclass to [WebAssembly] Remove InputChunk from Symbol baseclass. NFC..Feb 19 2018, 4:41 PM
sbc100 added a reviewer: ruiu.Feb 19 2018, 4:42 PM
ruiu added inline comments.Feb 19 2018, 8:34 PM
wasm/Symbols.cpp
33 ↗(On Diff #134990)

Newline before return.

39 ↗(On Diff #134990)

Ditto

wasm/Symbols.h
106 ↗(On Diff #134990)

I think Type is always given.

wasm/Writer.cpp
665 ↗(On Diff #134990)

I would recommend removing Symbol::isLive and use getChunk() and isLive() directly instead. It is because the notion of dead/alive is about chunks (or sections), and symbols are not directly considered as dead/alive. You could think that symbols are dead if they point to dead sections, and that makes some sense, but I think that not adding isLive() to Symbol makes things easier to understand.

sbc100 updated this revision to Diff 135080.Feb 20 2018, 9:39 AM
sbc100 marked 3 inline comments as done.
  • feedback
wasm/Writer.cpp
665 ↗(On Diff #134990)

OK. reverting for now.

I was looking at how COFF and ELF does. COFF has an isLive() method on the Symbol which works like this one, and ELF has Used bit on the symbol which seems to mean Live where its used.

We might want to unify these going forward, but keeping this change small.

ruiu accepted this revision.Feb 20 2018, 9:44 AM

LGTM

wasm/Symbols.h
125 ↗(On Diff #135080)

Style error

This revision is now accepted and ready to land.Feb 20 2018, 9:44 AM
sbc100 updated this revision to Diff 135083.Feb 20 2018, 9:46 AM
  • feedback
  • clang-format
This revision was automatically updated to reflect the committed changes.
This revision was automatically updated to reflect the committed changes.