This is an archive of the discontinued LLVM Phabricator instance.

[WebAssembly] Made disassembler only use stack instructions.
ClosedPublic

Authored by aardappel on Aug 27 2018, 2:00 PM.

Details

Summary

Now uses the StackBased bit from the tablegen defs to identify
stack instructions (and ignore register based or non-wasm instructions).

Also changed how we store operands, since we now have up to 16 of them
per instruction. To not cause static data bloat, these are compressed
into a tiny table.

+ a few other cleanups.

Tested:

  • MCTest
  • llvm-lit -v find test -name WebAssembly

Diff Detail

Repository
rL LLVM

Event Timeline

aardappel created this revision.Aug 27 2018, 2:00 PM
utils/TableGen/WebAssemblyDisassemblerEmitter.cpp
94 ↗(On Diff #162744)

Could flip these:

if (CurOperandList.size() < OperandTable.size()) {
  for (size_t J = 0; J < OperandTable.size() - CurOperandList.size(); ++J) {

which is iotally more efficient. Probably more readable the way you have it though. Was thinking we could omit the if entirely, but size isn't signed.

96 ↗(On Diff #162744)

Oh no, goto
This one's not too dreadful, but I think this is even easier:

size_t K;
for (K = 0; K < CurOperandList.size(); ++K) {
  if (OperandTable[J + K] != CurOperandList[K])
    break;
}
if (K == CurOperandList.size()) {
  OperandStart = J;
  break;
}
aardappel updated this revision to Diff 162763.Aug 27 2018, 3:31 PM
aardappel marked 2 inline comments as done.

Addressed review comments.

aardappel added inline comments.Aug 27 2018, 3:31 PM
utils/TableGen/WebAssemblyDisassemblerEmitter.cpp
94 ↗(On Diff #162744)

<=, but yes, that's better :)

tlively added inline comments.Aug 27 2018, 3:34 PM
utils/TableGen/WebAssemblyDisassemblerEmitter.cpp
47 ↗(On Diff #162744)

Def.getValueAsBit("StackBased") might be cleaner here. It assumes "StackBased" is present, but so does the current code.

aardappel added inline comments.Aug 27 2018, 3:37 PM
utils/TableGen/WebAssemblyDisassemblerEmitter.cpp
47 ↗(On Diff #162744)

Yes, I thought about using that, but would prefer it not to be throwing exceptions if we ever add instructions that don't have this field set.

dschuff accepted this revision.Aug 28 2018, 10:39 AM
This revision is now accepted and ready to land.Aug 28 2018, 10:39 AM
This revision was automatically updated to reflect the committed changes.