Skip to content

Commit

Permalink
[WebAssembly] ensure .functype applies to right label in assembler
Browse files Browse the repository at this point in the history
We used to require .functype immediately follows the label it sets the type of, but not all Clang output follows this rule.

Now we simply allow it on any symbol, but only assume its a function start for a defined symbol, which is simpler and more general.

Fixes (part of) https://bugs.llvm.org/show_bug.cgi?id=49036

Differential Revision: https://reviews.llvm.org/D96165
  • Loading branch information
aardappel committed Feb 5, 2021
1 parent 28c6b1e commit a872ee2
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 16 deletions.
15 changes: 3 additions & 12 deletions llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,6 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
// guarantee that correct order.
enum ParserState {
FileStart,
Label,
FunctionStart,
FunctionLocals,
Instructions,
Expand All @@ -223,9 +222,6 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
};
std::vector<NestingType> NestingStack;

// We track this to see if a .functype following a label is the same,
// as this is how we recognize the start of a function.
MCSymbol *LastLabel = nullptr;
MCSymbol *LastFunctionLabel = nullptr;

public:
Expand Down Expand Up @@ -687,11 +683,6 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
return false;
}

void onLabelParsed(MCSymbol *Symbol) override {
LastLabel = Symbol;
CurrentState = Label;
}

bool parseSignature(wasm::WasmSignature *Signature) {
if (expect(AsmToken::LParen, "("))
return true;
Expand Down Expand Up @@ -808,12 +799,12 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
if (SymName.empty())
return true;
auto WasmSym = cast<MCSymbolWasm>(Ctx.getOrCreateSymbol(SymName));
if (CurrentState == Label && WasmSym == LastLabel) {
if (WasmSym->isDefined()) {
// This .functype indicates a start of a function.
if (ensureEmptyNestingStack())
return true;
CurrentState = FunctionStart;
LastFunctionLabel = LastLabel;
LastFunctionLabel = WasmSym;
push(Function);
}
auto Signature = std::make_unique<wasm::WasmSignature>();
Expand Down Expand Up @@ -881,7 +872,7 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {

if (DirectiveID.getString() == ".local") {
if (CurrentState != FunctionStart)
return error(".local directive should follow the start of a function",
return error(".local directive should follow the start of a function: ",
Lexer.getTok());
SmallVector<wasm::ValType, 4> Locals;
if (parseRegTypeList(Locals))
Expand Down
11 changes: 7 additions & 4 deletions llvm/test/MC/WebAssembly/basic-assembly.s
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ empty_func:
end_function

test0:
# local labels can appear between label and its .functype.
.Ltest0begin:
# Test all types:
.functype test0 (i32, i64) -> (i32)
.eventtype __cpp_exception i32
Expand Down Expand Up @@ -116,17 +118,18 @@ test0:
.globaltype __stack_pointer, i32

.tabletype empty_eref_table, externref
empty_eref_table:
empty_eref_table:

.tabletype empty_fref_table, funcref
empty_fref_table:
empty_fref_table:



# CHECK: .text
# CHECK-LABEL: empty_func:
# CHECK-NEXT: .functype empty_func () -> ()
# CHECK-NEXT: end_function
# CHECK-LABEL: test0:
# CHECK-NEXT: .Ltest0begin:
# CHECK-NEXT: .functype test0 (i32, i64) -> (i32)
# CHECK-NEXT: .eventtype __cpp_exception i32
# CHECK-NEXT: .local f32, f64
Expand Down Expand Up @@ -226,6 +229,6 @@ empty_fref_table:

# CHECK: .tabletype empty_eref_table, externref
# CHECK-NEXT: empty_eref_table:

# CHECK: .tabletype empty_fref_table, funcref
# CHECK-NEXT: empty_fref_table:

0 comments on commit a872ee2

Please sign in to comment.