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

Paulo Matos via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 4 13:02:06 PST 2022


Author: Paulo Matos
Date: 2022-02-04T22:01:46+01:00
New Revision: c67c9cfe3f39141102364176bd883f74022bc133

URL: https://github.com/llvm/llvm-project/commit/c67c9cfe3f39141102364176bd883f74022bc133
DIFF: https://github.com/llvm/llvm-project/commit/c67c9cfe3f39141102364176bd883f74022bc133.diff

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

Reland of 00bf4755.

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/D118995

Added: 
    llvm/test/CodeGen/WebAssembly/only-data.ll
    llvm/test/CodeGen/WebAssembly/table-types.ll

Modified: 
    llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
    llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.h
    llvm/lib/Target/WebAssembly/WebAssemblyMCLowerPrePass.cpp
    llvm/test/CodeGen/WebAssembly/externref-tableget.ll
    llvm/test/CodeGen/WebAssembly/externref-tableset.ll
    llvm/test/CodeGen/WebAssembly/funcref-table_call.ll
    llvm/test/CodeGen/WebAssembly/funcref-tableget.ll
    llvm/test/CodeGen/WebAssembly/funcref-tableset.ll
    llvm/test/CodeGen/WebAssembly/global-get.ll
    llvm/test/CodeGen/WebAssembly/global-set.ll
    llvm/test/MC/WebAssembly/assembler-binary.ll
    llvm/test/MC/WebAssembly/stack-ptr-mclower.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
index bf326e5106be4..4dc2c4c9e29e2 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
@@ -180,26 +180,26 @@ void WebAssemblyAsmPrinter::emitGlobalVariable(const GlobalVariable *GV) {
   MCSymbolWasm *Sym = cast<MCSymbolWasm>(getSymbol(GV));
 
   if (!Sym->getType()) {
-    const WebAssemblyTargetLowering &TLI = *Subtarget->getTargetLowering();
     SmallVector<MVT, 1> VTs;
     Type *GlobalVT = GV->getValueType();
-    computeLegalValueVTs(TLI, GV->getParent()->getContext(),
-                         GV->getParent()->getDataLayout(), GlobalVT, VTs);
+    if (Subtarget) {
+      // Subtarget is only set when a function is defined, because
+      // each function can declare a 
diff erent subtarget. For example,
+      // on ARM a compilation unit might have a function on ARM and
+      // another on Thumb. Therefore only if Subtarget is non-null we
+      // can actually calculate the legal VTs.
+      const WebAssemblyTargetLowering &TLI = *Subtarget->getTargetLowering();
+      computeLegalValueVTs(TLI, GV->getParent()->getContext(),
+                           GV->getParent()->getDataLayout(), GlobalVT, VTs);
+    }
     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 +271,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,8 +378,11 @@ void WebAssemblyAsmPrinter::emitExternalDecls(const Module &M) {
     }
   }
 }
-  
+
 void WebAssemblyAsmPrinter::emitEndOfAsmFile(Module &M) {
+  // This is required to emit external declarations (like .functypes) when
+  // no functions are defined in the compilation unit and therefore,
+  // emitExternalDecls() is not called until now.
   emitExternalDecls(M);
 
   // When a function's address is taken, a TABLE_INDEX relocation is emitted
@@ -532,6 +551,7 @@ void WebAssemblyAsmPrinter::EmitTargetFeatures(Module &M) {
 }
 
 void WebAssemblyAsmPrinter::emitConstantPool() {
+  emitExternalDecls(*MMI->getModule());
   assert(MF->getConstantPool()->getConstants().empty() &&
          "WebAssembly disables constant pools");
 }
@@ -540,17 +560,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 6b2f2000a0bd9..fd15c8d793db7 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.h
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.h
@@ -66,10 +66,10 @@ class LLVM_LIBRARY_VISIBILITY WebAssemblyAsmPrinter final : public AsmPrinter {
   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 37ac8e75f4b79..21f6fd37d4026 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 1161b6e3659ac..655cbdf9a4730 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 bd5ad6942fcda..e22f5d054e418 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 4b5a9b42f68bc..37076db6da427 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 63cd69a2e9ffa..a34ba0f7c3b0c 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 ddc6c3e26b67d..257594dc7df0b 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 a31b1f1c30ebd..07477e690c683 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 54034e476fd66..44a9acebe9818 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/only-data.ll b/llvm/test/CodeGen/WebAssembly/only-data.ll
new file mode 100644
index 0000000000000..bf4979d022520
--- /dev/null
+++ b/llvm/test/CodeGen/WebAssembly/only-data.ll
@@ -0,0 +1,14 @@
+; RUN: llc < %s --mtriple=wasm32-unknown-unknown -asm-verbose=false -mattr=+reference-types | FileCheck %s
+; Verify that types are output for external symbols, even in the absence of any defined functions
+
+;      CHECK: .type foo, at object
+; CHECK-NEXT: .section .data.foo,"",@
+; CHECK-NEXT: .globl foo
+ at foo = local_unnamed_addr global i32 (i32)* @bar, align 4
+
+; CHECK-LABEL: foo:
+;  CHECK-NEXT: .int32 bar
+;  CHECK-NEXT: .size foo, 4
+
+; CHECK: .functype bar (i32) -> (i32)
+declare i32 @bar(i32 noundef)

diff  --git a/llvm/test/CodeGen/WebAssembly/table-types.ll b/llvm/test/CodeGen/WebAssembly/table-types.ll
new file mode 100644
index 0000000000000..f4a66629df43a
--- /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:
+ at 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:
+ at 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:
+ at extern_eref_table = external addrspace(1) global [0 x %externref]
+
+; CHECK: .tabletype fref_table, funcref
+; CHECK-NEXT: .globl fref_table
+; CHECK-LABEL: fref_table:
+ at 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:
+ at 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:
+ at 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 e5dcd270603ea..c3d6bd588d24a 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 d8280a9edb99a..c4b5b08109b14 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, at function
 ; CHECK-NEXT: bar:
 ; CHECK-NEXT:         .functype       bar () -> ()


        


More information about the llvm-commits mailing list