diff --git a/llvm/lib/MC/MCParser/WasmAsmParser.cpp b/llvm/lib/MC/MCParser/WasmAsmParser.cpp --- a/llvm/lib/MC/MCParser/WasmAsmParser.cpp +++ b/llvm/lib/MC/MCParser/WasmAsmParser.cpp @@ -90,15 +90,40 @@ return false; } - bool parseSectionFlags(StringRef FlagStr, bool &Passive) { - SmallVector Flags; - // If there are no flags, keep Flags empty - FlagStr.split(Flags, ",", -1, false); - for (auto &Flag : Flags) { - if (Flag == "passive") + bool parseSectionFlags(StringRef FlagStr, bool &Passive, bool &Group) { + for (char C : FlagStr) { + switch (C) { + case 'p': Passive = true; - else - return error("Expected section flags, instead got: ", Lexer->getTok()); + break; + case 'G': + Group = true; + break; + default: + return Parser->Error(getTok().getLoc(), + StringRef("Unexepcted section flag: ") + FlagStr); + } + } + return false; + } + + bool parseGroup(StringRef &GroupName) { + if (Lexer->isNot(AsmToken::Comma)) + return TokError("expected group name"); + Lex(); + if (Lexer->is(AsmToken::Integer)) { + GroupName = getTok().getString(); + Lex(); + } else if (Parser->parseIdentifier(GroupName)) { + return TokError("invalid group name"); + } + if (Lexer->is(AsmToken::Comma)) { + Lex(); + StringRef Linkage; + if (Parser->parseIdentifier(Linkage)) + return TokError("invalid linkage"); + if (Linkage != "comdat") + return TokError("Linkage must be 'comdat'"); } return false; } @@ -130,27 +155,34 @@ if (!Kind.hasValue()) return Parser->Error(Lexer->getLoc(), "unknown section kind: " + Name); - MCSectionWasm *Section = getContext().getWasmSection(Name, Kind.getValue()); // Update section flags if present in this .section directive bool Passive = false; - if (parseSectionFlags(getTok().getStringContents(), Passive)) + bool Group = false; + if (parseSectionFlags(getTok().getStringContents(), Passive, Group)) return true; - if (Passive) { - if (!Section->isWasmData()) - return Parser->Error(getTok().getLoc(), - "Only data sections can be passive"); - Section->setPassive(); - } - Lex(); - if (expect(AsmToken::Comma, ",") || expect(AsmToken::At, "@") || - expect(AsmToken::EndOfStatement, "eol")) + if (expect(AsmToken::Comma, ",") || expect(AsmToken::At, "@")) + return true; + + StringRef GroupName; + if (Group && parseGroup(GroupName)) + return true; + + if (expect(AsmToken::EndOfStatement, "eol")) return true; - auto WS = getContext().getWasmSection(Name, Kind.getValue()); + // TODO: Parse UniqueID + MCSectionWasm *WS = getContext().getWasmSection( + Name, Kind.getValue(), GroupName, MCContext::GenericSectionID); + if (Passive) { + if (!WS->isWasmData()) + return Parser->Error(getTok().getLoc(), + "Only data sections can be passive"); + WS->setPassive(); + } getStreamer().SwitchSection(WS); return false; } @@ -189,9 +221,13 @@ Lexer->is(AsmToken::Identifier))) return error("Expected label,@type declaration, got: ", Lexer->getTok()); auto TypeName = Lexer->getTok().getString(); - if (TypeName == "function") + if (TypeName == "function") { WasmSym->setType(wasm::WASM_SYMBOL_TYPE_FUNCTION); - else if (TypeName == "global") + auto *Current = + cast(getStreamer().getCurrentSection().first); + if (Current->getGroup()) + WasmSym->setComdat(true); + } else if (TypeName == "global") WasmSym->setType(wasm::WASM_SYMBOL_TYPE_GLOBAL); else if (TypeName == "object") WasmSym->setType(wasm::WASM_SYMBOL_TYPE_DATA); diff --git a/llvm/lib/MC/MCSectionWasm.cpp b/llvm/lib/MC/MCSectionWasm.cpp --- a/llvm/lib/MC/MCSectionWasm.cpp +++ b/llvm/lib/MC/MCSectionWasm.cpp @@ -64,7 +64,9 @@ OS << ",\""; if (IsPassive) - OS << "passive"; + OS << "p"; + if (Group) + OS << "G"; OS << '"'; @@ -78,6 +80,12 @@ // TODO: Print section type. + if (Group) { + OS << ","; + printName(OS, Group->getName()); + OS << ",comdat"; + } + if (isUnique()) OS << ",unique," << UniqueID; diff --git a/llvm/lib/MC/WasmObjectWriter.cpp b/llvm/lib/MC/WasmObjectWriter.cpp --- a/llvm/lib/MC/WasmObjectWriter.cpp +++ b/llvm/lib/MC/WasmObjectWriter.cpp @@ -1367,6 +1367,9 @@ if (Mode == DwoMode::DwoOnly && !isDwoSection(Sec)) continue; + LLVM_DEBUG(dbgs() << "Processing Section " << SectionName << " group " + << Section.getGroup() << "\n";); + // .init_array sections are handled specially elsewhere. if (SectionName.startswith(".init_array")) continue; diff --git a/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp b/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp --- a/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp +++ b/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp @@ -971,12 +971,27 @@ auto SymName = Symbol->getName(); if (SymName.startswith(".L")) return; // Local Symbol. + // Only create a new text section if we're already in one. + // TODO: If the user explicitly creates a new function section, we ignore + // its name when we create this one. It would be nice to honor their + // choice, while still ensuring that we create one if they forget. + // (that requires coordination with WasmAsmParser::parseSectionDirective) auto CWS = cast(getStreamer().getCurrentSection().first); if (!CWS || !CWS->getKind().isText()) return; auto SecName = ".text." + SymName; - auto WS = getContext().getWasmSection(SecName, SectionKind::getText()); + + auto *Group = CWS->getGroup(); + // If the current section is a COMDAT, also set the flag on the symbol. + // TODO: Currently the only place that the symbols' comdat flag matters is + // for importing comdat functions. But there's no way to specify that in + // assembly currently. + if (Group) + cast(Symbol)->setComdat(true); + auto *WS = + getContext().getWasmSection(SecName, SectionKind::getText(), Group, + MCContext::GenericSectionID, nullptr); getStreamer().SwitchSection(WS); // Also generate DWARF for this section if requested. if (getContext().getGenDwarfForAssembly()) diff --git a/llvm/test/MC/WebAssembly/comdat-sections.ll b/llvm/test/MC/WebAssembly/comdat-sections.ll --- a/llvm/test/MC/WebAssembly/comdat-sections.ll +++ b/llvm/test/MC/WebAssembly/comdat-sections.ll @@ -1,13 +1,28 @@ ; RUN: llc -dwarf-version=4 -generate-type-units \ ; RUN: -filetype=obj -O0 -mtriple=wasm32-unknown-unknown < %s \ -; RUN: | obj2yaml | FileCheck %s +; RUN: | obj2yaml | FileCheck --check-prefix=OBJ %s + +; RUN: llc -dwarf-version=4 -generate-type-units \ +; RUN: -filetype=asm -O0 -mtriple=wasm32-unknown-unknown < %s \ +; RUN: | FileCheck --check-prefix=ASM %s + + +; OBJ: Comdats: +; OBJ-NEXT: - Name: '4721183873463917179' +; OBJ-NEXT: Entries: +; OBJ-NEXT: - Kind: SECTION +; OBJ-NEXT: Index: 3 -; CHECK: Comdats: -; CHECK: - Name: '4721183873463917179' -; CHECK: Entries: -; CHECK: - Kind: SECTION -; CHECK: Index: 3 +; ASM: .section .debug_types,"G",@,4721183873463917179,comdat +; Here we are not trying to verify all of the debug info; just enough to ensure +; that the section contains a type unit for a type with matching signature +; ASM-NEXT: .int32 .Ldebug_info_end0-.Ldebug_info_start0 # Length of Unit +; ASM-NEXT: .Ldebug_info_start0: +; ASM-NEXT: .int16 4 # DWARF version number +; ASM-NEXT: .int32 .debug_abbrev0 # Offset Into Abbrev. Section +; ASM-NEXT: .int8 4 # Address Size (in bytes) +; ASM-NEXT: .int64 4721183873463917179 # Type Signature ; ModuleID = 't.cpp' source_filename = "t.cpp" diff --git a/llvm/test/MC/WebAssembly/comdat-sections.s b/llvm/test/MC/WebAssembly/comdat-sections.s new file mode 100644 --- /dev/null +++ b/llvm/test/MC/WebAssembly/comdat-sections.s @@ -0,0 +1,50 @@ +# RUN: llvm-mc -triple=wasm32 -filetype=obj %s -o - | obj2yaml | FileCheck %s + + .section .text.foo,"G",@,abc123,comdat + .globl foo + .type foo,@function +foo: + .functype foo () -> () + return + end_function + + .globl bar +bar: + .functype bar () -> () + return + end_function + + .section .debug_foo,"G",@,abc123,comdat + .int32 42 + .section .debug_foo,"G",@,duplicate,comdat + .int64 234 + +# Check that there are 2 identically-named custom sections, with the desired +# contents +# CHECK: - Type: CUSTOM +# CHECK-NEXT: Name: .debug_foo +# CHECK-NEXT: Payload: 2A000000 +# CHECK-NEXT: - Type: CUSTOM +# CHECK-NEXT: Name: .debug_foo +# CHECK-NEXT: Payload: EA00000000000000 + +# And check that they are in 2 different comdat groups +# CHECK-NEXT:- Type: CUSTOM +# CHECK-NEXT: Name: linking +# CHECK-NEXT: Version: 2 +# CHECK: Comdats: +# CHECK-NEXT: - Name: abc123 +# CHECK-NEXT: Entries: +# CHECK-NEXT: - Kind: FUNCTION +# CHECK-NEXT: Index: 0 + +# If the user forgets to create a new section for a function, one is created for +# them by the assembler. Check that it is also in the same group. +# CHECK-NEXT: - Kind: FUNCTION +# CHECK-NEXT: Index: 1 +# CHECK-NEXT: - Kind: SECTION +# CHECK-NEXT: Index: 4 +# CHECK-NEXT: - Name: duplicate +# CHECK-NEXT: Entries: +# CHECK-NEXT: - Kind: SECTION +# CHECK-NEXT: Index: 5 diff --git a/llvm/test/MC/WebAssembly/comdat.ll b/llvm/test/MC/WebAssembly/comdat.ll --- a/llvm/test/MC/WebAssembly/comdat.ll +++ b/llvm/test/MC/WebAssembly/comdat.ll @@ -1,4 +1,8 @@ ; RUN: llc -filetype=obj %s -o - | obj2yaml | FileCheck %s +; RUN: llc -filetype=asm %s -asm-verbose=false -o - | FileCheck --check-prefix=ASM %s +; RUN: llc -filetype=asm %s -o %t.s | llvm-mc -triple=wasm32 -filetype=obj %t.s -o - | obj2yaml | FileCheck %s +; These RUN lines verify the ll direct-to-object path, the ll->asm path, and the +; object output via asm. target triple = "wasm32-unknown-unknown" @@ -116,3 +120,19 @@ ; CHECK-NEXT: - Kind: DATA ; CHECK-NEXT: Index: 0 ; CHECK-NEXT: ... + + +; ASM: .section .text.basicInlineFn,"G",@,basicInlineFn,comdat +; ASM-NEXT: .weak basicInlineFn +; ASM-NEXT: .type basicInlineFn,@function +; ASM-NEXT: basicInlineFn: + +; ASM: .section .text.sharedFn,"G",@,sharedComdat,comdat +; ASM-NEXT: .weak sharedFn +; ASM-NEXT: .type sharedFn,@function +; ASM-NEXT: sharedFn: + +; ASM: .type constantData,@object +; ASM-NEXT: .section .rodata.constantData,"G",@,sharedComdat,comdat +; ASM-NEXT: .weak constantData +; ASM-NEXT: constantData: