[llvm-branch-commits] [llvm] dd1aa4f - Revert "[WebAssembly] Support COMDAT sections in assembly syntax"
Derek Schuff via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Thu Dec 10 16:00:09 PST 2020
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, at 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, at 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, at function
-; ASM-NEXT: basicInlineFn:
-
-; ASM: .section .text.sharedFn,"G",@,sharedComdat,comdat
-; ASM-NEXT: .weak sharedFn
-; ASM-NEXT: .type sharedFn, at function
-; ASM-NEXT: sharedFn:
-
-; ASM: .type constantData, at object
-; ASM-NEXT: .section .rodata.constantData,"G",@,sharedComdat,comdat
-; ASM-NEXT: .weak constantData
-; ASM-NEXT: constantData:
More information about the llvm-branch-commits
mailing list