Skip to content

Commit e3b79af

Browse files
hoodmanehoodmane
authored
[WebAssembly,llvm] Fix buildbot problems with llvm.wasm.ref.test.func (#150116)
PR #147486 broke the sanitizer and expensive-checks buildbot. These captures were needed when toWasmValType emitted a diagnostic but are no longer needed since we changed it to an assertion failure. This removes the unneeded captures and should fix the sanitizer-buildbot. I also fixed the codegen in the wasm64 target: table.get requires an i32 but in wasm64 the function pointer is an i64. We need an additional `i32.wrap_i64` to convert it. I also added `-verify-machineinstrs` to the tests so that the test suite validates this fix. Finally, I noticed that #150201 uses a feature of the intrinsic that is not covered by the tests, namely `ptr` arguments. So I added one additional test case to ensure that it works properly. cc @dschuff
1 parent 9cb5c00 commit e3b79af

File tree

2 files changed

+41
-7
lines changed

2 files changed

+41
-7
lines changed

‎llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ static SDValue getTagSymNode(int Tag, SelectionDAG *DAG) {
123123
static APInt encodeFunctionSignature(SelectionDAG *DAG, SDLoc &DL,
124124
SmallVector<MVT, 4> &Returns,
125125
SmallVector<MVT, 4> &Params) {
126-
auto toWasmValType = [&](MVT VT) {
126+
auto toWasmValType = [](MVT VT) {
127127
if (VT == MVT::i32) {
128128
return wasm::ValType::I32;
129129
}
@@ -244,10 +244,18 @@ void WebAssemblyDAGToDAGISel::Select(SDNode *Node) {
244244
MCSymbol *Table = WebAssembly::getOrCreateFunctionTableSymbol(
245245
MF.getContext(), Subtarget);
246246
SDValue TableSym = CurDAG->getMCSymbol(Table, PtrVT);
247-
SDValue FuncRef = SDValue(
248-
CurDAG->getMachineNode(WebAssembly::TABLE_GET_FUNCREF, DL,
249-
MVT::funcref, TableSym, Node->getOperand(1)),
250-
0);
247+
SDValue FuncPtr = Node->getOperand(1);
248+
if (Subtarget->hasAddr64() && FuncPtr.getValueType() == MVT::i64) {
249+
// table.get expects an i32 but on 64 bit platforms the function pointer
250+
// is an i64. In that case, i32.wrap_i64 to convert.
251+
FuncPtr = SDValue(CurDAG->getMachineNode(WebAssembly::I32_WRAP_I64, DL,
252+
MVT::i32, FuncPtr),
253+
0);
254+
}
255+
SDValue FuncRef =
256+
SDValue(CurDAG->getMachineNode(WebAssembly::TABLE_GET_FUNCREF, DL,
257+
MVT::funcref, TableSym, FuncPtr),
258+
0);
251259

252260
// Encode the signature information into the type index placeholder.
253261
// This gets decoded and converted into the actual type signature in

‎llvm/test/CodeGen/WebAssembly/ref-test-func.ll

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5
2-
; RUN: llc < %s --mtriple=wasm32-unknown-unknown -mcpu=mvp -mattr=+reference-types | FileCheck --check-prefixes CHECK,CHK32 %s
3-
; RUN: llc < %s --mtriple=wasm64-unknown-unknown -mcpu=mvp -mattr=+reference-types | FileCheck --check-prefixes CHECK,CHK64 %s
2+
; RUN: llc < %s --mtriple=wasm32-unknown-unknown -mcpu=mvp -mattr=+reference-types -verify-machineinstrs | FileCheck --check-prefixes CHECK,CHK32 %s
3+
; RUN: llc < %s --mtriple=wasm64-unknown-unknown -mcpu=mvp -mattr=+reference-types -verify-machineinstrs | FileCheck --check-prefixes CHECK,CHK64 %s
44

55
define void @test_fpsig_void_void(ptr noundef %func) local_unnamed_addr #0 {
66
; CHECK-LABEL: test_fpsig_void_void:
77
; CHK32: .functype test_fpsig_void_void (i32) -> ()
88
; CHK64: .functype test_fpsig_void_void (i64) -> ()
99
; CHECK-NEXT: # %bb.0: # %entry
1010
; CHECK-NEXT: local.get 0
11+
; CHK64-NEXT: i32.wrap_i64
1112
; CHECK-NEXT: table.get __indirect_function_table
1213
; CHECK-NEXT: ref.test () -> ()
1314
; CHECK-NEXT: call use
@@ -24,6 +25,7 @@ define void @test_fpsig_return_i32(ptr noundef %func) local_unnamed_addr #0 {
2425
; CHK64: .functype test_fpsig_return_i32 (i64) -> ()
2526
; CHECK-NEXT: # %bb.0: # %entry
2627
; CHECK-NEXT: local.get 0
28+
; CHK64-NEXT: i32.wrap_i64
2729
; CHECK-NEXT: table.get __indirect_function_table
2830
; CHECK-NEXT: ref.test () -> (i32)
2931
; CHECK-NEXT: call use
@@ -40,6 +42,7 @@ define void @test_fpsig_return_i64(ptr noundef %func) local_unnamed_addr #0 {
4042
; CHK64: .functype test_fpsig_return_i64 (i64) -> ()
4143
; CHECK-NEXT: # %bb.0: # %entry
4244
; CHECK-NEXT: local.get 0
45+
; CHK64-NEXT: i32.wrap_i64
4346
; CHECK-NEXT: table.get __indirect_function_table
4447
; CHECK-NEXT: ref.test () -> (i64)
4548
; CHECK-NEXT: call use
@@ -56,6 +59,7 @@ define void @test_fpsig_return_f32(ptr noundef %func) local_unnamed_addr #0 {
5659
; CHK64: .functype test_fpsig_return_f32 (i64) -> ()
5760
; CHECK-NEXT: # %bb.0: # %entry
5861
; CHECK-NEXT: local.get 0
62+
; CHK64-NEXT: i32.wrap_i64
5963
; CHECK-NEXT: table.get __indirect_function_table
6064
; CHECK-NEXT: ref.test () -> (f32)
6165
; CHECK-NEXT: call use
@@ -72,6 +76,7 @@ define void @test_fpsig_return_f64(ptr noundef %func) local_unnamed_addr #0 {
7276
; CHK64: .functype test_fpsig_return_f64 (i64) -> ()
7377
; CHECK-NEXT: # %bb.0: # %entry
7478
; CHECK-NEXT: local.get 0
79+
; CHK64-NEXT: i32.wrap_i64
7580
; CHECK-NEXT: table.get __indirect_function_table
7681
; CHECK-NEXT: ref.test () -> (f64)
7782
; CHECK-NEXT: call use
@@ -89,6 +94,7 @@ define void @test_fpsig_param_i32(ptr noundef %func) local_unnamed_addr #0 {
8994
; CHK64: .functype test_fpsig_param_i32 (i64) -> ()
9095
; CHECK-NEXT: # %bb.0: # %entry
9196
; CHECK-NEXT: local.get 0
97+
; CHK64-NEXT: i32.wrap_i64
9298
; CHECK-NEXT: table.get __indirect_function_table
9399
; CHECK-NEXT: ref.test (f64) -> ()
94100
; CHECK-NEXT: call use
@@ -106,6 +112,7 @@ define void @test_fpsig_multiple_params_and_returns(ptr noundef %func) local_unn
106112
; CHK64: .functype test_fpsig_multiple_params_and_returns (i64) -> ()
107113
; CHECK-NEXT: # %bb.0: # %entry
108114
; CHECK-NEXT: local.get 0
115+
; CHK64-NEXT: i32.wrap_i64
109116
; CHECK-NEXT: table.get __indirect_function_table
110117
; CHECK-NEXT: ref.test (i64, f32, i64) -> (i32, i64, f32, f64)
111118
; CHECK-NEXT: call use
@@ -117,4 +124,23 @@ entry:
117124
}
118125

119126

127+
define void @test_fpsig_ptrs(ptr noundef %func) local_unnamed_addr #0 {
128+
; CHECK-LABEL: test_fpsig_ptrs:
129+
; CHK32: .functype test_fpsig_ptrs (i32) -> ()
130+
; CHK64: .functype test_fpsig_ptrs (i64) -> ()
131+
; CHECK-NEXT: # %bb.0: # %entry
132+
; CHECK-NEXT: local.get 0
133+
; CHK64-NEXT: i32.wrap_i64
134+
; CHECK-NEXT: table.get __indirect_function_table
135+
; CHK32-NEXT: ref.test (i32, i32) -> (i32)
136+
; CHK64-NEXT: ref.test (i64, i64) -> (i64)
137+
; CHECK-NEXT: call use
138+
; CHECK-NEXT: # fallthrough-return
139+
entry:
140+
%res = tail call i32 (ptr, ...) @llvm.wasm.ref.test.func(ptr %func, ptr null, token poison, ptr null, ptr null)
141+
tail call void @use(i32 noundef %res) #3
142+
ret void
143+
}
144+
145+
120146
declare void @use(i32 noundef) local_unnamed_addr #1

0 commit comments

Comments
 (0)