From 0e37487df8e030b9e468166fa3d088678bf7d0a1 Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Wed, 29 Mar 2023 12:40:30 -0700 Subject: [PATCH] [WebAssembly] Fix selection of global calls When selecting calls, currently we unconditionally remove `Wrapper`s of the call target. But we are supposed to do that only when the target is a function, an external symbol (= library function), or an alias of a function. Otherwise we end up directly calling globals that are not functions. Fixes https://github.com/llvm/llvm-project/issues/60003. Reviewed By: tlively, HerrCai0907 Differential Revision: https://reviews.llvm.org/D147397 --- .../Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp | 18 ++++++++- llvm/test/CodeGen/WebAssembly/call.ll | 44 +++++++++++++++++++++- 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp index df79e55..9aacddb 100644 --- a/llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp +++ b/llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp @@ -249,8 +249,22 @@ void WebAssemblyDAGToDAGISel::Select(SDNode *Node) { SmallVector Ops; for (size_t i = 1; i < Node->getNumOperands(); ++i) { SDValue Op = Node->getOperand(i); - if (i == 1 && Op->getOpcode() == WebAssemblyISD::Wrapper) - Op = Op->getOperand(0); + // Remove the wrapper when the call target is a function, an external + // symbol (which will be lowered to a library function), or an alias of + // a function. If the target is not a function/external symbol, we + // shouldn't remove the wrapper, because we cannot call it directly and + // instead we want it to be loaded with a CONST instruction and called + // with a call_indirect later. + if (i == 1 && Op->getOpcode() == WebAssemblyISD::Wrapper) { + SDValue NewOp = Op->getOperand(0); + if (auto *GlobalOp = dyn_cast(NewOp.getNode())) { + if (isa( + GlobalOp->getGlobal()->stripPointerCastsAndAliases())) + Op = NewOp; + } else if (isa(NewOp.getNode())) { + Op = NewOp; + } + } Ops.push_back(Op); } diff --git a/llvm/test/CodeGen/WebAssembly/call.ll b/llvm/test/CodeGen/WebAssembly/call.ll index 0ae1adc..2f7658a 100644 --- a/llvm/test/CodeGen/WebAssembly/call.ll +++ b/llvm/test/CodeGen/WebAssembly/call.ll @@ -1,5 +1,5 @@ -; RUN: llc < %s -asm-verbose=false -disable-wasm-fallthrough-return-opt -wasm-keep-registers -mattr=+sign-ext,+simd128 | FileCheck --check-prefixes=CHECK,NO-TAIL %s -; RUN: llc < %s -asm-verbose=false -disable-wasm-fallthrough-return-opt -wasm-keep-registers -fast-isel -fast-isel-abort=1 -mattr=+sign-ext,+simd128 | FileCheck --check-prefixes=CHECK,NO-TAIL %s +; RUN: llc < %s -asm-verbose=false -disable-wasm-fallthrough-return-opt -wasm-keep-registers -mattr=+sign-ext,+simd128 | FileCheck --check-prefixes=CHECK,SLOW,NO-TAIL %s +; RUN: llc < %s -asm-verbose=false -disable-wasm-fallthrough-return-opt -wasm-keep-registers -fast-isel -fast-isel-abort=1 -mattr=+sign-ext,+simd128 | FileCheck --check-prefixes=CHECK,FAST,NO-TAIL %s ; RUN: llc < %s -asm-verbose=false -disable-wasm-fallthrough-return-opt -wasm-keep-registers -mattr=+sign-ext,+simd128,+tail-call | FileCheck --check-prefixes=CHECK,SLOW-TAIL %s ; RUN: llc < %s -asm-verbose=false -disable-wasm-fallthrough-return-opt -wasm-keep-registers -fast-isel -fast-isel-abort=1 -mattr=+sign-ext,+simd128,+tail-call | FileCheck --check-prefixes=CHECK,FAST-TAIL %s @@ -251,6 +251,46 @@ entry: ret void } +; Calling non-functional globals should be lowered to call_indirects. +; CHECK-LABEL: call_indirect_int: +; CHECK: i32.const $push[[L0:[0-9]+]]=, global_i8 +; CHECK-NEXT: call_indirect $pop[[L0]] +; CHECK-NEXT: i32.const $push[[L1:[0-9]+]]=, global_i32 +; CHECK-NEXT: call_indirect $pop[[L1]] +@global_i8 = global i8 0 +@global_i32 = global i32 0 +define void @call_indirect_int() { + call void @global_i8() + call void @global_i32() + ret void +} + +; Calling aliases of non-functional globals should be lowered to call_indirects. +; CHECK-LABEL: call_indirect_int_alias: +; CHECK: i32.const $push[[L0:[0-9]+]]=, global_i8_alias +; CHECK-NEXT: call_indirect $pop[[L0]] +; CHECK-NEXT: i32.const $push[[L1:[0-9]+]]=, global_i32_alias +; CHECK-NEXT: call_indirect $pop[[L1]] +@global_i8_alias = alias i8, ptr @global_i8 +@global_i32_alias = alias i32, ptr @global_i32 +define void @call_indirect_int_alias() { + call void @global_i8_alias() + call void @global_i32_alias() + ret void +} + +; Ideally calling aliases of functions should be lowered to direct calls. We +; support this in the normal (=slow) isel. +; CHECK-LABEL: call_func_alias: +; SLOW: call func_alias +; FAST: i32.const $push[[L0:[0-9]+]]=, func_alias +; FAST-NEXT: call_indirect $pop[[L0]] +@func_alias = alias void (), ptr @call_void_nullary +define void @call_func_alias() { + call void @func_alias() + ret void +} + ; TODO: test the following: ; - More argument combinations. ; - Tail call. -- 2.7.4