This is an archive of the discontinued LLVM Phabricator instance.

[WebAssembly] replaced .param/.result by .functype
ClosedPublic

Authored by aardappel on Nov 16 2018, 2:36 PM.

Details

Summary

This makes it easier/cleaner to generate a single signature from
this directive. Also:

  • Adds the symbol name, such that we don't depend on the location of this directive anymore.
  • Actually constructs the signature in the assembler, and make the assembler own it.
  • Refactor the use of MVT vs ValType in the streamer and assembler to require less conversions overall.
  • Changed 700 or so tests to use it.

Diff Detail

Event Timeline

aardappel created this revision.Nov 16 2018, 2:36 PM
sbc100 accepted this revision.Nov 16 2018, 2:56 PM

Nice! I wish we could do with without the lexer change and with a more asm-linux syntax.. but I guess that time for bikeshedding that is over.

lib/MC/MCParser/AsmLexer.cpp
653

Is there a risk we could break other asm backends with this?

This revision is now accepted and ready to land.Nov 16 2018, 2:56 PM
aardappel marked an inline comment as done.Nov 16 2018, 3:13 PM
aardappel added inline comments.
lib/MC/MCParser/AsmLexer.cpp
653

If there's other backend that allow parsing a - token followed by > token with no whitespace in between, then we will break them, yes. I think this is highly unlikely, as I don't know of any kind of expression where that could occur.

dschuff accepted this revision.Nov 16 2018, 4:11 PM

otherwise LGTM!

lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
189

at least the operator == overload on StringRef makes this code not as bad as it could be?

377

why set and then reset the symbol type? all of the intervening returns are error conditions, right?

aardappel marked 3 inline comments as done.Nov 16 2018, 5:17 PM
aardappel added inline comments.
lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
189

Yup, it's ok :)

377

That looks like a refactoring slip-up, will remove.

aardappel updated this revision to Diff 174488.Nov 16 2018, 5:20 PM
aardappel marked 2 inline comments as done.

Code review fixes: removed redundant setType()

aardappel updated this revision to Diff 174628.Nov 19 2018, 9:12 AM

Fixes because of just introduced WebAssemblyTargetNullStreamer class.

This revision was automatically updated to reflect the committed changes.