-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[WebAssembly] Implement addrspacecast to funcref #166820
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WebAssembly] Implement addrspacecast to funcref #166820
Conversation
|
@llvm/pr-subscribers-backend-webassembly Author: Demetrius Kanios (QuantumSegfault) ChangesAdds lowering of When given a constant function pointer, it lowers to a direct Full diff: https://github.com/llvm/llvm-project/pull/166820.diff 4 Files Affected:
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
index af322982d5355..782b878350b42 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
@@ -409,6 +409,10 @@ WebAssemblyTargetLowering::WebAssemblyTargetLowering(
setOperationAction(ISD::INTRINSIC_W_CHAIN, MVT::Other, Custom);
setOperationAction(ISD::INTRINSIC_VOID, MVT::Other, Custom);
+ // Allow converting function ptrs in address space 0 to WASM funcref (address
+ // space 20)
+ setOperationAction(ISD::ADDRSPACECAST, MVT::funcref, Custom);
+
setMaxAtomicSizeInBitsSupported(64);
// Always convert switches to br_tables unless there is only one case, which
@@ -1733,6 +1737,8 @@ SDValue WebAssemblyTargetLowering::LowerOperation(SDValue Op,
return LowerMUL_LOHI(Op, DAG);
case ISD::UADDO:
return LowerUADDO(Op, DAG);
+ case ISD::ADDRSPACECAST:
+ return LowerADDRSPACECAST(Op, DAG);
}
}
@@ -1876,6 +1882,58 @@ SDValue WebAssemblyTargetLowering::LowerUADDO(SDValue Op,
return DAG.getMergeValues(Ops, DL);
}
+SDValue WebAssemblyTargetLowering::LowerADDRSPACECAST(SDValue Op,
+ SelectionDAG &DAG) const {
+ SDLoc DL(Op);
+
+ AddrSpaceCastSDNode *ACN = cast<AddrSpaceCastSDNode>(Op.getNode());
+
+ if (ACN->getSrcAddressSpace() !=
+ WebAssembly::WasmAddressSpace::WASM_ADDRESS_SPACE_DEFAULT ||
+ ACN->getDestAddressSpace() !=
+ WebAssembly::WasmAddressSpace::WASM_ADDRESS_SPACE_FUNCREF)
+ return Op;
+
+ if (ACN->getValueType(0) != MVT::funcref) {
+ reportFatalInternalError("Cannot addrspacecast to funcref addrspace with "
+ "results other than MVT::funcref");
+ }
+
+ SDValue Src = ACN->getOperand(0);
+
+ // Lower addrspacecasts of direct/constant function ptrs to ref.func
+ if (auto *GA = dyn_cast<GlobalAddressSDNode>(
+ Src->getOpcode() == WebAssemblyISD::Wrapper ? Src->getOperand(0)
+ : Src)) {
+ auto *GV = GA->getGlobal();
+
+ if (const Function *F = dyn_cast<Function>(GV)) {
+ SDValue FnAddress = DAG.getTargetGlobalAddress(F, DL, MVT::i32);
+
+ SDValue RefFuncNode =
+ DAG.getNode(WebAssemblyISD::REF_FUNC, DL, MVT::funcref, FnAddress);
+ return RefFuncNode;
+ }
+ }
+
+ // Lower everything else to a table.get from the indirect function table
+ const MachineFunction &MF = DAG.getMachineFunction();
+
+ MVT PtrVT = getPointerTy(MF.getDataLayout());
+
+ MCSymbolWasm *Table =
+ WebAssembly::getOrCreateFunctionTableSymbol(MF.getContext(), Subtarget);
+ SDValue TableSym = DAG.getMCSymbol(Table, PtrVT);
+
+ SDValue TableSlot = Op.getOperand(0);
+
+ SDValue Result(DAG.getMachineNode(WebAssembly::TABLE_GET_FUNCREF, DL,
+ MVT::funcref, TableSym, TableSlot),
+ 0);
+
+ return Result;
+}
+
SDValue WebAssemblyTargetLowering::Replace128Op(SDNode *N,
SelectionDAG &DAG) const {
assert(Subtarget->hasWideArithmetic());
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.h b/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.h
index f7052989b3c75..c3cca072f1958 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.h
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.h
@@ -121,6 +121,7 @@ class WebAssemblyTargetLowering final : public TargetLowering {
SDValue LowerMUL_LOHI(SDValue Op, SelectionDAG &DAG) const;
SDValue Replace128Op(SDNode *N, SelectionDAG &DAG) const;
SDValue LowerUADDO(SDValue Op, SelectionDAG &DAG) const;
+ SDValue LowerADDRSPACECAST(SDValue Op, SelectionDAG &DAG) const;
// Custom DAG combine hooks
SDValue
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyInstrRef.td b/llvm/lib/Target/WebAssembly/WebAssemblyInstrRef.td
index 304c4f3fcb028..2589ab758638c 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyInstrRef.td
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyInstrRef.td
@@ -11,6 +11,11 @@
///
//===----------------------------------------------------------------------===//
+def WebAssemblyRefFunc_t : SDTypeProfile<1, 1, [SDTCisVT<0, funcref>, SDTCisPtrTy<1>]>;
+def WebAssemblyRefFunc :
+ SDNode<"WebAssemblyISD::REF_FUNC", WebAssemblyRefFunc_t,
+ []>;
+
multiclass REF_I<WebAssemblyRegClass rc, ValueType vt, string ht> {
defm REF_NULL_#rc : I<(outs rc:$dst), (ins),
(outs), (ins),
@@ -42,7 +47,7 @@ defm REF_TEST_FUNCREF : I<(outs I32:$res), (ins TypeIndex:$type, FUNCREF:$ref),
Requires<[HasGC]>;
defm REF_FUNC : I<(outs FUNCREF:$res), (ins function32_op:$func),
- (outs), (ins function32_op:$func), [],
+ (outs), (ins function32_op:$func), [(set FUNCREF:$res, (WebAssemblyRefFunc tglobaladdr:$func))],
"ref.func\t$func", "ref.func $func", 0xd2>,
Requires<[HasReferenceTypes]>;
diff --git a/llvm/test/CodeGen/WebAssembly/addrspacecast-funcref.ll b/llvm/test/CodeGen/WebAssembly/addrspacecast-funcref.ll
new file mode 100644
index 0000000000000..1ae676f1c99c8
--- /dev/null
+++ b/llvm/test/CodeGen/WebAssembly/addrspacecast-funcref.ll
@@ -0,0 +1,55 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 6
+; RUN: llc -mtriple=wasm32-unknown-unknown -mattr=+reference-types < %s | FileCheck -check-prefixes=CHECK,WASM32 %s
+; RUN: llc -mtriple=wasm64-unknown-unknown -mattr=+reference-types < %s | FileCheck -check-prefixes=CHECK,WASM64 %s
+
+%funcref = type ptr addrspace(20) ;; addrspace 20 is nonintegral
+
+declare void @foo();
+
+@global_var = local_unnamed_addr global i32 undef
+
+define %funcref @cast_const_funcptr() {
+; CHECK-LABEL: cast_const_funcptr:
+; CHECK: .functype cast_const_funcptr () -> (funcref)
+; CHECK-NEXT: # %bb.0:
+; CHECK-NEXT: ref.func foo
+; CHECK-NEXT: # fallthrough-return
+ %result = addrspacecast ptr @foo to ptr addrspace(20)
+ ret %funcref %result
+}
+
+define %funcref @cast_const_not_funcptr() {
+; WASM32-LABEL: cast_const_not_funcptr:
+; WASM32: .functype cast_const_not_funcptr () -> (funcref)
+; WASM32-NEXT: # %bb.0:
+; WASM32-NEXT: i32.const global_var
+; WASM32-NEXT: table.get __indirect_function_table
+; WASM32-NEXT: # fallthrough-return
+;
+; WASM64-LABEL: cast_const_not_funcptr:
+; WASM64: .functype cast_const_not_funcptr () -> (funcref)
+; WASM64-NEXT: # %bb.0:
+; WASM64-NEXT: i64.const global_var
+; WASM64-NEXT: table.get __indirect_function_table
+; WASM64-NEXT: # fallthrough-return
+ %result = addrspacecast ptr @global_var to ptr addrspace(20)
+ ret %funcref %result
+}
+
+define %funcref @cast_param_funcptr(ptr %funcptr) {
+; WASM32-LABEL: cast_param_funcptr:
+; WASM32: .functype cast_param_funcptr (i32) -> (funcref)
+; WASM32-NEXT: # %bb.0:
+; WASM32-NEXT: local.get 0
+; WASM32-NEXT: table.get __indirect_function_table
+; WASM32-NEXT: # fallthrough-return
+;
+; WASM64-LABEL: cast_param_funcptr:
+; WASM64: .functype cast_param_funcptr (i64) -> (funcref)
+; WASM64-NEXT: # %bb.0:
+; WASM64-NEXT: local.get 0
+; WASM64-NEXT: table.get __indirect_function_table
+; WASM64-NEXT: # fallthrough-return
+ %result = addrspacecast ptr %funcptr to ptr addrspace(20)
+ ret %funcref %result
+}
|
|
✅ With the latest revision this PR passed the undef deprecator. |
|
Requesting review. |
|
Ping |
dschuff
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I started a review last time but forgot to submit it 🤦
dschuff
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/187/builds/14282 Here is the relevant piece of the build log for the reference |
|
Looks like this test is failing in the LLVM configuration with the expensive checks enabled. I'm going to go ahead and revert it for now. If you build with that configuration (see the EXPENSIVE_CHECKS flag in https://llvm.org/docs/CMake.html) it should be pretty easy to reproduce. Then we can reland it. |
Reverts #166820 There was a failure in the ENABLE_EXPENSIVE_CHECKS configuration.
…(#170785) Reverts llvm/llvm-project#166820 There was a failure in the ENABLE_EXPENSIVE_CHECKS configuration.
|
I see what's happening. There's a mismatch in Wasm64 between the table types, the type TABLE_GET expects, and what I'm giving it. TABLE_GET only ever expects i32 as an index. However, in Wasm64, tables can have either i32 or i64 indices. However we aren't consistent about it. It seems that the Should we move to i64 across the board (why wasn't this done?)? That way we can make TABLE_GET and TABLE_SET address mode aware. Either that or we need a different ISD node to be 64-bit aware while the existing one remains not. |
|
+cc @sbc100 So the options are basically as you say. It seems like it would probably be more straightforward to move to table64 across the board for wasm64 and then have everything be architecture/address-mode dependent as we do for pointers. In principle we could also have 32-bit module-defined tables but there doesn't seem a lot of benefit or need for that right now AFAIK. |
Adds lowering of
addrspacecast [0 -> 20]to allow easy conversion of function pointers to WasmfuncrefWhen given a constant function pointer, it lowers to a direct
ref.func. Otherwise it lowers to atable.getfrom__indirect_function_tableusing the provided pointer as the index.