[llvm-branch-commits] [llvm] 4564553 - [WebAssembly] Support COMDAT sections in assembly syntax
Derek Schuff via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Thu Dec 10 14:50:45 PST 2020
Author: Derek Schuff
Date: 2020-12-10T14:46:24-08:00
New Revision: 4564553b8d8ab81dc21431a35275581cb42329c8
URL: https://github.com/llvm/llvm-project/commit/4564553b8d8ab81dc21431a35275581cb42329c8
DIFF: https://github.com/llvm/llvm-project/commit/4564553b8d8ab81dc21431a35275581cb42329c8.diff
LOG: [WebAssembly] Support COMDAT sections in assembly syntax
This CL changes the asm syntax for section flags, making them more like ELF
(previously "passive" was the only option). Now we also allow "G" to designate
COMDAT group sections. In these sections we set the appropriate comdat flag on
function symbols, and also avoid auto-creating a new section for them.
This also adds asm-based tests for the changes D92691 to go along with
the direct-to-object tests.
Differential Revision: https://reviews.llvm.org/D92952
Added:
llvm/test/MC/WebAssembly/comdat-sections.s
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:
################################################################################
diff --git a/llvm/lib/MC/MCParser/WasmAsmParser.cpp b/llvm/lib/MC/MCParser/WasmAsmParser.cpp
index c8f9807d0c54..0c255ef02d2a 100644
--- a/llvm/lib/MC/MCParser/WasmAsmParser.cpp
+++ b/llvm/lib/MC/MCParser/WasmAsmParser.cpp
@@ -90,15 +90,40 @@ class WasmAsmParser : public MCAsmParserExtension {
return false;
}
- 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")
+ 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 @@ 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;
- 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 @@ 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);
- else if (TypeName == "global")
+ auto *Current =
+ cast<MCSectionWasm>(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
index 27ed51802a2e..81dc4329be6a 100644
--- a/llvm/lib/MC/MCSectionWasm.cpp
+++ b/llvm/lib/MC/MCSectionWasm.cpp
@@ -64,7 +64,9 @@ void MCSectionWasm::PrintSwitchToSection(const MCAsmInfo &MAI, const Triple &T,
OS << ",\"";
if (IsPassive)
- OS << "passive";
+ OS << "p";
+ if (Group)
+ OS << "G";
OS << '"';
@@ -78,6 +80,12 @@ 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 77df4acfe11a..c8d2e5dbdbdc 100644
--- a/llvm/lib/MC/WasmObjectWriter.cpp
+++ b/llvm/lib/MC/WasmObjectWriter.cpp
@@ -1367,6 +1367,9 @@ 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 7426d87ccbc7..55f79bafe414 100644
--- a/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
+++ b/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
@@ -971,12 +971,27 @@ 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 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<MCSymbolWasm>(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
index 75381ff6f904..bb8421829f01 100644
--- 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
index 000000000000..291e5c6f8de1
--- /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, 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 f7809e859ef6..898278a49157 100644
--- 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 @@ 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