From c67c9cfe3f39141102364176bd883f74022bc133 Mon Sep 17 00:00:00 2001 From: Paulo Matos Date: Fri, 4 Feb 2022 22:01:41 +0100 Subject: [PATCH] [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 --- .../WebAssembly/WebAssemblyAsmPrinter.cpp | 79 +++++++++++-------- .../WebAssembly/WebAssemblyAsmPrinter.h | 2 +- .../WebAssembly/WebAssemblyMCLowerPrePass.cpp | 3 + .../CodeGen/WebAssembly/externref-tableget.ll | 3 +- .../CodeGen/WebAssembly/externref-tableset.ll | 3 +- .../CodeGen/WebAssembly/funcref-table_call.ll | 20 ++--- .../CodeGen/WebAssembly/funcref-tableget.ll | 3 +- .../CodeGen/WebAssembly/funcref-tableset.ll | 3 +- llvm/test/CodeGen/WebAssembly/global-get.ll | 14 ++-- llvm/test/CodeGen/WebAssembly/global-set.ll | 8 +- llvm/test/CodeGen/WebAssembly/only-data.ll | 14 ++++ llvm/test/CodeGen/WebAssembly/table-types.ll | 37 +++++++++ llvm/test/MC/WebAssembly/assembler-binary.ll | 2 +- llvm/test/MC/WebAssembly/stack-ptr-mclower.ll | 4 +- 14 files changed, 130 insertions(+), 65 deletions(-) create mode 100644 llvm/test/CodeGen/WebAssembly/only-data.ll create mode 100644 llvm/test/CodeGen/WebAssembly/table-types.ll diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp index bf326e5106be..4dc2c4c9e29e 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(getSymbol(GV)); if (!Sym->getType()) { - const WebAssemblyTargetLowering &TLI = *Subtarget->getTargetLowering(); SmallVector 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 different 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 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(); 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(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 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(this) - ->emitExternalDecls(*MMI->getModule()); -} - - void WebAssemblyAsmPrinter::emitFunctionBodyStart() { const Function &F = MF->getFunction(); SmallVector 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(); 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 +; 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: 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/only-data.ll b/llvm/test/CodeGen/WebAssembly/only-data.ll new file mode 100644 index 000000000000..bf4979d02252 --- /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,@object +; CHECK-NEXT: .section .data.foo,"",@ +; CHECK-NEXT: .globl foo +@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 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 () -> ()