llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lld Author: Max Desiatov (MaxDesiatov) <details> <summary>Changes</summary> A Wasm global in `addrspace(1)` lost its initializer, because the object writer hardcoded 0 into the Global section's init expression. This carries the constant's raw bits on `MCSymbolWasm` so the writer emits the real i32/i64/f32/f64 value. Added `llvm/test/CodeGen/WebAssembly/global-init.ll`, `clang/test/CodeGen/WebAssembly/wasm-globals.c`, and `lld/test/wasm/global-init.ll`. --- Full diff: https://github.com/llvm/llvm-project/pull/212007.diff 6 Files Affected: - (added) clang/test/CodeGen/WebAssembly/wasm-globals.c (+43) - (added) lld/test/wasm/global-init.ll (+26) - (modified) llvm/include/llvm/MC/MCSymbolWasm.h (+8) - (modified) llvm/lib/MC/WasmObjectWriter.cpp (+10-4) - (modified) llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp (+30-2) - (added) llvm/test/CodeGen/WebAssembly/global-init.ll (+44) ``````````diff diff --git a/clang/test/CodeGen/WebAssembly/wasm-globals.c b/clang/test/CodeGen/WebAssembly/wasm-globals.c new file mode 100644 index 0000000000000..31378f0c862c2 --- /dev/null +++ b/clang/test/CodeGen/WebAssembly/wasm-globals.c @@ -0,0 +1,43 @@ +// REQUIRES: webassembly-registered-target +// RUN: %clang_cc1 -triple wasm32-unknown-unknown -mrelocation-model static -O2 -emit-llvm -o - %s | FileCheck %s --check-prefix=IR +// RUN: %clang_cc1 -triple wasm32-unknown-unknown -mrelocation-model static -O2 -S -o - %s | FileCheck %s --check-prefix=ASM +// RUN: %clang_cc1 -triple wasm32-unknown-unknown -mrelocation-model static -O2 -emit-obj -o %t.o %s +// RUN: obj2yaml %t.o | FileCheck %s --check-prefix=OBJ + +// IR: @mut = {{.*}}addrspace(1) global i32 33554435 +int mut __attribute__((address_space(1))) = 0x02000003; + +// Without volatile the load of a constant global folds to i32.const, hiding +// the global.get. +// IR: @immut = {{.*}}addrspace(1) constant i32 42 +const volatile int immut __attribute__((address_space(1))) = 42; + +// ASM-LABEL: read_mut: +// ASM: global.get mut{{$}} +int read_mut(void) { return mut; } + +// ASM-LABEL: write_mut: +// ASM: global.set mut{{$}} +void write_mut(int x) { mut = x; } + +// ASM-LABEL: read_immut: +// ASM: global.get immut{{$}} +int read_immut(void) { return immut; } + +// ASM: .globaltype mut, i32{{$}} +// ASM: .globaltype immut, i32, immutable{{$}} + +// OBJ: - Type: GLOBAL +// OBJ-NEXT: Globals: +// OBJ-NEXT: - Index: 0 +// OBJ-NEXT: Type: I32 +// OBJ-NEXT: Mutable: true +// OBJ-NEXT: InitExpr: +// OBJ-NEXT: Opcode: I32_CONST +// OBJ-NEXT: Value: 33554435 +// OBJ-NEXT: - Index: 1 +// OBJ-NEXT: Type: I32 +// OBJ-NEXT: Mutable: false +// OBJ-NEXT: InitExpr: +// OBJ-NEXT: Opcode: I32_CONST +// OBJ-NEXT: Value: 42 diff --git a/lld/test/wasm/global-init.ll b/lld/test/wasm/global-init.ll new file mode 100644 index 0000000000000..b6f1be2682dce --- /dev/null +++ b/lld/test/wasm/global-init.ll @@ -0,0 +1,26 @@ +; RUN: llc -filetype=obj -mtriple=wasm32-unknown-unknown %s -o %t.o +; RUN: wasm-ld --no-entry --export=use --no-gc-sections %t.o -o %t.wasm +; RUN: obj2yaml %t.wasm | FileCheck %s + +; The constant initializer of an address-space-1 Wasm global must survive +; linking, and the global's name must appear in the "name" custom section. + +@gv = hidden addrspace(1) global i32 33554435 +define i32 @use() { + %v = load i32, ptr addrspace(1) @gv + ret i32 %v +} + +; CHECK: - Type: GLOBAL +; CHECK: - Index: 1 +; CHECK-NEXT: Type: I32 +; CHECK-NEXT: Mutable: true +; CHECK-NEXT: InitExpr: +; CHECK-NEXT: Opcode: I32_CONST +; CHECK-NEXT: Value: 33554435 + +; CHECK: - Type: CUSTOM +; CHECK: Name: name +; CHECK: GlobalNames: +; CHECK: - Index: 1 +; CHECK: Name: gv diff --git a/llvm/include/llvm/MC/MCSymbolWasm.h b/llvm/include/llvm/MC/MCSymbolWasm.h index 5c9f14e5e6d64..e18dbc29ac786 100644 --- a/llvm/include/llvm/MC/MCSymbolWasm.h +++ b/llvm/include/llvm/MC/MCSymbolWasm.h @@ -28,6 +28,7 @@ class MCSymbolWasm : public MCSymbol { wasm::WasmSignature *Signature = nullptr; std::optional<wasm::WasmGlobalType> GlobalType; std::optional<wasm::WasmTableType> TableType; + std::optional<uint64_t> GlobalInitValue; /// An expression describing how to calculate the size of a symbol. If a /// symbol has no size this field will be NULL. @@ -138,6 +139,13 @@ class MCSymbolWasm : public MCSymbol { } void setGlobalType(wasm::WasmGlobalType GT) { GlobalType = GT; } + bool hasGlobalInitValue() const { return GlobalInitValue.has_value(); } + uint64_t getGlobalInitValue() const { + assert(GlobalInitValue); + return *GlobalInitValue; + } + void setGlobalInitValue(uint64_t V) { GlobalInitValue = V; } + bool hasTableType() const { return TableType.has_value(); } const wasm::WasmTableType &getTableType() const { assert(hasTableType()); diff --git a/llvm/lib/MC/WasmObjectWriter.cpp b/llvm/lib/MC/WasmObjectWriter.cpp index 640b263fd7bc2..8e9154cd107af 100644 --- a/llvm/lib/MC/WasmObjectWriter.cpp +++ b/llvm/lib/MC/WasmObjectWriter.cpp @@ -919,16 +919,16 @@ void WasmObjectWriter::writeGlobalSection(ArrayRef<wasm::WasmGlobal> Globals) { W->OS << char(Global.InitExpr.Inst.Opcode); switch (Global.Type.Type) { case wasm::WASM_TYPE_I32: - encodeSLEB128(0, W->OS); + encodeSLEB128(Global.InitExpr.Inst.Value.Int32, W->OS); break; case wasm::WASM_TYPE_I64: - encodeSLEB128(0, W->OS); + encodeSLEB128(Global.InitExpr.Inst.Value.Int64, W->OS); break; case wasm::WASM_TYPE_F32: - writeI32(0); + writeI32(Global.InitExpr.Inst.Value.Float32); break; case wasm::WASM_TYPE_F64: - writeI64(0); + writeI64(Global.InitExpr.Inst.Value.Float64); break; case wasm::WASM_TYPE_EXTERNREF: writeValueType(wasm::ValType::EXTERNREF); @@ -1652,18 +1652,24 @@ uint64_t WasmObjectWriter::writeOneObject(MCAssembler &Asm, Global.Type = WS.getGlobalType(); Global.Index = NumGlobalImports + Globals.size(); Global.InitExpr.Extended = false; + uint64_t Bits = WS.hasGlobalInitValue() ? WS.getGlobalInitValue() : 0; switch (Global.Type.Type) { case wasm::WASM_TYPE_I32: Global.InitExpr.Inst.Opcode = wasm::WASM_OPCODE_I32_CONST; + Global.InitExpr.Inst.Value.Int32 = + static_cast<int32_t>(static_cast<uint32_t>(Bits)); break; case wasm::WASM_TYPE_I64: Global.InitExpr.Inst.Opcode = wasm::WASM_OPCODE_I64_CONST; + Global.InitExpr.Inst.Value.Int64 = static_cast<int64_t>(Bits); break; case wasm::WASM_TYPE_F32: Global.InitExpr.Inst.Opcode = wasm::WASM_OPCODE_F32_CONST; + Global.InitExpr.Inst.Value.Float32 = static_cast<uint32_t>(Bits); break; case wasm::WASM_TYPE_F64: Global.InitExpr.Inst.Opcode = wasm::WASM_OPCODE_F64_CONST; + Global.InitExpr.Inst.Value.Float64 = Bits; break; case wasm::WASM_TYPE_EXTERNREF: Global.InitExpr.Inst.Opcode = wasm::WASM_OPCODE_REF_NULL; diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp index c82743834311d..f5a3300350b8d 100644 --- a/llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp +++ b/llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp @@ -41,6 +41,7 @@ #include "llvm/CodeGen/MachineModuleInfoImpls.h" #include "llvm/CodeGen/MachinePassManager.h" #include "llvm/IR/Analysis.h" +#include "llvm/IR/Constants.h" #include "llvm/IR/DataLayout.h" #include "llvm/IR/DebugInfoMetadata.h" #include "llvm/IR/GlobalVariable.h" @@ -185,6 +186,26 @@ MCSymbolWasm *WebAssemblyAsmPrinter::getMCSymbolForFunction( return WasmSym; } +static std::optional<uint64_t> getWasmGlobalInitBits(const Constant *C) { + // Sub-i32 ints and half/bfloat legalize to a single i32/f32 MVT and so pass + // wasmSymbolSetType(), but their bits are not an i32/f32 init value; gate on + // the exact IR type so they fall back to the default 0 rather than emit a + // garbage init expr. + if (const auto *CI = dyn_cast<ConstantInt>(C)) { + unsigned BW = CI->getType()->getIntegerBitWidth(); + if (BW == 32 || BW == 64) + return CI->getValue().getZExtValue(); + return std::nullopt; + } + if (const auto *CF = dyn_cast<ConstantFP>(C)) { + Type *T = CF->getType(); + if (T->isFloatTy() || T->isDoubleTy()) + return CF->getValueAPF().bitcastToAPInt().getZExtValue(); + return std::nullopt; + } + return std::nullopt; +} + void WebAssemblyAsmPrinter::emitGlobalVariable(const GlobalVariable *GV) { if (GV->hasCommonLinkage()) { OutContext.reportError(SMLoc(), @@ -217,13 +238,20 @@ void WebAssemblyAsmPrinter::emitGlobalVariable(const GlobalVariable *GV) { } emitVisibility(Sym, GV->getVisibility(), !GV->isDeclaration()); + + // WasmObjectWriter reads these init bits back off the symbol to emit the init + // expression instead of the type's default 0. + if (GV->hasInitializer() && Sym->isGlobal()) { + if (std::optional<uint64_t> Bits = + getWasmGlobalInitBits(GV->getInitializer())) + Sym->setGlobalInitValue(*Bits); + } + emitSymbolType(Sym); if (GV->hasInitializer()) { assert(getSymbolPreferLocal(*GV) == Sym); emitLinkage(GV, Sym); OutStreamer->emitLabel(Sym); - // TODO: Actually emit the initializer value. Otherwise the global has the - // default value for its type (0, ref.null, etc). OutStreamer->addBlankLine(); } } diff --git a/llvm/test/CodeGen/WebAssembly/global-init.ll b/llvm/test/CodeGen/WebAssembly/global-init.ll new file mode 100644 index 0000000000000..72e5c180c649c --- /dev/null +++ b/llvm/test/CodeGen/WebAssembly/global-init.ll @@ -0,0 +1,44 @@ +; RUN: llc < %s -mtriple=wasm32-unknown-unknown -filetype=obj | obj2yaml | FileCheck %s + +; No function is defined here, so the AsmPrinter's Subtarget is null: emitting +; these address-space-1 globals must not crash, and each initializer must reach +; the Global section instead of the type's default 0. + +@i32v = addrspace(1) global i32 33554435 +@i64v = addrspace(1) global i64 8589934595 +@f32v = addrspace(1) global float 2.0 +@f64v = addrspace(1) global double 2.0 +@constv = addrspace(1) constant i32 42 + +; CHECK: - Type: GLOBAL +; CHECK-NEXT: Globals: +; CHECK-NEXT: - Index: 0 +; CHECK-NEXT: Type: I32 +; CHECK-NEXT: Mutable: true +; CHECK-NEXT: InitExpr: +; CHECK-NEXT: Opcode: I32_CONST +; CHECK-NEXT: Value: 33554435 +; CHECK-NEXT: - Index: 1 +; CHECK-NEXT: Type: I64 +; CHECK-NEXT: Mutable: true +; CHECK-NEXT: InitExpr: +; CHECK-NEXT: Opcode: I64_CONST +; CHECK-NEXT: Value: 8589934595 +; CHECK-NEXT: - Index: 2 +; CHECK-NEXT: Type: F32 +; CHECK-NEXT: Mutable: true +; CHECK-NEXT: InitExpr: +; CHECK-NEXT: Opcode: F32_CONST +; CHECK-NEXT: Value: 1073741824 +; CHECK-NEXT: - Index: 3 +; CHECK-NEXT: Type: F64 +; CHECK-NEXT: Mutable: true +; CHECK-NEXT: InitExpr: +; CHECK-NEXT: Opcode: F64_CONST +; CHECK-NEXT: Value: 4611686018427387904 +; CHECK-NEXT: - Index: 4 +; CHECK-NEXT: Type: I32 +; CHECK-NEXT: Mutable: false +; CHECK-NEXT: InitExpr: +; CHECK-NEXT: Opcode: I32_CONST +; CHECK-NEXT: Value: 42 `````````` </details> https://github.com/llvm/llvm-project/pull/212007 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
