Author: Derek Schuff Date: 2020-12-10T15:55:33-08:00 New Revision: dd1aa4fdd82bc4b33e9661eda6039760408501d9
URL: https://github.com/llvm/llvm-project/commit/dd1aa4fdd82bc4b33e9661eda6039760408501d9 DIFF: https://github.com/llvm/llvm-project/commit/dd1aa4fdd82bc4b33e9661eda6039760408501d9.diff LOG: Revert "[WebAssembly] Support COMDAT sections in assembly syntax" This reverts commit 4564553b8d8ab81dc21431a35275581cb42329c8. It broke several buildbots. Added: Modified: llvm/lib/MC/MCParser/WasmAsmParser.cpp llvm/lib/MC/MCSectionWasm.cpp llvm/lib/MC/WasmObjectWriter.cpp llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp llvm/test/MC/WebAssembly/comdat-sections.ll llvm/test/MC/WebAssembly/comdat.ll Removed: llvm/test/MC/WebAssembly/comdat-sections.s ################################################################################ diff --git a/llvm/lib/MC/MCParser/WasmAsmParser.cpp b/llvm/lib/MC/MCParser/WasmAsmParser.cpp index 0c255ef02d2a..c8f9807d0c54 100644 --- a/llvm/lib/MC/MCParser/WasmAsmParser.cpp +++ b/llvm/lib/MC/MCParser/WasmAsmParser.cpp @@ -90,40 +90,15 @@ class WasmAsmParser : public MCAsmParserExtension { return false; } - bool parseSectionFlags(StringRef FlagStr, bool &Passive, bool &Group) { - for (char C : FlagStr) { - switch (C) { - case 'p': + bool parseSectionFlags(StringRef FlagStr, bool &Passive) { + SmallVector<StringRef, 2> Flags; + // If there are no flags, keep Flags empty + FlagStr.split(Flags, ",", -1, false); + for (auto &Flag : Flags) { + if (Flag == "passive") Passive = true; - 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'"); + else + return error("Expected section flags, instead got: ", Lexer->getTok()); } return false; } @@ -155,34 +130,27 @@ class WasmAsmParser : public MCAsmParserExtension { 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; - bool Group = false; - if (parseSectionFlags(getTok().getStringContents(), Passive, Group)) - return true; - - Lex(); - - if (expect(AsmToken::Comma, ",") || expect(AsmToken::At, "@")) - return true; - - StringRef GroupName; - if (Group && parseGroup(GroupName)) - return true; - - if (expect(AsmToken::EndOfStatement, "eol")) + if (parseSectionFlags(getTok().getStringContents(), Passive)) return true; - // TODO: Parse UniqueID - MCSectionWasm *WS = getContext().getWasmSection( - Name, Kind.getValue(), GroupName, MCContext::GenericSectionID); if (Passive) { - if (!WS->isWasmData()) + if (!Section->isWasmData()) return Parser->Error(getTok().getLoc(), "Only data sections can be passive"); - WS->setPassive(); + Section->setPassive(); } + + Lex(); + + if (expect(AsmToken::Comma, ",") || expect(AsmToken::At, "@") || + expect(AsmToken::EndOfStatement, "eol")) + return true; + + auto WS = getContext().getWasmSection(Name, Kind.getValue()); getStreamer().SwitchSection(WS); return false; } @@ -221,13 +189,9 @@ class WasmAsmParser : public MCAsmParserExtension { 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); - auto *Current = - cast<MCSectionWasm>(getStreamer().getCurrentSection().first); - if (Current->getGroup()) - WasmSym->setComdat(true); - } else if (TypeName == "global") + 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 index 81dc4329be6a..27ed51802a2e 100644 --- a/llvm/lib/MC/MCSectionWasm.cpp +++ b/llvm/lib/MC/MCSectionWasm.cpp @@ -64,9 +64,7 @@ void MCSectionWasm::PrintSwitchToSection(const MCAsmInfo &MAI, const Triple &T, OS << ",\""; if (IsPassive) - OS << "p"; - if (Group) - OS << "G"; + OS << "passive"; OS << '"'; @@ -80,12 +78,6 @@ void MCSectionWasm::PrintSwitchToSection(const MCAsmInfo &MAI, const Triple &T, // 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 index c8d2e5dbdbdc..77df4acfe11a 100644 --- a/llvm/lib/MC/WasmObjectWriter.cpp +++ b/llvm/lib/MC/WasmObjectWriter.cpp @@ -1367,9 +1367,6 @@ uint64_t WasmObjectWriter::writeOneObject(MCAssembler &Asm, 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 index 55f79bafe414..7426d87ccbc7 100644 --- a/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp +++ b/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp @@ -971,27 +971,12 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser { 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<MCSectionWasm>(getStreamer().getCurrentSection().first); if (!CWS || !CWS->getKind().isText()) return; auto SecName = ".text." + SymName; - - 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<MCSymbolWasm>(Symbol)->setComdat(true); - auto *WS = - getContext().getWasmSection(SecName, SectionKind::getText(), Group, - MCContext::GenericSectionID, nullptr); + auto WS = getContext().getWasmSection(SecName, SectionKind::getText()); 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 index bb8421829f01..75381ff6f904 100644 --- a/llvm/test/MC/WebAssembly/comdat-sections.ll +++ b/llvm/test/MC/WebAssembly/comdat-sections.ll @@ -1,28 +1,13 @@ ; RUN: llc -dwarf-version=4 -generate-type-units \ ; RUN: -filetype=obj -O0 -mtriple=wasm32-unknown-unknown < %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 +; RUN: | obj2yaml | FileCheck %s -; 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 +; CHECK: Comdats: +; CHECK: - Name: '4721183873463917179' +; CHECK: Entries: +; CHECK: - Kind: SECTION +; CHECK: Index: 3 ; 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 deleted file mode 100644 index 291e5c6f8de1..000000000000 --- a/llvm/test/MC/WebAssembly/comdat-sections.s +++ /dev/null @@ -1,50 +0,0 @@ -# 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 diff erent 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 index 898278a49157..f7809e859ef6 100644 --- a/llvm/test/MC/WebAssembly/comdat.ll +++ b/llvm/test/MC/WebAssembly/comdat.ll @@ -1,8 +1,4 @@ ; 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" @@ -120,19 +116,3 @@ define linkonce_odr i32 @sharedFn() #1 comdat($sharedComdat) { ; 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: _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits