From 1b6d879ec1fc4e883d0f1f580bbe9230215d9e80 Mon Sep 17 00:00:00 2001 From: Sam Clegg Date: Sun, 6 Dec 2020 21:13:55 -0800 Subject: [PATCH] [WebAssembly] Fix code generated for atomic operations in PIC mode The main this this test does is to add the `IsNotPIC` predicate to the all the atomic instructions pattern that directly refer to `tglobaladdr`. This is because in PIC mode we need to generate separate instruction sequence (either a direct global.get, or __memory_base + offset) for accessing global addresses. As part of this change I noticed that many of the `Requires` attributes added to the instruction in `WebAssemblyInstrAtomics.td` were being honored. This is because the wrapped in a `let Predicates = [HasAtomics]` block and it seems that that outer wrapping overrides any `Requires` on defs within it. As a workaround I removed the outer `let` and added `HasAtomics` to all the inner `Requires`. I believe that all the instrucitons that don't have `Requires` explicit bottom out in `ATOMIC_I` and `ATOMIC_NRI` which have `HasAtomics` so this should not remove this predicate from any patterns (at least that is the idea). The alternative to this approach looks like implementing something like `PredicateControl` in `Mips.td` where we can split the predicates into groups so they don't clobber each other. Differential Revision: https://reviews.llvm.org/D92744 --- .../Target/WebAssembly/WebAssemblyInstrAtomics.td | 92 +++++++++------------- llvm/test/CodeGen/WebAssembly/atomic-pic.ll | 34 ++++++++ llvm/test/CodeGen/WebAssembly/offset-atomics.ll | 2 +- 3 files changed, 73 insertions(+), 55 deletions(-) create mode 100644 llvm/test/CodeGen/WebAssembly/atomic-pic.ll diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyInstrAtomics.td b/llvm/lib/Target/WebAssembly/WebAssemblyInstrAtomics.td index 3165685..22103b0 100644 --- a/llvm/lib/Target/WebAssembly/WebAssemblyInstrAtomics.td +++ b/llvm/lib/Target/WebAssembly/WebAssemblyInstrAtomics.td @@ -77,16 +77,15 @@ defm MEMORY_ATOMIC_WAIT64_A64 : } // mayLoad = 1 } // hasSideEffects = 1 -let Predicates = [HasAtomics] in { // Select notifys with no constant offset. def NotifyPatNoOffset_A32 : Pat<(i32 (int_wasm_memory_atomic_notify I32:$addr, I32:$count)), (MEMORY_ATOMIC_NOTIFY_A32 0, 0, I32:$addr, I32:$count)>, - Requires<[HasAddr32]>; + Requires<[HasAddr32, HasAtomics]>; def NotifyPatNoOffset_A64 : Pat<(i32 (int_wasm_memory_atomic_notify I64:$addr, I32:$count)), (MEMORY_ATOMIC_NOTIFY_A64 0, 0, I64:$addr, I32:$count)>, - Requires<[HasAddr64]>; + Requires<[HasAddr64, HasAtomics]>; // Select notifys with a constant offset. @@ -95,11 +94,11 @@ multiclass NotifyPatImmOff { def : Pat<(i32 (int_wasm_memory_atomic_notify (operand I32:$addr, imm:$off), I32:$count)), (!cast(inst#_A32) 0, imm:$off, I32:$addr, I32:$count)>, - Requires<[HasAddr32]>; + Requires<[HasAddr32, HasAtomics]>; def : Pat<(i32 (int_wasm_memory_atomic_notify (operand I64:$addr, imm:$off), I32:$count)), (!cast(inst#_A64) 0, imm:$off, I64:$addr, I32:$count)>, - Requires<[HasAddr64]>; + Requires<[HasAddr64, HasAtomics]>; } defm : NotifyPatImmOff; defm : NotifyPatImmOff; @@ -108,34 +107,34 @@ defm : NotifyPatImmOff; def NotifyPatOffsetOnly_A32 : Pat<(i32 (int_wasm_memory_atomic_notify imm:$off, I32:$count)), (MEMORY_ATOMIC_NOTIFY_A32 0, imm:$off, (CONST_I32 0), I32:$count)>, - Requires<[HasAddr32]>; + Requires<[HasAddr32, HasAtomics]>; def NotifyPatOffsetOnly_A64 : Pat<(i32 (int_wasm_memory_atomic_notify imm:$off, I32:$count)), (MEMORY_ATOMIC_NOTIFY_A64 0, imm:$off, (CONST_I64 0), I32:$count)>, - Requires<[HasAddr64]>; + Requires<[HasAddr64, HasAtomics]>; def NotifyPatGlobalAddrOffOnly_A32 : Pat<(i32 (int_wasm_memory_atomic_notify (WebAssemblywrapper tglobaladdr:$off), I32:$count)), (MEMORY_ATOMIC_NOTIFY_A32 0, tglobaladdr:$off, (CONST_I32 0), I32:$count) >, - Requires<[HasAddr32]>; + Requires<[HasAddr32, HasAtomics, IsNotPIC]>; def NotifyPatGlobalAddrOffOnly_A64 : Pat<(i32 (int_wasm_memory_atomic_notify (WebAssemblywrapper tglobaladdr:$off), I32:$count)), (MEMORY_ATOMIC_NOTIFY_A64 0, tglobaladdr:$off, (CONST_I64 0), I32:$count) >, - Requires<[HasAddr64]>; + Requires<[HasAddr64, HasAtomics, IsNotPIC]>; // Select waits with no constant offset. multiclass WaitPatNoOffset { def : Pat<(i32 (kind I32:$addr, ty:$exp, I64:$timeout)), (!cast(inst#_A32) 0, 0, I32:$addr, ty:$exp, I64:$timeout)>, - Requires<[HasAddr32]>; + Requires<[HasAddr32, HasAtomics]>; def : Pat<(i32 (kind I64:$addr, ty:$exp, I64:$timeout)), (!cast(inst#_A64) 0, 0, I64:$addr, ty:$exp, I64:$timeout)>, - Requires<[HasAddr64]>; + Requires<[HasAddr64, HasAtomics]>; } defm : WaitPatNoOffset; @@ -154,11 +153,11 @@ multiclass WaitPatImmOff(inst#_A32) 0, imm:$off, I32:$addr, ty:$exp, I64:$timeout)>, - Requires<[HasAddr32]>; + Requires<[HasAddr32, HasAtomics]>; def : Pat<(i32 (kind (operand I64:$addr, imm:$off), ty:$exp, I64:$timeout)), (!cast(inst#_A64) 0, imm:$off, I64:$addr, ty:$exp, I64:$timeout)>, - Requires<[HasAddr64]>; + Requires<[HasAddr64, HasAtomics]>; } defm : WaitPatImmOff; @@ -174,11 +173,11 @@ multiclass WaitPatOffsetOnly { def : Pat<(i32 (kind imm:$off, ty:$exp, I64:$timeout)), (!cast(inst#_A32) 0, imm:$off, (CONST_I32 0), ty:$exp, I64:$timeout)>, - Requires<[HasAddr32]>; + Requires<[HasAddr32, HasAtomics]>; def : Pat<(i32 (kind imm:$off, ty:$exp, I64:$timeout)), (!cast(inst#_A64) 0, imm:$off, (CONST_I64 0), ty:$exp, I64:$timeout)>, - Requires<[HasAddr64]>; + Requires<[HasAddr64, HasAtomics]>; } defm : WaitPatOffsetOnly; @@ -190,18 +189,17 @@ multiclass WaitPatGlobalAddrOffOnly { I64:$timeout)), (!cast(inst#_A32) 0, tglobaladdr:$off, (CONST_I32 0), ty:$exp, I64:$timeout)>, - Requires<[HasAddr32]>; + Requires<[HasAddr32, HasAtomics, IsNotPIC]>; def : Pat<(i32 (kind (WebAssemblywrapper tglobaladdr:$off), ty:$exp, I64:$timeout)), (!cast(inst#_A64) 0, tglobaladdr:$off, (CONST_I64 0), ty:$exp, I64:$timeout)>, - Requires<[HasAddr64]>; + Requires<[HasAddr64, HasAtomics, IsNotPIC]>; } defm : WaitPatGlobalAddrOffOnly; defm : WaitPatGlobalAddrOffOnly; -} // Predicates = [HasAtomics] //===----------------------------------------------------------------------===// // Atomic fences @@ -229,7 +227,6 @@ defm ATOMIC_LOAD_I32 : AtomicLoad; defm ATOMIC_LOAD_I64 : AtomicLoad; // Select loads with no constant offset. -let Predicates = [HasAtomics] in { defm : LoadPatNoOffset; defm : LoadPatNoOffset; @@ -248,7 +245,6 @@ defm : LoadPatOffsetOnly; defm : LoadPatGlobalAddrOffOnly; defm : LoadPatGlobalAddrOffOnly; -} // Predicates = [HasAtomics] // Extending loads. Note that there are only zero-extending atomic loads, no // sign-extending loads. @@ -293,7 +289,6 @@ def sext_aload_8_64 : def sext_aload_16_64 : PatFrag<(ops node:$addr), (anyext (i32 (atomic_load_16 node:$addr)))>; -let Predicates = [HasAtomics] in { // Select zero-extending loads with no constant offset. defm : LoadPatNoOffset; defm : LoadPatNoOffset; @@ -352,7 +347,6 @@ defm : LoadPatGlobalAddrOffOnly; defm : LoadPatGlobalAddrOffOnly; defm : LoadPatGlobalAddrOffOnly; -} // Predicates = [HasAtomics] //===----------------------------------------------------------------------===// // Atomic stores @@ -371,16 +365,15 @@ defm ATOMIC_STORE_I64 : AtomicStore; // store: (store $val, $ptr) // atomic_store: (store $ptr, $val) -let Predicates = [HasAtomics] in { // Select stores with no constant offset. multiclass AStorePatNoOffset { def : Pat<(kind I32:$addr, ty:$val), (!cast(inst#_A32) 0, 0, I32:$addr, ty:$val)>, - Requires<[HasAddr32]>; + Requires<[HasAddr32, HasAtomics]>; def : Pat<(kind I64:$addr, ty:$val), (!cast(inst#_A64) 0, 0, I64:$addr, ty:$val)>, - Requires<[HasAddr64]>; + Requires<[HasAddr64, HasAtomics]>; } defm : AStorePatNoOffset; defm : AStorePatNoOffset; @@ -392,10 +385,10 @@ multiclass AStorePatImmOff { def : Pat<(kind (operand I32:$addr, imm:$off), ty:$val), (!cast(inst#_A32) 0, imm:$off, I32:$addr, ty:$val)>, - Requires<[HasAddr32]>; + Requires<[HasAddr32, HasAtomics]>; def : Pat<(kind (operand I64:$addr, imm:$off), ty:$val), (!cast(inst#_A64) 0, imm:$off, I64:$addr, ty:$val)>, - Requires<[HasAddr64]>; + Requires<[HasAddr64, HasAtomics]>; } defm : AStorePatImmOff; defm : AStorePatImmOff; @@ -404,10 +397,10 @@ defm : AStorePatImmOff; multiclass AStorePatOffsetOnly { def : Pat<(kind imm:$off, ty:$val), (!cast(inst#_A32) 0, imm:$off, (CONST_I32 0), ty:$val)>, - Requires<[HasAddr32]>; + Requires<[HasAddr32, HasAtomics]>; def : Pat<(kind imm:$off, ty:$val), (!cast(inst#_A64) 0, imm:$off, (CONST_I64 0), ty:$val)>, - Requires<[HasAddr64]>; + Requires<[HasAddr64, HasAtomics]>; } defm : AStorePatOffsetOnly; defm : AStorePatOffsetOnly; @@ -415,15 +408,14 @@ defm : AStorePatOffsetOnly; multiclass AStorePatGlobalAddrOffOnly { def : Pat<(kind (WebAssemblywrapper tglobaladdr:$off), ty:$val), (!cast(inst#_A32) 0, tglobaladdr:$off, (CONST_I32 0), ty:$val)>, - Requires<[HasAddr32]>; + Requires<[HasAddr32, HasAtomics, IsNotPIC]>; def : Pat<(kind (WebAssemblywrapper tglobaladdr:$off), ty:$val), (!cast(inst#_A64) 0, tglobaladdr:$off, (CONST_I64 0), ty:$val)>, - Requires<[HasAddr64]>; + Requires<[HasAddr64, HasAtomics, IsNotPIC]>; } defm : AStorePatGlobalAddrOffOnly; defm : AStorePatGlobalAddrOffOnly; -} // Predicates = [HasAtomics] // Truncating stores. defm ATOMIC_STORE8_I32 : AtomicStore; @@ -444,7 +436,6 @@ def trunc_astore_8_64 : trunc_astore_64; def trunc_astore_16_64 : trunc_astore_64; def trunc_astore_32_64 : trunc_astore_64; -let Predicates = [HasAtomics] in { // Truncating stores with no constant offset defm : AStorePatNoOffset; @@ -482,7 +473,6 @@ defm : AStorePatGlobalAddrOffOnly; defm : AStorePatGlobalAddrOffOnly; defm : AStorePatGlobalAddrOffOnly; -} // Predicates = [HasAtomics] //===----------------------------------------------------------------------===// // Atomic binary read-modify-writes @@ -588,10 +578,10 @@ defm ATOMIC_RMW32_U_XCHG_I64 : multiclass BinRMWPatNoOffset { def : Pat<(ty (kind I32:$addr, ty:$val)), (!cast(inst#_A32) 0, 0, I32:$addr, ty:$val)>, - Requires<[HasAddr32]>; + Requires<[HasAddr32, HasAtomics]>; def : Pat<(ty (kind I64:$addr, ty:$val)), (!cast(inst#_A64) 0, 0, I64:$addr, ty:$val)>, - Requires<[HasAddr64]>; + Requires<[HasAddr64, HasAtomics]>; } // Select binary RMWs with a constant offset. @@ -601,29 +591,29 @@ multiclass BinRMWPatImmOff { def : Pat<(ty (kind (operand I32:$addr, imm:$off), ty:$val)), (!cast(inst#_A32) 0, imm:$off, I32:$addr, ty:$val)>, - Requires<[HasAddr32]>; + Requires<[HasAddr32, HasAtomics]>; def : Pat<(ty (kind (operand I64:$addr, imm:$off), ty:$val)), (!cast(inst#_A64) 0, imm:$off, I64:$addr, ty:$val)>, - Requires<[HasAddr64]>; + Requires<[HasAddr64, HasAtomics]>; } // Select binary RMWs with just a constant offset. multiclass BinRMWPatOffsetOnly { def : Pat<(ty (kind imm:$off, ty:$val)), (!cast(inst#_A32) 0, imm:$off, (CONST_I32 0), ty:$val)>, - Requires<[HasAddr32]>; + Requires<[HasAddr32, HasAtomics]>; def : Pat<(ty (kind imm:$off, ty:$val)), (!cast(inst#_A64) 0, imm:$off, (CONST_I64 0), ty:$val)>, - Requires<[HasAddr64]>; + Requires<[HasAddr64, HasAtomics]>; } multiclass BinRMWPatGlobalAddrOffOnly { def : Pat<(ty (kind (WebAssemblywrapper tglobaladdr:$off), ty:$val)), (!cast(inst#_A32) 0, tglobaladdr:$off, (CONST_I32 0), ty:$val)>, - Requires<[HasAddr32]>; + Requires<[HasAddr32, HasAtomics, IsNotPIC]>; def : Pat<(ty (kind (WebAssemblywrapper tglobaladdr:$off), ty:$val)), (!cast(inst#_A64) 0, tglobaladdr:$off, (CONST_I64 0), ty:$val)>, - Requires<[HasAddr64]>; + Requires<[HasAddr64, HasAtomics, IsNotPIC]>; } // Patterns for various addressing modes. @@ -644,7 +634,6 @@ multiclass BinRMWPattern; } -let Predicates = [HasAtomics] in { defm : BinRMWPattern; defm : BinRMWPattern; defm : BinRMWPattern; -} // Predicates = [HasAtomics] // Truncating & zero-extending binary RMW patterns. // These are combined patterns of truncating store patterns and zero-extending @@ -760,7 +748,6 @@ multiclass BinRMWTruncExtPattern< defm : BinRMWPatGlobalAddrOffOnly, inst16_64>; } -let Predicates = [HasAtomics] in { defm : BinRMWTruncExtPattern< atomic_load_add_8, atomic_load_add_16, atomic_load_add_32, atomic_load_add_64, "ATOMIC_RMW8_U_ADD_I32", "ATOMIC_RMW16_U_ADD_I32", @@ -786,7 +773,6 @@ defm : BinRMWTruncExtPattern< "ATOMIC_RMW8_U_XCHG_I32", "ATOMIC_RMW16_U_XCHG_I32", "ATOMIC_RMW8_U_XCHG_I64", "ATOMIC_RMW16_U_XCHG_I64", "ATOMIC_RMW32_U_XCHG_I64">; -} // Predicates = [HasAtomics] //===----------------------------------------------------------------------===// // Atomic ternary read-modify-writes @@ -835,10 +821,10 @@ defm ATOMIC_RMW32_U_CMPXCHG_I64 : multiclass TerRMWPatNoOffset { def : Pat<(ty (kind I32:$addr, ty:$exp, ty:$new)), (!cast(inst#_A32) 0, 0, I32:$addr, ty:$exp, ty:$new)>, - Requires<[HasAddr32]>; + Requires<[HasAddr32, HasAtomics]>; def : Pat<(ty (kind I64:$addr, ty:$exp, ty:$new)), (!cast(inst#_A64) 0, 0, I64:$addr, ty:$exp, ty:$new)>, - Requires<[HasAddr64]>; + Requires<[HasAddr64, HasAtomics]>; } // Select ternary RMWs with a constant offset. @@ -848,10 +834,10 @@ multiclass TerRMWPatImmOff { def : Pat<(ty (kind (operand I32:$addr, imm:$off), ty:$exp, ty:$new)), (!cast(inst#_A32) 0, imm:$off, I32:$addr, ty:$exp, ty:$new)>, - Requires<[HasAddr32]>; + Requires<[HasAddr32, HasAtomics]>; def : Pat<(ty (kind (operand I64:$addr, imm:$off), ty:$exp, ty:$new)), (!cast(inst#_A64) 0, imm:$off, I64:$addr, ty:$exp, ty:$new)>, - Requires<[HasAddr64]>; + Requires<[HasAddr64, HasAtomics]>; } // Select ternary RMWs with just a constant offset. @@ -868,11 +854,11 @@ multiclass TerRMWPatGlobalAddrOffOnly { def : Pat<(ty (kind (WebAssemblywrapper tglobaladdr:$off), ty:$exp, ty:$new)), (!cast(inst#_A32) 0, tglobaladdr:$off, (CONST_I32 0), ty:$exp, ty:$new)>, - Requires<[HasAddr32]>; + Requires<[HasAddr32, HasAtomics, IsNotPIC]>; def : Pat<(ty (kind (WebAssemblywrapper tglobaladdr:$off), ty:$exp, ty:$new)), (!cast(inst#_A64) 0, tglobaladdr:$off, (CONST_I64 0), ty:$exp, ty:$new)>, - Requires<[HasAddr64]>; + Requires<[HasAddr64, HasAtomics, IsNotPIC]>; } // Patterns for various addressing modes. @@ -893,7 +879,6 @@ multiclass TerRMWPattern; } -let Predicates = [HasAtomics] in defm : TerRMWPattern; @@ -1002,7 +987,6 @@ multiclass TerRMWTruncExtPattern< defm : TerRMWPatGlobalAddrOffOnly, inst16_64>; } -let Predicates = [HasAtomics] in defm : TerRMWTruncExtPattern< atomic_cmp_swap_8, atomic_cmp_swap_16, atomic_cmp_swap_32, atomic_cmp_swap_64, "ATOMIC_RMW8_U_CMPXCHG_I32", "ATOMIC_RMW16_U_CMPXCHG_I32", diff --git a/llvm/test/CodeGen/WebAssembly/atomic-pic.ll b/llvm/test/CodeGen/WebAssembly/atomic-pic.ll new file mode 100644 index 0000000..2c4f922 --- /dev/null +++ b/llvm/test/CodeGen/WebAssembly/atomic-pic.ll @@ -0,0 +1,34 @@ +; RUN: llc < %s -asm-verbose=false -relocation-model=pic -fast-isel -wasm-disable-explicit-locals -wasm-keep-registers -mattr=+atomics,+sign-ext | FileCheck %s +; RUN: llc < %s -asm-verbose=false -relocation-model=pic -fast-isel=false -wasm-disable-explicit-locals -wasm-keep-registers -mattr=+atomics,+sign-ext | FileCheck %s + +; Test that atomic operations in PIC mode. Specifically we verify +; that atomic operations on global address load addres via @GOT or +; @MBREL relocations. + +target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128" +target triple = "wasm32-unknown-emscripten" + +@external_global = external global i32 +@hidden_global = external hidden global i32 + +define i32 @rmw_add_external_global() { +; CHECK-LABEL: rmw_add_external_global: +; CHECK: global.get $push[[L0:[0-9]+]]=, external_global@GOT{{$}} +; CHECK-NEXT: i32.const $push[[L1:[0-9]+]]=, 42{{$}} +; CHECK-NEXT: i32.atomic.rmw.add $push[[L2:[0-9]+]]=, 0($pop[[L0]]), $pop[[L1]]{{$}} +; CHECK-NEXT: end_function + %1 = atomicrmw add i32* @external_global, i32 42 seq_cst + ret i32 %1 +} + +define i32 @rmw_add_hidden_global() { +; CHECK-LABEL: rmw_add_hidden_global: +; CHECK: global.get $push[[L0:[0-9]+]]=, __memory_base{{$}} +; CHECK-NEXT: i32.const $push[[L1:[0-9]+]]=, hidden_global@MBREL{{$}} +; CHECK-NEXT: i32.add $push[[L2:[0-9]+]]=, $pop[[L0]], $pop[[L1]]{{$}} +; CHECK-NEXT: i32.const $push[[L3:[0-9]+]]=, 42{{$}} +; CHECK-NEXT: i32.atomic.rmw.add $push[[L4:[0-9]+]]=, 0($pop[[L2]]), $pop[[L3]]{{$}} +; CHECK-NEXT: end_function + %1 = atomicrmw add i32* @hidden_global, i32 42 seq_cst + ret i32 %1 +} diff --git a/llvm/test/CodeGen/WebAssembly/offset-atomics.ll b/llvm/test/CodeGen/WebAssembly/offset-atomics.ll index 13b07d3..27c2aea 100644 --- a/llvm/test/CodeGen/WebAssembly/offset-atomics.ll +++ b/llvm/test/CodeGen/WebAssembly/offset-atomics.ll @@ -1,4 +1,4 @@ -; RUN: not --crash llc < %s -asm-verbose=false -disable-wasm-fallthrough-return-opt +; RUN: not --crash llc > /dev/null < %s -asm-verbose=false -disable-wasm-fallthrough-return-opt ; RUN: llc < %s -asm-verbose=false -disable-wasm-fallthrough-return-opt -wasm-disable-explicit-locals -wasm-keep-registers -mattr=+atomics,+sign-ext | FileCheck %s ; Test that atomic loads are assembled properly. -- 2.7.4