This is an archive of the discontinued LLVM Phabricator instance.

[WebAssembly][NFC] Remove WebAssemblyStackifier TableGen backend
ClosedPublic

Authored by tlively on Oct 15 2018, 4:56 PM.

Details

Summary

Replace its functionality with a TableGen InstrInfo relational
instruction mapping. Although arguably more complex than the TableGen
backend, the relational mapping is a smaller maintenance burden than a
TableGen backend.

Diff Detail

Repository
rL LLVM

Event Timeline

tlively created this revision.Oct 15 2018, 4:56 PM
tlively updated this revision to Diff 169884.Oct 16 2018, 1:33 PM
  • Add BaseName to BR_TABLE instructions and add assert
nhaehnle removed a subscriber: nhaehnle.Oct 17 2018, 3:19 AM

Hah, generally looks even neater :)

lib/Target/WebAssembly/WebAssemblyInstrFormats.td
21 ↗(On Diff #169884)

Why does this need to be a string instead of a bit?

aheejin added inline comments.Oct 17 2018, 4:17 PM
lib/Transforms/IPO/HotColdSplitting.cpp
153 ↗(On Diff #169884)

I think this change has been committed, so you can remove this change

This looks neat!

dschuff added inline comments.Oct 19 2018, 10:44 AM
lib/Target/WebAssembly/WebAssemblyMCInstLower.cpp
36 ↗(On Diff #169884)

Probably should #undef GET_INSTRMAP_INFO afterwards just for completeness.

tlively updated this revision to Diff 170222.Oct 19 2018, 11:25 AM
tlively marked an inline comment as done.
  • Rebase
tlively added inline comments.Oct 19 2018, 11:25 AM
lib/Target/WebAssembly/WebAssemblyInstrFormats.td
21 ↗(On Diff #169884)

I specified that the values of the StackBased form the columns of the relational mapping. Internally, TableGen will uses these values to form identifiers in an enum. I guess since the values are used as part of identifiers, TableGen requires them to be strings. It seemed better to make this field a string than to introduce a redundant string field that contains the same information.

lib/Target/WebAssembly/WebAssemblyMCInstLower.cpp
36 ↗(On Diff #169884)

This is done automatically inside WebAssemblyGenInstrInfo.inc. Other includes of TableGen files do not have an undef either.

lib/Transforms/IPO/HotColdSplitting.cpp
153 ↗(On Diff #169884)

Oops! I didn't mean to get this in here.

aheejin accepted this revision.Oct 22 2018, 11:21 AM

Thanks!

utils/TableGen/WebAssemblyDisassemblerEmitter.cpp
51 ↗(On Diff #170222)

This looks a bit arcane.. but it's pre-existing, so fine I guess.

This revision is now accepted and ready to land.Oct 22 2018, 11:21 AM

Oh, and maybe it's better to mark this as NFC.

aardappel accepted this revision.Oct 22 2018, 2:30 PM
aardappel added inline comments.
lib/Target/WebAssembly/WebAssemblyInstrFormats.td
21 ↗(On Diff #169884)

Ah ok, thanks.

tlively retitled this revision from [WebAssembly] Remove WebAssemblyStackifier TableGen backend to [WebAssembly][NFC] Remove WebAssemblyStackifier TableGen backend.Oct 22 2018, 2:51 PM
This revision was automatically updated to reflect the committed changes.
llvm/trunk/utils/TableGen/WebAssemblyDisassemblerEmitter.cpp