[WebAssembly] Refactor and fix emission of external IR global decls

This patches fixes the visibility and linkage information of symbols
referring to IR globals.

Emission of external declarations is now done in the first execution
of emitConstantPool rather than in emitLinkage (and a few other
places). This is the point where we have already gathered information
about used symbols (by running the MC Lower PrePass) and not yet
started emitting any functions so that any declarations that need to
be emitted are done so at the top of the file before any functions.

This changes the order of a few directives in the final asm file which
required an update to a few tests.

Reviewed By: sbc100

Differential Revision: https://reviews.llvm.org/D118122

13 files changed, 104 insertions, 64 deletions

diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
index bf326e5106be..589e3a1aad29 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
@@ -188,18 +188,11 @@ void WebAssemblyAsmPrinter::emitGlobalVariable(const GlobalVariable *GV) {
     WebAssembly::wasmSymbolSetType(Sym, GlobalVT, VTs);
   }
 
-  // If the GlobalVariable refers to a table, we handle it here instead of
-  // in emitExternalDecls
-  if (Sym->isTable()) {
-    getTargetStreamer()->emitTableType(Sym);
-    return;
-  }
-
   emitVisibility(Sym, GV->getVisibility(), !GV->isDeclaration());
+  emitSymbolType(Sym);
   if (GV->hasInitializer()) {
     assert(getSymbolPreferLocal(*GV) == Sym);
     emitLinkage(GV, Sym);
-    getTargetStreamer()->emitGlobalType(Sym);
     OutStreamer->emitLabel(Sym);
     // TODO: Actually emit the initializer value.  Otherwise the global has the
     // default value for its type (0, ref.null, etc).
@@ -271,31 +264,47 @@ MCSymbol *WebAssemblyAsmPrinter::getOrCreateWasmSymbol(StringRef Name) {
   return WasmSym;
 }
 
+void WebAssemblyAsmPrinter::emitSymbolType(const MCSymbolWasm *Sym) {
+  Optional<wasm::WasmSymbolType> WasmTy = Sym->getType();
+  if (!WasmTy)
+    return;
+
+  switch (WasmTy.getValue()) {
+  case wasm::WASM_SYMBOL_TYPE_GLOBAL:
+    getTargetStreamer()->emitGlobalType(Sym);
+    break;
+  case wasm::WASM_SYMBOL_TYPE_TAG:
+    getTargetStreamer()->emitTagType(Sym);
+    break;
+  case wasm::WASM_SYMBOL_TYPE_TABLE:
+    getTargetStreamer()->emitTableType(Sym);
+    break;
+  default:
+    break; // We only handle globals, tags and tables here
+  }
+}
+
 void WebAssemblyAsmPrinter::emitExternalDecls(const Module &M) {
   if (signaturesEmitted)
     return;
   signaturesEmitted = true;
 
   // Normally symbols for globals get discovered as the MI gets lowered,
-  // but we need to know about them ahead of time.
+  // but we need to know about them ahead of time. This will however,
+  // only find symbols that have been used. Unused symbols from globals will
+  // not be found here.
   MachineModuleInfoWasm &MMIW = MMI->getObjFileInfo<MachineModuleInfoWasm>();
   for (const auto &Name : MMIW.MachineSymbolsUsed) {
     getOrCreateWasmSymbol(Name.getKey());
   }
 
   for (auto &It : OutContext.getSymbols()) {
-    // Emit .globaltype, .tagtype, or .tabletype declarations.
+    // Emit .globaltype, .tagtype, or .tabletype declarations for extern
+    // declarations, i.e. those that have only been declared (but not defined)
+    // in the current module
     auto Sym = cast<MCSymbolWasm>(It.getValue());
-    if (Sym->getType() == wasm::WASM_SYMBOL_TYPE_GLOBAL) {
-      // .globaltype already handled by emitGlobalVariable for defined
-      // variables; here we make sure the types of external wasm globals get
-      // written to the file.
-      if (Sym->isUndefined())
-        getTargetStreamer()->emitGlobalType(Sym);
-    } else if (Sym->getType() == wasm::WASM_SYMBOL_TYPE_TAG)
-      getTargetStreamer()->emitTagType(Sym);
-    else if (Sym->getType() == wasm::WASM_SYMBOL_TYPE_TABLE)
-      getTargetStreamer()->emitTableType(Sym);
+    if (!Sym->isDefined())
+      emitSymbolType(Sym);
   }
 
   DenseSet<MCSymbol *> InvokeSymbols;
@@ -362,10 +371,8 @@ void WebAssemblyAsmPrinter::emitExternalDecls(const Module &M) {
     }
   }
 }
-  
-void WebAssemblyAsmPrinter::emitEndOfAsmFile(Module &M) {
-  emitExternalDecls(M);
 
+void WebAssemblyAsmPrinter::emitEndOfAsmFile(Module &M) {
   // When a function's address is taken, a TABLE_INDEX relocation is emitted
   // against the function symbol at the use site.  However the relocation
   // doesn't explicitly refer to the table.  In the future we may want to
@@ -532,6 +539,8 @@ void WebAssemblyAsmPrinter::EmitTargetFeatures(Module &M) {
 }
 
 void WebAssemblyAsmPrinter::emitConstantPool() {
+  const Module *M = MMI->getModule();
+  emitExternalDecls(*M);
   assert(MF->getConstantPool()->getConstants().empty() &&
          "WebAssembly disables constant pools");
 }
@@ -540,17 +549,6 @@ void WebAssemblyAsmPrinter::emitJumpTableInfo() {
   // Nothing to do; jump tables are incorporated into the instruction stream.
 }
 
-void WebAssemblyAsmPrinter::emitLinkage(const GlobalValue *GV, MCSymbol *Sym)
-  const {
-  AsmPrinter::emitLinkage(GV, Sym);
-  // This gets called before the function label and type are emitted.
-  // We use it to emit signatures of external functions.
-  // FIXME casts!
-  const_cast<WebAssemblyAsmPrinter *>(this)
-    ->emitExternalDecls(*MMI->getModule());
-}
-
-
 void WebAssemblyAsmPrinter::emitFunctionBodyStart() {
   const Function &F = MF->getFunction();
   SmallVector<MVT, 1> ResultVTs;
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.h b/llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.h
index 6b2f2000a0bd..fd15c8d793db 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.h
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.h
@@ -66,10 +66,10 @@ public:
   void emitEndOfAsmFile(Module &M) override;
   void EmitProducerInfo(Module &M);
   void EmitTargetFeatures(Module &M);
+  void emitSymbolType(const MCSymbolWasm *Sym);
   void emitGlobalVariable(const GlobalVariable *GV) override;
   void emitJumpTableInfo() override;
   void emitConstantPool() override;
-  void emitLinkage(const GlobalValue *, MCSymbol *) const override;
   void emitFunctionBodyStart() override;
   void emitInstruction(const MachineInstr *MI) override;
   bool PrintAsmOperand(const MachineInstr *MI, unsigned OpNo,
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyMCLowerPrePass.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyMCLowerPrePass.cpp
index 37ac8e75f4b7..21f6fd37d402 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyMCLowerPrePass.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyMCLowerPrePass.cpp
@@ -65,6 +65,9 @@ ModulePass *llvm::createWebAssemblyMCLowerPrePass() {
 // for all functions before AsmPrinter. If this way of doing things is ever
 // suboptimal, we could opt to make it a MachineFunctionPass and instead use
 // something like createBarrierNoopPass() to enforce ordering.
+//
+// The information stored here is essential for emitExternalDecls in the Wasm
+// AsmPrinter
 bool WebAssemblyMCLowerPrePass::runOnModule(Module &M) {
   auto *MMIWP = getAnalysisIfAvailable<MachineModuleInfoWrapperPass>();
   if (!MMIWP)
diff --git a/llvm/test/CodeGen/WebAssembly/externref-tableget.ll b/llvm/test/CodeGen/WebAssembly/externref-tableget.ll
index 1161b6e3659a..655cbdf9a473 100644
--- a/llvm/test/CodeGen/WebAssembly/externref-tableget.ll
+++ b/llvm/test/CodeGen/WebAssembly/externref-tableget.ll
@@ -73,4 +73,5 @@ define %externref @get_externref_from_table_with_var_offset2(i32 %i) {
   ret %externref %ref
 }
 
-; CHECK: .tabletype externref_table, externref
+; CHECK:       .tabletype externref_table, externref
+; CHECK-LABEL: externref_table:
diff --git a/llvm/test/CodeGen/WebAssembly/externref-tableset.ll b/llvm/test/CodeGen/WebAssembly/externref-tableset.ll
index bd5ad6942fcd..e22f5d054e41 100644
--- a/llvm/test/CodeGen/WebAssembly/externref-tableset.ll
+++ b/llvm/test/CodeGen/WebAssembly/externref-tableset.ll
@@ -94,4 +94,5 @@ define void @set_externref_table_with_id_from_call(%externref %g) {
   ret void
 }
 
-; CHECK: .tabletype externref_table, externref
+;       CHECK: .tabletype externref_table, externref
+; CHECK-LABEL: externref_table:
diff --git a/llvm/test/CodeGen/WebAssembly/funcref-table_call.ll b/llvm/test/CodeGen/WebAssembly/funcref-table_call.ll
index 4b5a9b42f68b..37076db6da42 100644
--- a/llvm/test/CodeGen/WebAssembly/funcref-table_call.ll
+++ b/llvm/test/CodeGen/WebAssembly/funcref-table_call.ll
@@ -5,16 +5,9 @@
 
 @funcref_table = local_unnamed_addr addrspace(1) global [0 x %funcref] undef
 
-define void @call_funcref_from_table(i32 %i) {
-  %p = getelementptr [0 x %funcref], [0 x %funcref] addrspace (1)* @funcref_table, i32 0, i32 %i
-  %ref = load %funcref, %funcref addrspace(1)* %p
-  %fn = bitcast %funcref %ref to %funcptr
-  call addrspace(20) void %fn()
-  ret void
-}
-
-; CHECK: .tabletype      __funcref_call_table, funcref, 1
+;  CHECK: .tabletype  __funcref_call_table, funcref, 1
 
+define void @call_funcref_from_table(i32 %i) {
 ; CHECK-LABEL: call_funcref_from_table:
 ; CHECK-NEXT: .functype       call_funcref_from_table (i32) -> ()
 ; CHECK-NEXT: i32.const       0
@@ -27,6 +20,13 @@ define void @call_funcref_from_table(i32 %i) {
 ; CHECK-NEXT: ref.null_func
 ; CHECK-NEXT: table.set       __funcref_call_table
 ; CHECK-NEXT: end_function
+  %p = getelementptr [0 x %funcref], [0 x %funcref] addrspace (1)* @funcref_table, i32 0, i32 %i
+  %ref = load %funcref, %funcref addrspace(1)* %p
+  %fn = bitcast %funcref %ref to %funcptr
+  call addrspace(20) void %fn()
+  ret void
+}
 
-; CHECK: .tabletype     funcref_table, funcref
+;       CHECK: .tabletype funcref_table, funcref
+; CHECK-LABEL: funcref_table:
 
diff --git a/llvm/test/CodeGen/WebAssembly/funcref-tableget.ll b/llvm/test/CodeGen/WebAssembly/funcref-tableget.ll
index 63cd69a2e9ff..a34ba0f7c3b0 100644
--- a/llvm/test/CodeGen/WebAssembly/funcref-tableget.ll
+++ b/llvm/test/CodeGen/WebAssembly/funcref-tableget.ll
@@ -72,4 +72,5 @@ define %funcref @get_funcref_from_table_with_var_offset2(i32 %i) {
   ret %funcref %ref
 }
 
-; CHECK: .tabletype funcref_table, funcref
+;       CHECK: .tabletype funcref_table, funcref
+; CHECK-LABEL: funcref_table:
diff --git a/llvm/test/CodeGen/WebAssembly/funcref-tableset.ll b/llvm/test/CodeGen/WebAssembly/funcref-tableset.ll
index ddc6c3e26b67..257594dc7df0 100644
--- a/llvm/test/CodeGen/WebAssembly/funcref-tableset.ll
+++ b/llvm/test/CodeGen/WebAssembly/funcref-tableset.ll
@@ -78,4 +78,5 @@ define void @set_funcref_table_with_var_offset2(%funcref %g, i32 %i) {
   ret void
 }
 
-; CHECK: .tabletype funcref_table, funcref
+;       CHECK: .tabletype funcref_table, funcref
+; CHECK-LABEL: funcref_table:
diff --git a/llvm/test/CodeGen/WebAssembly/global-get.ll b/llvm/test/CodeGen/WebAssembly/global-get.ll
index a31b1f1c30eb..07477e690c68 100644
--- a/llvm/test/CodeGen/WebAssembly/global-get.ll
+++ b/llvm/test/CodeGen/WebAssembly/global-get.ll
@@ -54,28 +54,26 @@ define i32 @return_extern_i32_global() {
 }
 
 
-; CHECK: .globl i32_global
 ; CHECK: .globaltype i32_global, i32
+; CHECK: .globl i32_global
 ; CHECK-LABEL: i32_global:
 
-; CHECK: .globl i64_global
 ; CHECK: .globaltype i64_global, i64
+; CHECK: .globl i64_global
 ; CHECK-LABEL: i64_global:
 
-; CHECK: .globl f32_global
 ; CHECK: .globaltype f32_global, f32
+; CHECK: .globl f32_global
 ; CHECK-LABEL: f32_global:
 
-; CHECK: .globl f64_global
 ; CHECK: .globaltype f64_global, f64
+; CHECK: .globl f64_global
 ; CHECK-LABEL: f64_global:
 
-; FIXME: are we still expecting these to be emitted?
-
+; CHECK: .globaltype i32_external_used, i32
 ; CHECK-NOT: .global i32_external_used
-; CHECK-NOT: .globaltype i32_external_used, i32
 ; CHECK-NOT: i32_external_used:
 
+; CHECK: .globaltype i32_external_unused, i32
 ; CHECK-NOT: .global i32_external_unused
-; CHECK-NOT: .globaltype i32_external_unused, i32
 ; CHECK-NOT: i32_external_unused:
diff --git a/llvm/test/CodeGen/WebAssembly/global-set.ll b/llvm/test/CodeGen/WebAssembly/global-set.ll
index 54034e476fd6..44a9acebe981 100644
--- a/llvm/test/CodeGen/WebAssembly/global-set.ll
+++ b/llvm/test/CodeGen/WebAssembly/global-set.ll
@@ -45,18 +45,18 @@ define void @set_f64_global(double %v) {
   ret void
 }
 
-; CHECK: .globl i32_global
 ; CHECK: .globaltype i32_global, i32
+; CHECK: .globl i32_global
 ; CHECK-LABEL: i32_global:
 
-; CHECK: .globl i64_global
 ; CHECK: .globaltype i64_global, i64
+; CHECK: .globl i64_global
 ; CHECK-LABEL: i64_global:
 
-; CHECK: .globl f32_global
 ; CHECK: .globaltype f32_global, f32
+; CHECK: .globl f32_global
 ; CHECK-LABEL: f32_global:
 
-; CHECK: .globl f64_global
 ; CHECK: .globaltype f64_global, f64
+; CHECK: .globl f64_global
 ; CHECK-LABEL: f64_global:
diff --git a/llvm/test/CodeGen/WebAssembly/table-types.ll b/llvm/test/CodeGen/WebAssembly/table-types.ll
new file mode 100644
index 000000000000..f4a66629df43
--- /dev/null
+++ b/llvm/test/CodeGen/WebAssembly/table-types.ll
@@ -0,0 +1,37 @@
+; RUN: llc < %s --mtriple=wasm32-unknown-unknown -asm-verbose=false -mattr=+reference-types | FileCheck %s
+
+%extern = type opaque
+%externref = type %extern addrspace(10)* ;; addrspace 10 is nonintegral
+
+%func = type void ()
+%funcref = type %func addrspace(20)* ;; addrspace 20 is nonintegral
+
+; CHECK: .tabletype eref_table, externref
+; CHECK-NEXT: .globl eref_table
+; CHECK-LABEL: eref_table:
+@eref_table = local_unnamed_addr addrspace(1) global [0 x %externref] undef
+
+; CHECK-NOT: .globl .Lprivate_eref_table
+; CHECK: .tabletype .Lprivate_eref_table, externref
+; CHECK-LABEL: .Lprivate_eref_table:
+@private_eref_table = private local_unnamed_addr addrspace(1) global [0 x %externref] undef
+
+; CHECK: .tabletype extern_eref_table, externref
+; CHECK-NOT: .globl extern_eref_table
+; CHECK-NOT: extern_eref_table:
+@extern_eref_table = external addrspace(1) global [0 x %externref]
+
+; CHECK: .tabletype fref_table, funcref
+; CHECK-NEXT: .globl fref_table
+; CHECK-LABEL: fref_table:
+@fref_table = local_unnamed_addr addrspace(1) global [0 x %funcref] undef
+
+; CHECK-NOT: .globl .Lprivate_fref_table
+; CHECK: .tabletype .Lprivate_fref_table, funcref
+; CHECK-LABEL: .Lprivate_fref_table:
+@private_fref_table = private local_unnamed_addr addrspace(1) global [0 x %funcref] undef
+
+; CHECK: .tabletype extern_fref_table, funcref
+; CHECK-NOT: .globl extern_fref_table
+; CHECK-NOT: extern_fref_table:
+@extern_fref_table = external addrspace(1) global [0 x %funcref]
diff --git a/llvm/test/MC/WebAssembly/assembler-binary.ll b/llvm/test/MC/WebAssembly/assembler-binary.ll
index e5dcd270603e..c3d6bd588d24 100644
--- a/llvm/test/MC/WebAssembly/assembler-binary.ll
+++ b/llvm/test/MC/WebAssembly/assembler-binary.ll
@@ -22,8 +22,8 @@ entry:
 
 ; ASM:     	.text
 ; ASM:      	.file	"assembler-binary.ll"
-; ASM:      	.globl	foo
 ; ASM:       	.functype	bar () -> ()
+; ASM:      	.globl	foo
 ; ASM:      foo:
 ; ASM-NEXT: 	.functype	foo (i32) -> ()
 ; ASM-NEXT: 	call	bar
diff --git a/llvm/test/MC/WebAssembly/stack-ptr-mclower.ll b/llvm/test/MC/WebAssembly/stack-ptr-mclower.ll
index d8280a9edb99..c4b5b08109b1 100644
--- a/llvm/test/MC/WebAssembly/stack-ptr-mclower.ll
+++ b/llvm/test/MC/WebAssembly/stack-ptr-mclower.ll
@@ -8,7 +8,7 @@ define hidden void @bar() #0 {
 }
 
 ; Function that uses explict stack, and should generate a reference to
-; __stack_pointer, along with the corresponding reloction entry.
+; __stack_pointer, along with the corresponding relocation entry.
 define hidden void @foo() #0 {
 entry:
   alloca i32, align 4
@@ -17,10 +17,10 @@ entry:
 
 ; CHECK:              .text
 ; CHECK-NEXT:         .file   "stack-ptr-mclower.ll"
+; CHECK-NEXT:         .globaltype     __stack_pointer, [[PTR]]
 ; CHECK-NEXT:         .section        .text.bar,"",@
 ; CHECK-NEXT:         .hidden bar
 ; CHECK-NEXT:         .globl  bar
-; CHECK-NEXT:         .globaltype     __stack_pointer, [[PTR]]
 ; CHECK-NEXT:         .type   bar,@function
 ; CHECK-NEXT: bar:
 ; CHECK-NEXT:         .functype       bar () -> ()