[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
This is a reland of rG4564553b8d8a with a fix to the lit pipeline in
llvm/test/MC/WebAssembly/comdat.ll

diff --git a/llvm/lib/MC/MCParser/WasmAsmParser.cpp b/llvm/lib/MC/MCParser/WasmAsmParser.cpp
index c8f9807..0c255ef 100644 (file)
--- a/llvm/lib/MC/MCParser/WasmAsmParser.cpp
+++ b/llvm/lib/MC/MCParser/WasmAsmParser.cpp
@@ -90,15 +90,40 @@ public:
     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 @@ public:
     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 @@ public:
           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<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 27ed518..81dc432 100644 (file)
--- 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 77df4ac..c8d2e5d 100644 (file)
--- 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 7426d87..55f79ba 100644 (file)
--- a/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
+++ b/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
@@ -971,12 +971,27 @@ public:
     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 75381ff..bb84218 100644 (file)
--- 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 (file)
index 0000000..291e5c6
--- /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
index f7809e8..5af940d 100644 (file)
--- 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 - | llvm-mc -triple=wasm32 -filetype=obj  -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,@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: