Page MenuHomePhabricator

[WebAssembly][MC] Record limit constraints for table sizes
ClosedPublic

Authored by wingo on Mar 23 2021, 8:22 AM.

Details

Summary

This commit adds a full WasmTableType to MCSymbolWasm, differing from
the current situation (just an ElemType) in that it additionally records
a WasmLimits.

We add support for specifying the limits in .S files also, via the
following syntax variations:

.tabletype SYM, ELEMTYPE
.tabletype SYM, ELEMTYPE, MINSIZE
.tabletype SYM, ELEMTYPE, MINSIZE, MAXSIZE

Depends on D99186.

Diff Detail

Event Timeline

wingo created this revision.Mar 23 2021, 8:22 AM
wingo requested review of this revision.Mar 23 2021, 8:22 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 23 2021, 8:22 AM
wingo added 1 blocking reviewer(s): sbc100.Mar 23 2021, 8:22 AM
sbc100 accepted this revision.Mar 23 2021, 9:27 AM

Great!

llvm/include/llvm/MC/MCSymbolWasm.h
111

Can you use WASM_TYPE_FUNCREF and avoid that cast?

llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
866–867

Using auto all over the place makes this harder to review and goes against llvm policy. I guess its pre-existing code so you don't need to change it here.. but I wish it this had a type name

872–873

All these casts make me this that WasmTableType should have ValType rather than uint8_t

874

make this into single condition (with no curly braces)?

This revision is now accepted and ready to land.Mar 23 2021, 9:27 AM
wingo marked 3 inline comments as done.Mar 24 2021, 1:38 AM
wingo added inline comments.
llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
866–867

Yeah I sympathize. I'll add an explicit type for ElemType here.

872–873

Yeah. Will need to change in any case once we get typed function references. There are some related cases that should probably change at the same time, like BinaryFormat/Wasm.h:WasmGlobalType::Type.

wingo updated this revision to Diff 332894.Mar 24 2021, 1:44 AM
wingo marked an inline comment as done.

Address feedback

This revision was landed with ongoing or failed builds.Mar 24 2021, 1:47 AM
This revision was automatically updated to reflect the committed changes.