[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

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;

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,

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)

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:

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:

llvm/test/CodeGen/WebAssembly/funcref-table_call.ll

@@ -5,16 +5,9 @@
 
 @funcref_table = local_unnamed_addr addrspace(1) global [0 x %funcref] undef
 
+;  CHECK: .tabletype  __funcref_call_table, funcref, 1
+
 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-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:
 

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:

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:

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:

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:

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]

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

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 () -> ()