[WebAssembly] Fix for PIC external symbol ISEL
authorWouter van Oortmerssen <aardappel@gmail.com>
Thu, 25 Mar 2021 23:04:10 +0000 (16:04 -0700)
committerWouter van Oortmerssen <aardappel@gmail.com>
Thu, 8 Apr 2021 19:07:38 +0000 (12:07 -0700)
wasm64 was missing DAG ISEL patterns for external symbol based global.get, but simply adding these analogous to the existing 32-bit versions doesn't work.
This is because we are conflating the 32-bit global index with the pointer represented by the external symbol, which for wasm32 happened to work.
The simplest fix is to pretend we have a 64-bit global index. This sounds incorrect, but is immaterial since once this index is stored as a MachineOperand it becomes 64-bit anyway (and has been all along). As such, the EmitInstrWithCustomInserter based implementation I experimented with become a no-op and no further changes in the C++ code are required.

Differential Revision: https://reviews.llvm.org/D99904

llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td
llvm/test/CodeGen/WebAssembly/load-store-pic.ll

index cf897a9..241112d 100644 (file)
@@ -130,8 +130,14 @@ def bb_op : Operand<OtherVT>;
 let OperandType = "OPERAND_LOCAL" in
 def local_op : Operand<i32>;
 
-let OperandType = "OPERAND_GLOBAL" in
-def global_op : Operand<i32>;
+let OperandType = "OPERAND_GLOBAL" in {
+  // The operand to global instructions is always a 32-bit index.
+  def global_op32 : Operand<i32>;
+  // In PIC mode however, we temporarily represent this index as an external
+  // symbol, which to LLVM is a pointer, so in wasm64 mode it is easiest to
+  // pretend we use a 64-bit index for it.
+  def global_op64 : Operand<i64>;
+}
 
 let OperandType = "OPERAND_I32IMM" in
 def i32imm_op : Operand<i32>;
@@ -251,7 +257,7 @@ defm "": ARGUMENT<EXTERNREF, externref>;
 
 // local.get and local.set are not generated by instruction selection; they
 // are implied by virtual register uses and defs.
-multiclass LOCAL<WebAssemblyRegClass vt> {
+multiclass LOCAL<WebAssemblyRegClass vt, Operand global_op> {
   let hasSideEffects = 0 in {
   // COPY is not an actual instruction in wasm, but since we allow local.get and
   // local.set to be implicit during most of codegen, we can have a COPY which
@@ -312,13 +318,13 @@ multiclass LOCAL<WebAssemblyRegClass vt> {
 
 } // hasSideEffects = 0
 }
-defm "" : LOCAL<I32>;
-defm "" : LOCAL<I64>;
-defm "" : LOCAL<F32>;
-defm "" : LOCAL<F64>;
-defm "" : LOCAL<V128>, Requires<[HasSIMD128]>;
-defm "" : LOCAL<FUNCREF>, Requires<[HasReferenceTypes]>;
-defm "" : LOCAL<EXTERNREF>, Requires<[HasReferenceTypes]>;
+defm "" : LOCAL<I32, global_op32>;
+defm "" : LOCAL<I64, global_op64>;  // 64-bit only needed for pointers.
+defm "" : LOCAL<F32, global_op32>;
+defm "" : LOCAL<F64, global_op32>;
+defm "" : LOCAL<V128, global_op32>, Requires<[HasSIMD128]>;
+defm "" : LOCAL<FUNCREF, global_op32>, Requires<[HasReferenceTypes]>;
+defm "" : LOCAL<EXTERNREF, global_op32>, Requires<[HasReferenceTypes]>;
 
 let isMoveImm = 1, isAsCheapAsAMove = 1, isReMaterializable = 1 in {
 defm CONST_I32 : I<(outs I32:$res), (ins i32imm_op:$imm),
@@ -346,6 +352,8 @@ def : Pat<(i64 (WebAssemblywrapper tglobaladdr:$addr)),
 
 def : Pat<(i32 (WebAssemblywrapper tglobaladdr:$addr)),
           (GLOBAL_GET_I32 tglobaladdr:$addr)>, Requires<[IsPIC, HasAddr32]>;
+def : Pat<(i64 (WebAssemblywrapper tglobaladdr:$addr)),
+          (GLOBAL_GET_I64 tglobaladdr:$addr)>, Requires<[IsPIC, HasAddr64]>;
 
 def : Pat<(i32 (WebAssemblywrapperPIC tglobaladdr:$addr)),
           (CONST_I32 tglobaladdr:$addr)>, Requires<[IsPIC, HasAddr32]>;
@@ -359,6 +367,8 @@ def : Pat<(i64 (WebAssemblywrapper tglobaltlsaddr:$addr)),
 
 def : Pat<(i32 (WebAssemblywrapper texternalsym:$addr)),
           (GLOBAL_GET_I32 texternalsym:$addr)>, Requires<[IsPIC, HasAddr32]>;
+def : Pat<(i64 (WebAssemblywrapper texternalsym:$addr)),
+          (GLOBAL_GET_I64 texternalsym:$addr)>, Requires<[IsPIC, HasAddr64]>;
 
 def : Pat<(i32 (WebAssemblywrapper texternalsym:$addr)),
           (CONST_I32 texternalsym:$addr)>, Requires<[IsNotPIC, HasAddr32]>;
index 5f09025..ffbc104 100644 (file)
@@ -1,12 +1,11 @@
-; RUN: llc < %s -asm-verbose=false -relocation-model=pic -fast-isel -wasm-disable-explicit-locals -wasm-keep-registers | FileCheck %s -check-prefixes=PIC,CHECK
-; RUN: llc < %s -asm-verbose=false -relocation-model=pic -fast-isel=false -wasm-disable-explicit-locals -wasm-keep-registers | FileCheck %s -check-prefixes=PIC,CHECK
+; RUN: llc < %s --mtriple=wasm32-unknown-emscripten -asm-verbose=false -relocation-model=pic -fast-isel -wasm-disable-explicit-locals -wasm-keep-registers | FileCheck %s -check-prefixes=PIC,CHECK -DPTR=i32
+; RUN: llc < %s --mtriple=wasm32-unknown-emscripten -asm-verbose=false -relocation-model=pic -fast-isel=false -wasm-disable-explicit-locals -wasm-keep-registers | FileCheck %s -check-prefixes=PIC,CHECK -DPTR=i32
+; RUN: llc < %s --mtriple=wasm64-unknown-emscripten -asm-verbose=false -relocation-model=pic -fast-isel -wasm-disable-explicit-locals -wasm-keep-registers | FileCheck %s -check-prefixes=PIC,CHECK -DPTR=i64
+; RUN: llc < %s --mtriple=wasm64-unknown-emscripten -asm-verbose=false -relocation-model=pic -fast-isel=false -wasm-disable-explicit-locals -wasm-keep-registers | FileCheck %s -check-prefixes=PIC,CHECK -DPTR=i64
 
 ; Test that globals assemble as expected with -fPIC.
 ; We test here both with and without fast-isel.
 
-target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
-target triple = "wasm32-unknown-emscripten"
-
 @hidden_global         = external hidden global i32
 @hidden_global_array   = external hidden global [10 x i32]
 @external_global       = external        global i32
@@ -20,8 +19,8 @@ declare i32 @foo();
 define i32 @load_hidden_global() {
 ; CHECK-LABEL: load_hidden_global:
 ; PIC:         global.get $push[[L0:[0-9]+]]=, __memory_base{{$}}
-; PIC-NEXT:    i32.const $push[[L1:[0-9]+]]=, hidden_global@MBREL{{$}}
-; PIC-NEXT:    i32.add $push[[L2:[0-9]+]]=, $pop[[L0]], $pop[[L1]]{{$}}
+; PIC-NEXT:    [[PTR]].const $push[[L1:[0-9]+]]=, hidden_global@MBREL{{$}}
+; PIC-NEXT:    [[PTR]].add $push[[L2:[0-9]+]]=, $pop[[L0]], $pop[[L1]]{{$}}
 ; PIC-NEXT:    i32.load $push[[L3:[0-9]+]]=, 0($pop[[L2]]){{$}}
 ; CHECK-NEXT:    end_function
 
@@ -32,10 +31,10 @@ define i32 @load_hidden_global() {
 define i32 @load_hidden_global_offset() {
 ; CHECK-LABEL: load_hidden_global_offset:
 ; PIC:         global.get $push[[L0:[0-9]+]]=, __memory_base{{$}}
-; PIC-NEXT:    i32.const $push[[L1:[0-9]+]]=, hidden_global_array@MBREL{{$}}
-; PIC-NEXT:    i32.add $push[[L2:[0-9]+]]=, $pop[[L0]], $pop[[L1:[0-9]+]]{{$}}
-; PIC-NEXT:    i32.const $push[[L3:[0-9]+]]=, 20{{$}}
-; PIC-NEXT:    i32.add $push[[L4:[0-9]+]]=, $pop[[L2]], $pop[[L3]]{{$}}
+; PIC-NEXT:    [[PTR]].const $push[[L1:[0-9]+]]=, hidden_global_array@MBREL{{$}}
+; PIC-NEXT:    [[PTR]].add $push[[L2:[0-9]+]]=, $pop[[L0]], $pop[[L1:[0-9]+]]{{$}}
+; PIC-NEXT:    [[PTR]].const $push[[L3:[0-9]+]]=, 20{{$}}
+; PIC-NEXT:    [[PTR]].add $push[[L4:[0-9]+]]=, $pop[[L2]], $pop[[L3]]{{$}}
 ; PIC-NEXT:    i32.load $push{{[0-9]+}}=, 0($pop[[L4]]){{$}}
 ; CHECK-NEXT:  end_function
 
@@ -49,8 +48,8 @@ define i32 @load_hidden_global_offset() {
 define void @store_hidden_global(i32 %n) {
 ; CHECK-LABEL: store_hidden_global:
 ; PIC:         global.get $push[[L0:[0-9]+]]=, __memory_base{{$}}
-; PIC-NEXT:    i32.const $push[[L1:[0-9]+]]=, hidden_global@MBREL{{$}}
-; PIC-NEXT:    i32.add $push[[L2:[0-9]+]]=, $pop[[L0]], $pop[[L1]]{{$}}
+; PIC-NEXT:    [[PTR]].const $push[[L1:[0-9]+]]=, hidden_global@MBREL{{$}}
+; PIC-NEXT:    [[PTR]].add $push[[L2:[0-9]+]]=, $pop[[L0]], $pop[[L1]]{{$}}
 ; PIC-NEXT:    i32.store 0($pop[[L2]]), $0{{$}}
 ; CHECK-NEXT:    end_function
 
@@ -61,10 +60,10 @@ define void @store_hidden_global(i32 %n) {
 define void @store_hidden_global_offset(i32 %n) {
 ; CHECK-LABEL: store_hidden_global_offset:
 ; PIC:         global.get $push[[L0:[0-9]+]]=, __memory_base{{$}}
-; PIC-NEXT:    i32.const $push[[L1:[0-9]+]]=, hidden_global_array@MBREL{{$}}
-; PIC-NEXT:    i32.add $push[[L2:[0-9]+]]=, $pop[[L0]], $pop[[L1]]{{$}}
-; PIC-NEXT:    i32.const $push[[L3:[0-9]+]]=, 20{{$}}
-; PIC-NEXT:    i32.add $push[[L4:[0-9]+]]=, $pop[[L2]], $pop[[L3]]{{$}}
+; PIC-NEXT:    [[PTR]].const $push[[L1:[0-9]+]]=, hidden_global_array@MBREL{{$}}
+; PIC-NEXT:    [[PTR]].add $push[[L2:[0-9]+]]=, $pop[[L0]], $pop[[L1]]{{$}}
+; PIC-NEXT:    [[PTR]].const $push[[L3:[0-9]+]]=, 20{{$}}
+; PIC-NEXT:    [[PTR]].add $push[[L4:[0-9]+]]=, $pop[[L2]], $pop[[L3]]{{$}}
 ; PIC-NEXT:    i32.store 0($pop[[L4]]), $0{{$}}
 
 ; CHECK-NEXT:   end_function
@@ -92,8 +91,8 @@ define i32 @load_external_global() {
 define i32 @load_external_global_offset() {
 ; CHECK-LABEL:  load_external_global_offset:
 ; PIC:          global.get $push[[L0:[0-9]+]]=, external_global_array@GOT{{$}}
-; PIC-NEXT:     i32.const $push[[L1:[0-9]+]]=, 20{{$}}
-; PIC-NEXT:     i32.add $push[[L2:[0-9]+]]=, $pop[[L0]], $pop[[L1]]{{$}}
+; PIC-NEXT:     [[PTR]].const $push[[L1:[0-9]+]]=, 20{{$}}
+; PIC-NEXT:     [[PTR]].add $push[[L2:[0-9]+]]=, $pop[[L0]], $pop[[L1]]{{$}}
 ; PIC-NEXT:     i32.load $push{{[0-9]+}}=, 0($pop[[L2]]){{$}}
 
 ; CHECK-NEXT:   end_function
@@ -119,8 +118,8 @@ define void @store_external_global(i32 %n) {
 define void @store_external_global_offset(i32 %n) {
 ; CHECK-LABEL:  store_external_global_offset:
 ; PIC:          global.get $push[[L0:[0-9]+]]=, external_global_array@GOT{{$}}
-; PIC-NEXT:     i32.const $push[[L1:[0-9]+]]=, 20{{$}}
-; PIC-NEXT:     i32.add $push[[L2:[0-9]+]]=, $pop[[L0]], $pop[[L1]]{{$}}
+; PIC-NEXT:     [[PTR]].const $push[[L1:[0-9]+]]=, 20{{$}}
+; PIC-NEXT:     [[PTR]].add $push[[L2:[0-9]+]]=, $pop[[L0]], $pop[[L1]]{{$}}
 ; PIC-NEXT:     i32.store 0($pop[[L2]]), $0{{$}}
 
 ; CHECK-NEXT:   end_function
@@ -130,4 +129,4 @@ define void @store_external_global_offset(i32 %n) {
   ret void
 }
 
-; PIC: .globaltype __memory_base, i32
+; PIC: .globaltype __memory_base, [[PTR]]