From d66b6a8bc3cb2ae30f0ccc3289f07da6cf2ca903 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Fri, 16 Feb 2024 16:57:34 -0500 Subject: [PATCH 01/37] Add error reg support --- src/coreclr/jit/codegenarmarch.cpp | 18 +++++ src/coreclr/jit/codegenxarch.cpp | 17 +++++ src/coreclr/jit/compiler.h | 3 +- src/coreclr/jit/compiler.hpp | 1 + src/coreclr/jit/gentree.cpp | 5 +- src/coreclr/jit/gentree.h | 5 ++ src/coreclr/jit/gtlist.h | 1 + src/coreclr/jit/importercalls.cpp | 106 +++++++++++++++++++++++++++-- src/coreclr/jit/targetamd64.h | 5 ++ src/coreclr/jit/targetarm64.h | 5 ++ src/coreclr/jit/valuenum.cpp | 1 + 11 files changed, 160 insertions(+), 7 deletions(-) diff --git a/src/coreclr/jit/codegenarmarch.cpp b/src/coreclr/jit/codegenarmarch.cpp index 92e99471956f0d..c7f6f60acc7c91 100644 --- a/src/coreclr/jit/codegenarmarch.cpp +++ b/src/coreclr/jit/codegenarmarch.cpp @@ -439,8 +439,15 @@ void CodeGen::genCodeForTreeNode(GenTree* treeNode) case GT_CMPXCHG: genCodeForCmpXchg(treeNode->AsCmpXchg()); break; + #endif // TARGET_ARM64 +#ifdef SWIFT_SUPPORT + case GT_SWIFT_ERROR: + treeNode->SetRegNum(SWIFT_ERROR_REG); + break; +#endif // SWIFT_SUPPORT + case GT_RELOAD: // do nothing - reload is just a marker. // The parent node will call genConsumeReg on this which will trigger the unspill of this node's child @@ -3425,6 +3432,17 @@ void CodeGen::genCall(GenTreeCall* call) genDefineTempLabel(genCreateTempLabel()); } +#ifdef SWIFT_SUPPORT + // Clear the Swift error register before calling a Swift method, + // so we can check if it set the error register after returning. + // (Flag is only set if we know we need to check the error register) + if ((call->gtCallMoreFlags & GTF_CALL_M_SWIFT_ERROR_HANDLING) != 0) + { + assert(call->unmgdCallConv == CorInfoCallConvExtension::Swift); + instGen_Set_Reg_To_Zero(EA_PTRSIZE, SWIFT_ERROR_REG); + } +#endif // SWIFT_SUPPORT + genCallInstruction(call); genDefinePendingCallLabel(call); diff --git a/src/coreclr/jit/codegenxarch.cpp b/src/coreclr/jit/codegenxarch.cpp index b877262b1583d8..6176c48ec7afcd 100644 --- a/src/coreclr/jit/codegenxarch.cpp +++ b/src/coreclr/jit/codegenxarch.cpp @@ -2107,6 +2107,12 @@ void CodeGen::genCodeForTreeNode(GenTree* treeNode) case GT_NOP: break; +#ifdef SWIFT_SUPPORT + case GT_SWIFT_ERROR: + treeNode->SetRegNum(SWIFT_ERROR_REG); + break; +#endif // SWIFT_SUPPORT + case GT_KEEPALIVE: genConsumeRegs(treeNode->AsOp()->gtOp1); break; @@ -6089,6 +6095,17 @@ void CodeGen::genCall(GenTreeCall* call) instGen(INS_vzeroupper); } +#ifdef SWIFT_SUPPORT + // Clear the Swift error register before calling a Swift method, + // so we can check if it set the error register after returning. + // (Flag is only set if we know we need to check the error register) + if ((call->gtCallMoreFlags & GTF_CALL_M_SWIFT_ERROR_HANDLING) != 0) + { + assert(call->unmgdCallConv == CorInfoCallConvExtension::Swift); + instGen_Set_Reg_To_Zero(EA_PTRSIZE, SWIFT_ERROR_REG); + } +#endif // SWIFT_SUPPORT + genCallInstruction(call X86_ARG(stackArgBytes)); genDefinePendingCallLabel(call); diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index a18fee959436b4..53a5d1d51c796e 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -4363,7 +4363,7 @@ class Compiler void impCheckForPInvokeCall( GenTreeCall* call, CORINFO_METHOD_HANDLE methHnd, CORINFO_SIG_INFO* sig, unsigned mflags, BasicBlock* block); GenTreeCall* impImportIndirectCall(CORINFO_SIG_INFO* sig, const DebugInfo& di = DebugInfo()); - void impPopArgsForUnmanagedCall(GenTreeCall* call, CORINFO_SIG_INFO* sig); + void impPopArgsForUnmanagedCall(GenTreeCall* call, CORINFO_SIG_INFO* sig, const bool spillAllArgs); void impInsertHelperCall(CORINFO_HELPER_DESC* helperCall); void impHandleAccessAllowed(CorInfoIsAccessAllowedResult result, CORINFO_HELPER_DESC* helperCall); @@ -11315,6 +11315,7 @@ class GenTreeVisitor case GT_PINVOKE_EPILOG: case GT_IL_OFFSET: case GT_NOP: + case GT_SWIFT_ERROR: break; // Lclvar unary operators diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index 59e53baeb19f8a..5e12202b202223 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -4246,6 +4246,7 @@ void GenTree::VisitOperands(TVisitor visitor) case GT_PINVOKE_EPILOG: case GT_IL_OFFSET: case GT_NOP: + case GT_SWIFT_ERROR: return; // Unary operators with an optional operand diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index ad68efef307b77..a0b7306e214408 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -7420,6 +7420,7 @@ bool GenTree::OperSupportsOrderingSideEffect() const case GT_CMPXCHG: case GT_MEMORYBARRIER: case GT_CATCH_ARG: + case GT_SWIFT_ERROR: return true; default: return false; @@ -8758,7 +8759,7 @@ GenTreeStoreDynBlk* Compiler::gtNewStoreDynBlkNode(GenTree* addr, // // Arguments: // type - Type of the store -// addr - Destionation address +// addr - Destination address // data - Value to store // indirFlags - Indirection flags // @@ -10304,6 +10305,7 @@ GenTreeUseEdgeIterator::GenTreeUseEdgeIterator(GenTree* node) case GT_PINVOKE_EPILOG: case GT_IL_OFFSET: case GT_NOP: + case GT_SWIFT_ERROR: m_state = -1; return; @@ -12410,6 +12412,7 @@ void Compiler::gtDispLeaf(GenTree* tree, IndentStack* indentStack) case GT_MEMORYBARRIER: case GT_PINVOKE_PROLOG: case GT_JMPTABLE: + case GT_SWIFT_ERROR: break; case GT_RET_EXPR: diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 1bd7ce27004fe6..1a72ec1a6e86bd 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -4114,8 +4114,13 @@ enum GenTreeCallFlags : unsigned int GTF_CALL_M_HAS_LATE_DEVIRT_INFO = 0x01000000, // this call has late devirtualzation info GTF_CALL_M_LDVIRTFTN_INTERFACE = 0x02000000, // ldvirtftn on an interface type GTF_CALL_M_CAST_CAN_BE_EXPANDED = 0x04000000, // this cast (helper call) can be expanded if it's profitable. To be removed. + GTF_CALL_M_CAST_OBJ_NONNULL = 0x08000000, // if we expand this specific cast we don't need to check the input object for null // NOTE: if needed, this flag can be removed, and we can introduce new _NONNUL cast helpers + +#ifdef SWIFT_SUPPORT + GTF_CALL_M_SWIFT_ERROR_HANDLING = 0x10000000, // call uses the Swift calling convention, and error register will be checked after it returns. +#endif // SWIFT_SUPPORT }; inline constexpr GenTreeCallFlags operator ~(GenTreeCallFlags a) diff --git a/src/coreclr/jit/gtlist.h b/src/coreclr/jit/gtlist.h index 597a9e471d5b1f..0914e5fcaad922 100644 --- a/src/coreclr/jit/gtlist.h +++ b/src/coreclr/jit/gtlist.h @@ -37,6 +37,7 @@ GTNODE(LABEL , GenTree ,0,0,GTK_LEAF) // Jump- GTNODE(JMP , GenTreeVal ,0,0,GTK_LEAF|GTK_NOVALUE) // Jump to another function GTNODE(FTN_ADDR , GenTreeFptrVal ,0,0,GTK_LEAF) // Address of a function GTNODE(RET_EXPR , GenTreeRetExpr ,0,0,GTK_LEAF|DBK_NOTLIR) // Place holder for the return expression from an inline candidate +GTNODE(SWIFT_ERROR , GenTree ,0,0,GTK_LEAF) // Error register value post-Swift call //----------------------------------------------------------------------------- // Constant nodes: diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index b263285c42b545..60d670b06b6d21 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -98,6 +98,12 @@ var_types Compiler::impImportCall(OPCODE opcode, CORINFO_SIG_INFO calliSig; NewCallArg extraArg; +#ifdef SWIFT_SUPPORT + // Swift calls may use special register types that require additional IL, + // so if we're importing a Swift call, look for these types in the signature + int swiftErrorIndex = -1; +#endif // SWIFT_SUPPORT + /*------------------------------------------------------------------------- * First create the call node */ @@ -651,6 +657,8 @@ var_types Compiler::impImportCall(OPCODE opcode, if (call->gtFlags & GTF_CALL_UNMANAGED) { + assert(call->IsCall()); + // We set up the unmanaged call by linking the frame, disabling GC, etc // This needs to be cleaned up on return. // In addition, native calls have different normalization rules than managed code @@ -662,8 +670,58 @@ var_types Compiler::impImportCall(OPCODE opcode, } checkForSmallType = true; + bool spillAllArgs = false; + +#ifdef SWIFT_SUPPORT + // We are importing an unmanaged Swift call, which might require special parameter handling: + // - SwiftError* is passed to capture the address of an error thrown during the Swift call. + // - SwiftSelf is passed to enable calling an instance method of a Swift object. + if (call->AsCall()->unmgdCallConv == CorInfoCallConvExtension::Swift) + { + // Check the signature of the Swift call for the special types. + CORINFO_ARG_LIST_HANDLE sigArg = sig->args; + + for (unsigned short argIndex = 0; argIndex < sig->numArgs; + sigArg = info.compCompHnd->getArgNext(sigArg), argIndex++) + { + CORINFO_CLASS_HANDLE argClass; + CorInfoType argType = strip(info.compCompHnd->getArgType(sig, sigArg, &argClass)); + bool argIsByrefOrPtr = false; + + if ((argType == CORINFO_TYPE_BYREF) || (argType == CORINFO_TYPE_PTR)) + { + argClass = info.compCompHnd->getArgClass(sig, sigArg); + argType = info.compCompHnd->getChildType(argClass, &argClass); + argIsByrefOrPtr = true; + } - impPopArgsForUnmanagedCall(call->AsCall(), sig); + if ((argType != CORINFO_TYPE_VALUECLASS) || !info.compCompHnd->isIntrinsicType(argClass)) + { + continue; + } + + const char* namespaceName; + const char* className = info.compCompHnd->getClassNameFromMetadata(argClass, &namespaceName); + + if ((strcmp(className, "SwiftError") == 0) && + (strcmp(namespaceName, "System.Runtime.InteropServices.Swift") == 0)) + { + // For error handling purposes, we expect a pointer to a SwiftError to be passed + assert(argIsByrefOrPtr); + if (swiftErrorIndex != -1) + { + BADCODE("Duplicate SwiftError* parameter"); + } + + swiftErrorIndex = argIndex; + spillAllArgs = true; + } + // TODO: Handle SwiftSelf, SwiftAsync + } + } +#endif // SWIFT_SUPPORT + + impPopArgsForUnmanagedCall(call->AsCall(), sig, spillAllArgs); goto DONE; } @@ -1485,6 +1543,35 @@ var_types Compiler::impImportCall(OPCODE opcode, impPushOnStack(call, tiRetVal); } +#ifdef SWIFT_SUPPORT + if (swiftErrorIndex >= 0) + { + // This Swift call uses the error register + assert(call->IsCall()); + assert(call->AsCall()->unmgdCallConv == CorInfoCallConvExtension::Swift); + assert(swiftErrorIndex < (int)sig->numArgs); + CallArg* errorArg = call->AsCall()->gtArgs.GetArgByIndex(swiftErrorIndex); + + assert(errorArg != nullptr); + GenTree* argNode = errorArg->GetNode(); + assert(argNode != nullptr); + + // SwiftError* arg should have been spilled to a local temp variable + assert(argNode->OperIs(GT_LCL_VAR)); + + // Store the error register value to where the SwiftError* points to + GenTree* errorRegNode = new (this, GT_SWIFT_ERROR) GenTree(GT_SWIFT_ERROR, TYP_REF); + errorRegNode->SetHasOrderingSideEffect(); + + GenTree* argNodeCopy = gtNewLclvNode(argNode->AsLclVar()->GetLclNum(), argNode->TypeGet()); + GenTreeStoreInd* swiftErrorStore = gtNewStoreIndNode(argNodeCopy->TypeGet(), argNodeCopy, errorRegNode); + impAppendTree(swiftErrorStore, CHECK_SPILL_ALL, impCurStmtDI, false); + + // Indicate the error register will be checked after this call returns + call->AsCall()->gtCallMoreFlags |= GTF_CALL_M_SWIFT_ERROR_HANDLING; + } +#endif // SWIFT_SUPPORT + return callRetTyp; } #ifdef _PREFAST_ @@ -1822,7 +1909,7 @@ GenTreeCall* Compiler::impImportIndirectCall(CORINFO_SIG_INFO* sig, const DebugI /*****************************************************************************/ -void Compiler::impPopArgsForUnmanagedCall(GenTreeCall* call, CORINFO_SIG_INFO* sig) +void Compiler::impPopArgsForUnmanagedCall(GenTreeCall* call, CORINFO_SIG_INFO* sig, const bool spillAllArgs) { assert(call->gtFlags & GTF_CALL_UNMANAGED); @@ -1842,13 +1929,23 @@ void Compiler::impPopArgsForUnmanagedCall(GenTreeCall* call, CORINFO_SIG_INFO* s if (call->unmgdCallConv == CorInfoCallConvExtension::Thiscall) { - assert(argsToReverse); + assert(argsToReverse != 0); argsToReverse--; } #ifndef TARGET_X86 // Don't reverse args on ARM or x64 - first four args always placed in regs in order argsToReverse = 0; + + // Fast path for spilling all args + if (spillAllArgs) + { + for (unsigned level = 0; level < verCurrentState.esStackDepth; level++) + { + impSpillStackEntry(level, + BAD_VAR_NUM DEBUGARG(false) DEBUGARG("impPopArgsForUnmanagedCall - spillAllArgs=true")); + } + } #endif for (unsigned level = verCurrentState.esStackDepth - argsToReverse; level < verCurrentState.esStackDepth; level++) @@ -5610,8 +5707,7 @@ void Compiler::impCheckForPInvokeCall( // return here without inlining the native call. if (unmanagedCallConv == CorInfoCallConvExtension::Managed || unmanagedCallConv == CorInfoCallConvExtension::Fastcall || - unmanagedCallConv == CorInfoCallConvExtension::FastcallMemberFunction || - unmanagedCallConv == CorInfoCallConvExtension::Swift) + unmanagedCallConv == CorInfoCallConvExtension::FastcallMemberFunction) { return; } diff --git a/src/coreclr/jit/targetamd64.h b/src/coreclr/jit/targetamd64.h index 4abe71984b57cf..3a5491f08b8d5c 100644 --- a/src/coreclr/jit/targetamd64.h +++ b/src/coreclr/jit/targetamd64.h @@ -563,4 +563,9 @@ #define RBM_STACK_PROBE_HELPER_TRASH RBM_RAX #endif // !UNIX_AMD64_ABI + #define SWIFT_SUPPORT + #define SWIFT_ERROR_REG REG_R12 + #define SWIFT_ERROR_RBM RBM_R12 + #define SWIFT_SELF_REG REG_R13 + // clang-format on diff --git a/src/coreclr/jit/targetarm64.h b/src/coreclr/jit/targetarm64.h index 3646ecb4407bf7..b27614dff0112c 100644 --- a/src/coreclr/jit/targetarm64.h +++ b/src/coreclr/jit/targetarm64.h @@ -370,4 +370,9 @@ #define REG_ZERO_INIT_FRAME_REG2 REG_R10 #define REG_ZERO_INIT_FRAME_SIMD REG_V16 + #define SWIFT_SUPPORT + #define SWIFT_ERROR_REG REG_R21 + #define SWIFT_ERROR_RBM RBM_R21 + #define SWIFT_SELF_REG REG_R20 + // clang-format on diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 977e984865aac7..6d0fb2d624e5a9 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -11256,6 +11256,7 @@ void Compiler::fgValueNumberTree(GenTree* tree) break; // These do not represent values. + case GT_SWIFT_ERROR: case GT_NO_OP: case GT_NOP: case GT_JMP: // Control flow From 0da12b32af6f4756dd2ee9e67d8ea3824b94b504 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Fri, 16 Feb 2024 17:35:21 -0500 Subject: [PATCH 02/37] Kill error reg if calling a Swift method --- src/coreclr/jit/codegenarmarch.cpp | 4 ++-- src/coreclr/jit/codegenxarch.cpp | 4 ++-- src/coreclr/jit/importercalls.cpp | 8 ++++---- src/coreclr/jit/lsrabuild.cpp | 10 ++++++++++ src/coreclr/jit/lsraxarch.cpp | 13 +++++++++++++ src/coreclr/jit/targetamd64.h | 4 ++-- src/coreclr/jit/targetarm64.h | 4 ++-- 7 files changed, 35 insertions(+), 12 deletions(-) diff --git a/src/coreclr/jit/codegenarmarch.cpp b/src/coreclr/jit/codegenarmarch.cpp index c7f6f60acc7c91..01684bc5dc304c 100644 --- a/src/coreclr/jit/codegenarmarch.cpp +++ b/src/coreclr/jit/codegenarmarch.cpp @@ -444,7 +444,7 @@ void CodeGen::genCodeForTreeNode(GenTree* treeNode) #ifdef SWIFT_SUPPORT case GT_SWIFT_ERROR: - treeNode->SetRegNum(SWIFT_ERROR_REG); + treeNode->SetRegNum(REG_SWIFT_ERROR); break; #endif // SWIFT_SUPPORT @@ -3439,7 +3439,7 @@ void CodeGen::genCall(GenTreeCall* call) if ((call->gtCallMoreFlags & GTF_CALL_M_SWIFT_ERROR_HANDLING) != 0) { assert(call->unmgdCallConv == CorInfoCallConvExtension::Swift); - instGen_Set_Reg_To_Zero(EA_PTRSIZE, SWIFT_ERROR_REG); + instGen_Set_Reg_To_Zero(EA_PTRSIZE, REG_SWIFT_ERROR); } #endif // SWIFT_SUPPORT diff --git a/src/coreclr/jit/codegenxarch.cpp b/src/coreclr/jit/codegenxarch.cpp index 6176c48ec7afcd..6e0fd51194c7bb 100644 --- a/src/coreclr/jit/codegenxarch.cpp +++ b/src/coreclr/jit/codegenxarch.cpp @@ -2109,7 +2109,7 @@ void CodeGen::genCodeForTreeNode(GenTree* treeNode) #ifdef SWIFT_SUPPORT case GT_SWIFT_ERROR: - treeNode->SetRegNum(SWIFT_ERROR_REG); + treeNode->SetRegNum(REG_SWIFT_ERROR); break; #endif // SWIFT_SUPPORT @@ -6102,7 +6102,7 @@ void CodeGen::genCall(GenTreeCall* call) if ((call->gtCallMoreFlags & GTF_CALL_M_SWIFT_ERROR_HANDLING) != 0) { assert(call->unmgdCallConv == CorInfoCallConvExtension::Swift); - instGen_Set_Reg_To_Zero(EA_PTRSIZE, SWIFT_ERROR_REG); + instGen_Set_Reg_To_Zero(EA_PTRSIZE, REG_SWIFT_ERROR); } #endif // SWIFT_SUPPORT diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 60d670b06b6d21..5b75c5f5f1283f 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -685,13 +685,13 @@ var_types Compiler::impImportCall(OPCODE opcode, sigArg = info.compCompHnd->getArgNext(sigArg), argIndex++) { CORINFO_CLASS_HANDLE argClass; - CorInfoType argType = strip(info.compCompHnd->getArgType(sig, sigArg, &argClass)); - bool argIsByrefOrPtr = false; + CorInfoType argType = strip(info.compCompHnd->getArgType(sig, sigArg, &argClass)); + bool argIsByrefOrPtr = false; if ((argType == CORINFO_TYPE_BYREF) || (argType == CORINFO_TYPE_PTR)) { - argClass = info.compCompHnd->getArgClass(sig, sigArg); - argType = info.compCompHnd->getChildType(argClass, &argClass); + argClass = info.compCompHnd->getArgClass(sig, sigArg); + argType = info.compCompHnd->getChildType(argClass, &argClass); argIsByrefOrPtr = true; } diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index 0c1d3f74475c8d..68bcddda62c40b 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -880,6 +880,16 @@ regMaskTP LinearScan::getKillSetForCall(GenTreeCall* call) assert(!call->IsVirtualStub() || ((killMask & compiler->virtualStubParamInfo->GetRegMask()) == compiler->virtualStubParamInfo->GetRegMask())); #endif // !TARGET_ARM + +#ifdef SWIFT_SUPPORT + // Swift calls that throw may trash the callee-saved error register, + // so don't use the register post-call until it is consumed by SwiftError (if ever) + if (call->unmgdCallConv == CorInfoCallConvExtension::Swift) + { + killMask |= RBM_SWIFT_ERROR; + } +#endif // SWIFT_SUPPORT + return killMask; } diff --git a/src/coreclr/jit/lsraxarch.cpp b/src/coreclr/jit/lsraxarch.cpp index cb2b82d5d8ce92..2e52cc3b67a59f 100644 --- a/src/coreclr/jit/lsraxarch.cpp +++ b/src/coreclr/jit/lsraxarch.cpp @@ -633,6 +633,19 @@ int LinearScan::BuildNode(GenTree* tree) } break; +#ifdef SWIFT_SUPPORT + case GT_SWIFT_ERROR: + srcCount = 0; + assert(dstCount == 1); + + // After a Swift call potentially trashes the error register, + // the register can be used again only if there is a GT_SWIFT_ERROR node to consume it + // (i.e. the register's value is saved to a SwiftError) + addRefsForPhysRegMask(RBM_SWIFT_ERROR, currentLoc, RefTypeKill, true); + BuildDef(tree, RBM_SWIFT_ERROR); + break; +#endif // SWIFT_SUPPORT + } // end switch (tree->OperGet()) // We need to be sure that we've set srcCount and dstCount appropriately. diff --git a/src/coreclr/jit/targetamd64.h b/src/coreclr/jit/targetamd64.h index 3a5491f08b8d5c..147355c2474a24 100644 --- a/src/coreclr/jit/targetamd64.h +++ b/src/coreclr/jit/targetamd64.h @@ -564,8 +564,8 @@ #endif // !UNIX_AMD64_ABI #define SWIFT_SUPPORT - #define SWIFT_ERROR_REG REG_R12 - #define SWIFT_ERROR_RBM RBM_R12 + #define REG_SWIFT_ERROR REG_R12 + #define RBM_SWIFT_ERROR RBM_R12 #define SWIFT_SELF_REG REG_R13 // clang-format on diff --git a/src/coreclr/jit/targetarm64.h b/src/coreclr/jit/targetarm64.h index b27614dff0112c..74a14535f15075 100644 --- a/src/coreclr/jit/targetarm64.h +++ b/src/coreclr/jit/targetarm64.h @@ -371,8 +371,8 @@ #define REG_ZERO_INIT_FRAME_SIMD REG_V16 #define SWIFT_SUPPORT - #define SWIFT_ERROR_REG REG_R21 - #define SWIFT_ERROR_RBM RBM_R21 + #define REG_SWIFT_ERROR REG_R21 + #define RBM_SWIFT_ERROR RBM_R21 #define SWIFT_SELF_REG REG_R20 // clang-format on From 56eb2a487e0f5a3720e739ed5a2050dfe4981097 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Fri, 16 Feb 2024 17:38:03 -0500 Subject: [PATCH 03/37] Style --- src/coreclr/jit/lsraxarch.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/lsraxarch.cpp b/src/coreclr/jit/lsraxarch.cpp index 2e52cc3b67a59f..66e7fccbd0bc51 100644 --- a/src/coreclr/jit/lsraxarch.cpp +++ b/src/coreclr/jit/lsraxarch.cpp @@ -637,7 +637,7 @@ int LinearScan::BuildNode(GenTree* tree) case GT_SWIFT_ERROR: srcCount = 0; assert(dstCount == 1); - + // After a Swift call potentially trashes the error register, // the register can be used again only if there is a GT_SWIFT_ERROR node to consume it // (i.e. the register's value is saved to a SwiftError) From 2c533aab85406617a3046c9d57745edc328f58c0 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Fri, 16 Feb 2024 17:43:04 -0500 Subject: [PATCH 04/37] Don't forget arm64 lsra --- src/coreclr/jit/codegenarmarch.cpp | 1 - src/coreclr/jit/lsraarm64.cpp | 13 +++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/codegenarmarch.cpp b/src/coreclr/jit/codegenarmarch.cpp index 01684bc5dc304c..442a71ea95390a 100644 --- a/src/coreclr/jit/codegenarmarch.cpp +++ b/src/coreclr/jit/codegenarmarch.cpp @@ -439,7 +439,6 @@ void CodeGen::genCodeForTreeNode(GenTree* treeNode) case GT_CMPXCHG: genCodeForCmpXchg(treeNode->AsCmpXchg()); break; - #endif // TARGET_ARM64 #ifdef SWIFT_SUPPORT diff --git a/src/coreclr/jit/lsraarm64.cpp b/src/coreclr/jit/lsraarm64.cpp index ea3bc9d7fb37e0..84e88ed36d5a0d 100644 --- a/src/coreclr/jit/lsraarm64.cpp +++ b/src/coreclr/jit/lsraarm64.cpp @@ -1282,6 +1282,19 @@ int LinearScan::BuildNode(GenTree* tree) srcCount = BuildSelect(tree->AsOp()); break; +#ifdef SWIFT_SUPPORT + case GT_SWIFT_ERROR: + srcCount = 0; + assert(dstCount == 1); + + // After a Swift call potentially trashes the error register, + // the register can be used again only if there is a GT_SWIFT_ERROR node to consume it + // (i.e. the register's value is saved to a SwiftError) + addRefsForPhysRegMask(RBM_SWIFT_ERROR, currentLoc, RefTypeKill, true); + BuildDef(tree, RBM_SWIFT_ERROR); + break; +#endif // SWIFT_SUPPORT + } // end switch (tree->OperGet()) if (tree->IsUnusedValue() && (dstCount != 0)) From 49bf6399e8258b68ea079568f52416fc30d937b8 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Fri, 16 Feb 2024 17:46:31 -0500 Subject: [PATCH 05/37] Remove space --- src/coreclr/jit/gentree.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 1a72ec1a6e86bd..f0e7b8d61d46ab 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -4114,7 +4114,6 @@ enum GenTreeCallFlags : unsigned int GTF_CALL_M_HAS_LATE_DEVIRT_INFO = 0x01000000, // this call has late devirtualzation info GTF_CALL_M_LDVIRTFTN_INTERFACE = 0x02000000, // ldvirtftn on an interface type GTF_CALL_M_CAST_CAN_BE_EXPANDED = 0x04000000, // this cast (helper call) can be expanded if it's profitable. To be removed. - GTF_CALL_M_CAST_OBJ_NONNULL = 0x08000000, // if we expand this specific cast we don't need to check the input object for null // NOTE: if needed, this flag can be removed, and we can introduce new _NONNUL cast helpers From 0d359214793beb98f9db3a958a8be6a501782c36 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Fri, 16 Feb 2024 17:51:55 -0500 Subject: [PATCH 06/37] Re-disable Swift call conv --- src/coreclr/jit/importercalls.cpp | 3 ++- src/coreclr/jit/targetamd64.h | 8 ++++---- src/coreclr/jit/targetarm64.h | 8 ++++---- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 5b75c5f5f1283f..8797096e3985aa 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -5707,7 +5707,8 @@ void Compiler::impCheckForPInvokeCall( // return here without inlining the native call. if (unmanagedCallConv == CorInfoCallConvExtension::Managed || unmanagedCallConv == CorInfoCallConvExtension::Fastcall || - unmanagedCallConv == CorInfoCallConvExtension::FastcallMemberFunction) + unmanagedCallConv == CorInfoCallConvExtension::FastcallMemberFunction || + unmanagedCallConv == CorInfoCallConvExtension::Swift) { return; } diff --git a/src/coreclr/jit/targetamd64.h b/src/coreclr/jit/targetamd64.h index 147355c2474a24..ddd46664262d14 100644 --- a/src/coreclr/jit/targetamd64.h +++ b/src/coreclr/jit/targetamd64.h @@ -563,9 +563,9 @@ #define RBM_STACK_PROBE_HELPER_TRASH RBM_RAX #endif // !UNIX_AMD64_ABI - #define SWIFT_SUPPORT - #define REG_SWIFT_ERROR REG_R12 - #define RBM_SWIFT_ERROR RBM_R12 - #define SWIFT_SELF_REG REG_R13 + // #define SWIFT_SUPPORT + // #define REG_SWIFT_ERROR REG_R12 + // #define RBM_SWIFT_ERROR RBM_R12 + // #define SWIFT_SELF_REG REG_R13 // clang-format on diff --git a/src/coreclr/jit/targetarm64.h b/src/coreclr/jit/targetarm64.h index 74a14535f15075..32e45928fd4596 100644 --- a/src/coreclr/jit/targetarm64.h +++ b/src/coreclr/jit/targetarm64.h @@ -370,9 +370,9 @@ #define REG_ZERO_INIT_FRAME_REG2 REG_R10 #define REG_ZERO_INIT_FRAME_SIMD REG_V16 - #define SWIFT_SUPPORT - #define REG_SWIFT_ERROR REG_R21 - #define RBM_SWIFT_ERROR RBM_R21 - #define SWIFT_SELF_REG REG_R20 + // #define SWIFT_SUPPORT + // #define REG_SWIFT_ERROR REG_R21 + // #define RBM_SWIFT_ERROR RBM_R21 + // #define SWIFT_SELF_REG REG_R20 // clang-format on From d1618c3616014aab367006c3b7a5d9a656517099 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Fri, 16 Feb 2024 17:53:08 -0500 Subject: [PATCH 07/37] Fix comment --- src/coreclr/jit/importercalls.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 8797096e3985aa..9b3c3d192b2dc4 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -99,7 +99,7 @@ var_types Compiler::impImportCall(OPCODE opcode, NewCallArg extraArg; #ifdef SWIFT_SUPPORT - // Swift calls may use special register types that require additional IL, + // Swift calls may use special register types that require additional IR to handle, // so if we're importing a Swift call, look for these types in the signature int swiftErrorIndex = -1; #endif // SWIFT_SUPPORT From 19668284a10de03e6995334fcf8be7f876f99e5f Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Fri, 16 Feb 2024 17:53:47 -0500 Subject: [PATCH 08/37] Fix another comment --- src/coreclr/jit/importercalls.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 9b3c3d192b2dc4..0be116634086f7 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -675,7 +675,6 @@ var_types Compiler::impImportCall(OPCODE opcode, #ifdef SWIFT_SUPPORT // We are importing an unmanaged Swift call, which might require special parameter handling: // - SwiftError* is passed to capture the address of an error thrown during the Swift call. - // - SwiftSelf is passed to enable calling an instance method of a Swift object. if (call->AsCall()->unmgdCallConv == CorInfoCallConvExtension::Swift) { // Check the signature of the Swift call for the special types. From 62fb26a5d30d1c3fef2f6a92b28f3dbdef51687f Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Fri, 16 Feb 2024 18:03:32 -0500 Subject: [PATCH 09/37] Kill error reg only if Swift method has error handling --- src/coreclr/jit/lsrabuild.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index 68bcddda62c40b..83aa30020e9ea0 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -883,9 +883,12 @@ regMaskTP LinearScan::getKillSetForCall(GenTreeCall* call) #ifdef SWIFT_SUPPORT // Swift calls that throw may trash the callee-saved error register, - // so don't use the register post-call until it is consumed by SwiftError (if ever) - if (call->unmgdCallConv == CorInfoCallConvExtension::Swift) + // so don't use the register post-call until it is consumed by SwiftError. + // GTF_CALL_M_SWIFT_ERROR_HANDLING indicates the call has a SwiftError* argument, + // so the error register value will eventually be consumed post-call. + if ((call->gtCallMoreFlags & GTF_CALL_M_SWIFT_ERROR_HANDLING) != 0) { + assert(call->unmgdCallConv == CorInfoCallConvExtension::Swift); killMask |= RBM_SWIFT_ERROR; } #endif // SWIFT_SUPPORT From 197f704e62eb4f7320c48e151a18757b32398fd9 Mon Sep 17 00:00:00 2001 From: Aman Khalid Date: Fri, 16 Feb 2024 18:32:01 -0500 Subject: [PATCH 10/37] Add GTF_CALL, GTF_GLOB_REF flags to errorRegNode Co-authored-by: Jakob Botsch Nielsen --- src/coreclr/jit/importercalls.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 0be116634086f7..514ddbfe9d3c0c 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -1561,6 +1561,7 @@ var_types Compiler::impImportCall(OPCODE opcode, // Store the error register value to where the SwiftError* points to GenTree* errorRegNode = new (this, GT_SWIFT_ERROR) GenTree(GT_SWIFT_ERROR, TYP_REF); errorRegNode->SetHasOrderingSideEffect(); + errorRegNode->gtFlags |= GTF_CALL | GTF_GLOB_REF; GenTree* argNodeCopy = gtNewLclvNode(argNode->AsLclVar()->GetLclNum(), argNode->TypeGet()); GenTreeStoreInd* swiftErrorStore = gtNewStoreIndNode(argNodeCopy->TypeGet(), argNodeCopy, errorRegNode); From 83b052496b3783c18c83ed34c39505fb30b8b0a4 Mon Sep 17 00:00:00 2001 From: Aman Khalid Date: Fri, 16 Feb 2024 18:32:19 -0500 Subject: [PATCH 11/37] Use TYP_I_IMPL Co-authored-by: Jakob Botsch Nielsen --- src/coreclr/jit/importercalls.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 514ddbfe9d3c0c..62acd450665ec2 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -1559,7 +1559,7 @@ var_types Compiler::impImportCall(OPCODE opcode, assert(argNode->OperIs(GT_LCL_VAR)); // Store the error register value to where the SwiftError* points to - GenTree* errorRegNode = new (this, GT_SWIFT_ERROR) GenTree(GT_SWIFT_ERROR, TYP_REF); + GenTree* errorRegNode = new (this, GT_SWIFT_ERROR) GenTree(GT_SWIFT_ERROR, TYP_I_IMPL); errorRegNode->SetHasOrderingSideEffect(); errorRegNode->gtFlags |= GTF_CALL | GTF_GLOB_REF; From 984edf6accc499ec54ccc50a8aff4f7cfbae4994 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Fri, 16 Feb 2024 19:00:51 -0500 Subject: [PATCH 12/37] add genCodeForSwiftErrorReg --- src/coreclr/jit/codegen.h | 3 +++ src/coreclr/jit/codegenarmarch.cpp | 22 +++++++++++++++++++++- src/coreclr/jit/codegenxarch.cpp | 22 +++++++++++++++++++++- src/coreclr/jit/importercalls.cpp | 4 ++-- src/coreclr/jit/targetamd64.h | 8 ++++---- src/coreclr/jit/targetarm64.h | 8 ++++---- 6 files changed, 55 insertions(+), 12 deletions(-) diff --git a/src/coreclr/jit/codegen.h b/src/coreclr/jit/codegen.h index afd9e42a9d2cdd..a3a9c3c411963c 100644 --- a/src/coreclr/jit/codegen.h +++ b/src/coreclr/jit/codegen.h @@ -1186,6 +1186,9 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX void genCodeForCpBlkHelper(GenTreeBlk* cpBlkNode); #endif void genCodeForPhysReg(GenTreePhysReg* tree); +#ifdef SWIFT_SUPPORT + void genCodeForSwiftErrorReg(GenTree* tree); +#endif // SWIFT_SUPPORT void genCodeForNullCheck(GenTreeIndir* tree); void genCodeForCmpXchg(GenTreeCmpXchg* tree); void genCodeForReuseVal(GenTree* treeNode); diff --git a/src/coreclr/jit/codegenarmarch.cpp b/src/coreclr/jit/codegenarmarch.cpp index 442a71ea95390a..7e2e15f8fbaed8 100644 --- a/src/coreclr/jit/codegenarmarch.cpp +++ b/src/coreclr/jit/codegenarmarch.cpp @@ -443,7 +443,7 @@ void CodeGen::genCodeForTreeNode(GenTree* treeNode) #ifdef SWIFT_SUPPORT case GT_SWIFT_ERROR: - treeNode->SetRegNum(REG_SWIFT_ERROR); + genCodeForSwiftErrorReg(treeNode); break; #endif // SWIFT_SUPPORT @@ -1524,6 +1524,26 @@ void CodeGen::genCodeForPhysReg(GenTreePhysReg* tree) genProduceReg(tree); } +#ifdef SWIFT_SUPPORT +//--------------------------------------------------------------------- +// genCodeForSwiftErrorReg - generate code for a GT_SWIFT_ERROR node +// +// Arguments +// tree - the GT_SWIFT_ERROR node +// +// Return value: +// None +// +void CodeGen::genCodeForSwiftErrorReg(GenTree* tree) +{ + assert(tree->OperIs(GT_SWIFT_ERROR)); + + tree->SetRegNum(REG_SWIFT_ERROR); + + genProduceReg(tree); +} +#endif // SWIFT_SUPPORT + //--------------------------------------------------------------------- // genCodeForNullCheck - generate code for a GT_NULLCHECK node // diff --git a/src/coreclr/jit/codegenxarch.cpp b/src/coreclr/jit/codegenxarch.cpp index 6e0fd51194c7bb..e1fe3ffd701acd 100644 --- a/src/coreclr/jit/codegenxarch.cpp +++ b/src/coreclr/jit/codegenxarch.cpp @@ -2109,7 +2109,7 @@ void CodeGen::genCodeForTreeNode(GenTree* treeNode) #ifdef SWIFT_SUPPORT case GT_SWIFT_ERROR: - treeNode->SetRegNum(REG_SWIFT_ERROR); + genCodeForSwiftErrorReg(treeNode); break; #endif // SWIFT_SUPPORT @@ -4683,6 +4683,26 @@ void CodeGen::genCodeForPhysReg(GenTreePhysReg* tree) genProduceReg(tree); } +#ifdef SWIFT_SUPPORT +//--------------------------------------------------------------------- +// genCodeForSwiftErrorReg - generate code for a GT_SWIFT_ERROR node +// +// Arguments +// tree - the GT_SWIFT_ERROR node +// +// Return value: +// None +// +void CodeGen::genCodeForSwiftErrorReg(GenTree* tree) +{ + assert(tree->OperIs(GT_SWIFT_ERROR)); + + tree->SetRegNum(REG_SWIFT_ERROR); + + genProduceReg(tree); +} +#endif // SWIFT_SUPPORT + //--------------------------------------------------------------------- // genCodeForNullCheck - generate code for a GT_NULLCHECK node // diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 0be116634086f7..da2472009de6f0 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -5706,8 +5706,8 @@ void Compiler::impCheckForPInvokeCall( // return here without inlining the native call. if (unmanagedCallConv == CorInfoCallConvExtension::Managed || unmanagedCallConv == CorInfoCallConvExtension::Fastcall || - unmanagedCallConv == CorInfoCallConvExtension::FastcallMemberFunction || - unmanagedCallConv == CorInfoCallConvExtension::Swift) + unmanagedCallConv == CorInfoCallConvExtension::FastcallMemberFunction)// || + //unmanagedCallConv == CorInfoCallConvExtension::Swift) { return; } diff --git a/src/coreclr/jit/targetamd64.h b/src/coreclr/jit/targetamd64.h index ddd46664262d14..147355c2474a24 100644 --- a/src/coreclr/jit/targetamd64.h +++ b/src/coreclr/jit/targetamd64.h @@ -563,9 +563,9 @@ #define RBM_STACK_PROBE_HELPER_TRASH RBM_RAX #endif // !UNIX_AMD64_ABI - // #define SWIFT_SUPPORT - // #define REG_SWIFT_ERROR REG_R12 - // #define RBM_SWIFT_ERROR RBM_R12 - // #define SWIFT_SELF_REG REG_R13 + #define SWIFT_SUPPORT + #define REG_SWIFT_ERROR REG_R12 + #define RBM_SWIFT_ERROR RBM_R12 + #define SWIFT_SELF_REG REG_R13 // clang-format on diff --git a/src/coreclr/jit/targetarm64.h b/src/coreclr/jit/targetarm64.h index 32e45928fd4596..74a14535f15075 100644 --- a/src/coreclr/jit/targetarm64.h +++ b/src/coreclr/jit/targetarm64.h @@ -370,9 +370,9 @@ #define REG_ZERO_INIT_FRAME_REG2 REG_R10 #define REG_ZERO_INIT_FRAME_SIMD REG_V16 - // #define SWIFT_SUPPORT - // #define REG_SWIFT_ERROR REG_R21 - // #define RBM_SWIFT_ERROR RBM_R21 - // #define SWIFT_SELF_REG REG_R20 + #define SWIFT_SUPPORT + #define REG_SWIFT_ERROR REG_R21 + #define RBM_SWIFT_ERROR RBM_R21 + #define SWIFT_SELF_REG REG_R20 // clang-format on From 8152cad313caea7ecab4e9b36607d985a6a89a86 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Fri, 16 Feb 2024 19:01:26 -0500 Subject: [PATCH 13/37] Remove SwiftError* call arg after adding IR for it --- src/coreclr/jit/importercalls.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index da2472009de6f0..c15c0a1fbaae76 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -1568,6 +1568,9 @@ var_types Compiler::impImportCall(OPCODE opcode, // Indicate the error register will be checked after this call returns call->AsCall()->gtCallMoreFlags |= GTF_CALL_M_SWIFT_ERROR_HANDLING; + + // Swift call isn't going to use the SwiftError* arg, so don't bother emitting it + call->AsCall()->gtArgs.Remove(errorArg); } #endif // SWIFT_SUPPORT From dc2268da5f3da60e59a8281077e8b2e810dc4e56 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Fri, 16 Feb 2024 19:38:32 -0500 Subject: [PATCH 14/37] Remove sig parsing to impPopArgsForUnmanagedCall --- src/coreclr/jit/codegenxarch.cpp | 9 +- src/coreclr/jit/compiler.h | 2 +- src/coreclr/jit/importercalls.cpp | 150 ++++++++++++++++-------------- 3 files changed, 91 insertions(+), 70 deletions(-) diff --git a/src/coreclr/jit/codegenxarch.cpp b/src/coreclr/jit/codegenxarch.cpp index e1fe3ffd701acd..d1d7448de1d6d0 100644 --- a/src/coreclr/jit/codegenxarch.cpp +++ b/src/coreclr/jit/codegenxarch.cpp @@ -4697,7 +4697,14 @@ void CodeGen::genCodeForSwiftErrorReg(GenTree* tree) { assert(tree->OperIs(GT_SWIFT_ERROR)); - tree->SetRegNum(REG_SWIFT_ERROR); + var_types targetType = tree->TypeGet(); + regNumber targetReg = tree->GetRegNum(); + + + assert(targetReg == REG_SWIFT_ERROR); + + inst_Mov(targetType, targetReg, REG_SWIFT_ERROR, /* canSkip */ true); + genTransferRegGCState(targetReg, REG_SWIFT_ERROR); genProduceReg(tree); } diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 53a5d1d51c796e..588c06ec5b60ce 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -4363,7 +4363,7 @@ class Compiler void impCheckForPInvokeCall( GenTreeCall* call, CORINFO_METHOD_HANDLE methHnd, CORINFO_SIG_INFO* sig, unsigned mflags, BasicBlock* block); GenTreeCall* impImportIndirectCall(CORINFO_SIG_INFO* sig, const DebugInfo& di = DebugInfo()); - void impPopArgsForUnmanagedCall(GenTreeCall* call, CORINFO_SIG_INFO* sig, const bool spillAllArgs); + void impPopArgsForUnmanagedCall(GenTreeCall* call, CORINFO_SIG_INFO* sig, /* OUT */ CallArg** swiftErrorArg, /* OUT */ CallArg** swiftSelfArg); void impInsertHelperCall(CORINFO_HELPER_DESC* helperCall); void impHandleAccessAllowed(CorInfoIsAccessAllowedResult result, CORINFO_HELPER_DESC* helperCall); diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index c15c0a1fbaae76..23ec1b7a8c7cba 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -98,11 +98,10 @@ var_types Compiler::impImportCall(OPCODE opcode, CORINFO_SIG_INFO calliSig; NewCallArg extraArg; -#ifdef SWIFT_SUPPORT // Swift calls may use special register types that require additional IR to handle, // so if we're importing a Swift call, look for these types in the signature - int swiftErrorIndex = -1; -#endif // SWIFT_SUPPORT + CallArg* swiftErrorArg = nullptr; + CallArg* swiftSelfArg = nullptr; /*------------------------------------------------------------------------- * First create the call node @@ -670,57 +669,8 @@ var_types Compiler::impImportCall(OPCODE opcode, } checkForSmallType = true; - bool spillAllArgs = false; - -#ifdef SWIFT_SUPPORT - // We are importing an unmanaged Swift call, which might require special parameter handling: - // - SwiftError* is passed to capture the address of an error thrown during the Swift call. - if (call->AsCall()->unmgdCallConv == CorInfoCallConvExtension::Swift) - { - // Check the signature of the Swift call for the special types. - CORINFO_ARG_LIST_HANDLE sigArg = sig->args; - - for (unsigned short argIndex = 0; argIndex < sig->numArgs; - sigArg = info.compCompHnd->getArgNext(sigArg), argIndex++) - { - CORINFO_CLASS_HANDLE argClass; - CorInfoType argType = strip(info.compCompHnd->getArgType(sig, sigArg, &argClass)); - bool argIsByrefOrPtr = false; - if ((argType == CORINFO_TYPE_BYREF) || (argType == CORINFO_TYPE_PTR)) - { - argClass = info.compCompHnd->getArgClass(sig, sigArg); - argType = info.compCompHnd->getChildType(argClass, &argClass); - argIsByrefOrPtr = true; - } - - if ((argType != CORINFO_TYPE_VALUECLASS) || !info.compCompHnd->isIntrinsicType(argClass)) - { - continue; - } - - const char* namespaceName; - const char* className = info.compCompHnd->getClassNameFromMetadata(argClass, &namespaceName); - - if ((strcmp(className, "SwiftError") == 0) && - (strcmp(namespaceName, "System.Runtime.InteropServices.Swift") == 0)) - { - // For error handling purposes, we expect a pointer to a SwiftError to be passed - assert(argIsByrefOrPtr); - if (swiftErrorIndex != -1) - { - BADCODE("Duplicate SwiftError* parameter"); - } - - swiftErrorIndex = argIndex; - spillAllArgs = true; - } - // TODO: Handle SwiftSelf, SwiftAsync - } - } -#endif // SWIFT_SUPPORT - - impPopArgsForUnmanagedCall(call->AsCall(), sig, spillAllArgs); + impPopArgsForUnmanagedCall(call->AsCall(), sig, &swiftErrorArg, &swiftSelfArg); goto DONE; } @@ -1543,16 +1493,13 @@ var_types Compiler::impImportCall(OPCODE opcode, } #ifdef SWIFT_SUPPORT - if (swiftErrorIndex >= 0) + if (swiftErrorArg != nullptr) { // This Swift call uses the error register assert(call->IsCall()); assert(call->AsCall()->unmgdCallConv == CorInfoCallConvExtension::Swift); - assert(swiftErrorIndex < (int)sig->numArgs); - CallArg* errorArg = call->AsCall()->gtArgs.GetArgByIndex(swiftErrorIndex); - assert(errorArg != nullptr); - GenTree* argNode = errorArg->GetNode(); + GenTree* argNode = swiftErrorArg->GetNode(); assert(argNode != nullptr); // SwiftError* arg should have been spilled to a local temp variable @@ -1570,7 +1517,7 @@ var_types Compiler::impImportCall(OPCODE opcode, call->AsCall()->gtCallMoreFlags |= GTF_CALL_M_SWIFT_ERROR_HANDLING; // Swift call isn't going to use the SwiftError* arg, so don't bother emitting it - call->AsCall()->gtArgs.Remove(errorArg); + call->AsCall()->gtArgs.Remove(swiftErrorArg); } #endif // SWIFT_SUPPORT @@ -1911,7 +1858,7 @@ GenTreeCall* Compiler::impImportIndirectCall(CORINFO_SIG_INFO* sig, const DebugI /*****************************************************************************/ -void Compiler::impPopArgsForUnmanagedCall(GenTreeCall* call, CORINFO_SIG_INFO* sig, const bool spillAllArgs) +void Compiler::impPopArgsForUnmanagedCall(GenTreeCall* call, CORINFO_SIG_INFO* sig, /* OUT */ CallArg** swiftErrorArg, /* OUT */ CallArg** swiftSelfArg) { assert(call->gtFlags & GTF_CALL_UNMANAGED); @@ -1935,19 +1882,73 @@ void Compiler::impPopArgsForUnmanagedCall(GenTreeCall* call, CORINFO_SIG_INFO* s argsToReverse--; } -#ifndef TARGET_X86 - // Don't reverse args on ARM or x64 - first four args always placed in regs in order - argsToReverse = 0; +#ifdef SWIFT_SUPPORT + unsigned short swiftErrorIndex = sig->numArgs; - // Fast path for spilling all args - if (spillAllArgs) + // We are importing an unmanaged Swift call, which might require special parameter handling: + if (call->unmgdCallConv == CorInfoCallConvExtension::Swift) { - for (unsigned level = 0; level < verCurrentState.esStackDepth; level++) + bool spillAllArgs = false; + + // Check the signature of the Swift call for the special types. + CORINFO_ARG_LIST_HANDLE sigArg = sig->args; + + for (unsigned short argIndex = 0; argIndex < sig->numArgs; + sigArg = info.compCompHnd->getArgNext(sigArg), argIndex++) { - impSpillStackEntry(level, - BAD_VAR_NUM DEBUGARG(false) DEBUGARG("impPopArgsForUnmanagedCall - spillAllArgs=true")); + CORINFO_CLASS_HANDLE argClass; + CorInfoType argType = strip(info.compCompHnd->getArgType(sig, sigArg, &argClass)); + bool argIsByrefOrPtr = false; + + if ((argType == CORINFO_TYPE_BYREF) || (argType == CORINFO_TYPE_PTR)) + { + argClass = info.compCompHnd->getArgClass(sig, sigArg); + argType = info.compCompHnd->getChildType(argClass, &argClass); + argIsByrefOrPtr = true; + } + + if ((argType != CORINFO_TYPE_VALUECLASS) || !info.compCompHnd->isIntrinsicType(argClass)) + { + continue; + } + + const char* namespaceName; + const char* className = info.compCompHnd->getClassNameFromMetadata(argClass, &namespaceName); + + if ((strcmp(className, "SwiftError") == 0) && + (strcmp(namespaceName, "System.Runtime.InteropServices.Swift") == 0)) + { + // For error handling purposes, we expect a pointer to a SwiftError to be passed + assert(argIsByrefOrPtr); + if (swiftErrorIndex != sig->numArgs) + { + BADCODE("Duplicate SwiftError* parameter"); + } + + swiftErrorIndex = argIndex; + spillAllArgs = true; + } + // TODO: Handle SwiftSelf, SwiftAsync + } + + // Don't need to reverse args for Swift calls + argsToReverse = 0; + + // If using one of the Swift register types, spill all args to the stack + if (spillAllArgs) + { + for (unsigned level = 0; level < verCurrentState.esStackDepth; level++) + { + impSpillStackEntry(level, + BAD_VAR_NUM DEBUGARG(false) DEBUGARG("impPopArgsForUnmanagedCall - spillAllArgs=true")); + } } } +#endif // SWIFT_SUPPORT + +#ifndef TARGET_X86 + // Don't reverse args on ARM or x64 - first four args always placed in regs in order + argsToReverse = 0; #endif for (unsigned level = verCurrentState.esStackDepth - argsToReverse; level < verCurrentState.esStackDepth; level++) @@ -1991,6 +1992,7 @@ void Compiler::impPopArgsForUnmanagedCall(GenTreeCall* call, CORINFO_SIG_INFO* s assert(thisPtr->TypeGet() == TYP_I_IMPL || thisPtr->TypeGet() == TYP_BYREF); } + unsigned short argIndex = 0; for (CallArg& arg : call->gtArgs.Args()) { GenTree* argNode = arg.GetEarlyNode(); @@ -2013,6 +2015,18 @@ void Compiler::impPopArgsForUnmanagedCall(GenTreeCall* call, CORINFO_SIG_INFO* s assert(!"*** invalid IL: gc ref passed to unmanaged call"); } } + +#ifdef SWIFT_SUPPORT + if (argIndex == swiftErrorIndex) + { + // Found the SwiftError* arg + assert(swiftErrorArg != nullptr); + *swiftErrorArg = &arg; + } + // TODO: SwiftSelf, SwiftAsync +#endif // SWIFT_SUPPORT + + argIndex++; } } From 8f92c30697071feb69fdc1efa699b4cede74c209 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Fri, 16 Feb 2024 19:58:57 -0500 Subject: [PATCH 15/37] Add impAppendSwiftErrorStore --- src/coreclr/jit/compiler.h | 4 ++ src/coreclr/jit/gentree.cpp | 3 ++ src/coreclr/jit/importercalls.cpp | 68 +++++++++++++++++++------------ 3 files changed, 50 insertions(+), 25 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 588c06ec5b60ce..f2fda2592b34db 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -4365,6 +4365,10 @@ class Compiler GenTreeCall* impImportIndirectCall(CORINFO_SIG_INFO* sig, const DebugInfo& di = DebugInfo()); void impPopArgsForUnmanagedCall(GenTreeCall* call, CORINFO_SIG_INFO* sig, /* OUT */ CallArg** swiftErrorArg, /* OUT */ CallArg** swiftSelfArg); +#ifdef SWIFT_SUPPORT + void impAppendSwiftErrorStore(GenTreeCall* call, CallArg* const swiftErrorArg); +#endif // SWIFT_SUPPORT + void impInsertHelperCall(CORINFO_HELPER_DESC* helperCall); void impHandleAccessAllowed(CorInfoIsAccessAllowedResult result, CORINFO_HELPER_DESC* helperCall); void impHandleAccessAllowedInternal(CorInfoIsAccessAllowedResult result, CORINFO_HELPER_DESC* helperCall); diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index a0b7306e214408..aa695c466a0654 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -7066,6 +7066,9 @@ bool GenTree::OperRequiresCallFlag(Compiler* comp) const case GT_KEEPALIVE: return true; + case GT_SWIFT_ERROR: + return true; + case GT_INTRINSIC: return comp->IsIntrinsicImplementedByUserCall(this->AsIntrinsic()->gtIntrinsicName); diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 23ec1b7a8c7cba..cf2b38131f36ec 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -1493,31 +1493,11 @@ var_types Compiler::impImportCall(OPCODE opcode, } #ifdef SWIFT_SUPPORT + // If call is a Swift call with error handling, append additional IR + // to handle storing the error register's value post-call. if (swiftErrorArg != nullptr) { - // This Swift call uses the error register - assert(call->IsCall()); - assert(call->AsCall()->unmgdCallConv == CorInfoCallConvExtension::Swift); - - GenTree* argNode = swiftErrorArg->GetNode(); - assert(argNode != nullptr); - - // SwiftError* arg should have been spilled to a local temp variable - assert(argNode->OperIs(GT_LCL_VAR)); - - // Store the error register value to where the SwiftError* points to - GenTree* errorRegNode = new (this, GT_SWIFT_ERROR) GenTree(GT_SWIFT_ERROR, TYP_REF); - errorRegNode->SetHasOrderingSideEffect(); - - GenTree* argNodeCopy = gtNewLclvNode(argNode->AsLclVar()->GetLclNum(), argNode->TypeGet()); - GenTreeStoreInd* swiftErrorStore = gtNewStoreIndNode(argNodeCopy->TypeGet(), argNodeCopy, errorRegNode); - impAppendTree(swiftErrorStore, CHECK_SPILL_ALL, impCurStmtDI, false); - - // Indicate the error register will be checked after this call returns - call->AsCall()->gtCallMoreFlags |= GTF_CALL_M_SWIFT_ERROR_HANDLING; - - // Swift call isn't going to use the SwiftError* arg, so don't bother emitting it - call->AsCall()->gtArgs.Remove(swiftErrorArg); + impAppendSwiftErrorStore(call->AsCall(), swiftErrorArg); } #endif // SWIFT_SUPPORT @@ -1885,12 +1865,12 @@ void Compiler::impPopArgsForUnmanagedCall(GenTreeCall* call, CORINFO_SIG_INFO* s #ifdef SWIFT_SUPPORT unsigned short swiftErrorIndex = sig->numArgs; - // We are importing an unmanaged Swift call, which might require special parameter handling: + // We are importing an unmanaged Swift call, which might require special parameter handling if (call->unmgdCallConv == CorInfoCallConvExtension::Swift) { bool spillAllArgs = false; - // Check the signature of the Swift call for the special types. + // Check the signature of the Swift call for the special types CORINFO_ARG_LIST_HANDLE sigArg = sig->args; for (unsigned short argIndex = 0; argIndex < sig->numArgs; @@ -2030,6 +2010,44 @@ void Compiler::impPopArgsForUnmanagedCall(GenTreeCall* call, CORINFO_SIG_INFO* s } } +#ifdef SWIFT_SUPPORT +//------------------------------------------------------------------------ +// impAppendSwiftErrorStore: Append IR to store the Swift error register value +// to the SwiftError* argument specified by swiftErrorArg, post-Swift call +// +// Arguments: +// call - the Swift call +// swiftErrorArg - the SwiftError* argument passed to call +// +void Compiler::impAppendSwiftErrorStore(GenTreeCall* call, CallArg* const swiftErrorArg) +{ + assert(call != nullptr); + assert(call->unmgdCallConv == CorInfoCallConvExtension::Swift); + assert(swiftErrorArg != nullptr); + + GenTree* const argNode = swiftErrorArg->GetNode(); + assert(argNode != nullptr); + + // SwiftError* arg should have been spilled to a local temp variable + assert(argNode->OperIs(GT_LCL_VAR)); + + // Store the error register value to where the SwiftError* points to + GenTree* errorRegNode = new (this, GT_SWIFT_ERROR) GenTree(GT_SWIFT_ERROR, TYP_I_IMPL); + errorRegNode->SetHasOrderingSideEffect(); + errorRegNode->gtFlags |= (GTF_CALL | GTF_GLOB_REF); + + GenTree* argNodeCopy = gtNewLclvNode(argNode->AsLclVar()->GetLclNum(), argNode->TypeGet()); + GenTreeStoreInd* swiftErrorStore = gtNewStoreIndNode(argNodeCopy->TypeGet(), argNodeCopy, errorRegNode); + impAppendTree(swiftErrorStore, CHECK_SPILL_ALL, impCurStmtDI, false); + + // Indicate the error register will be checked after this call returns + call->gtCallMoreFlags |= GTF_CALL_M_SWIFT_ERROR_HANDLING; + + // Swift call isn't going to use the SwiftError* arg, so don't bother emitting it + call->gtArgs.Remove(swiftErrorArg); +} +#endif // SWIFT_SUPPORT + //------------------------------------------------------------------------ // impInitializeArrayIntrinsic: Attempts to replace a call to InitializeArray // with a GT_COPYBLK node. From 61851e1041d1bc5765a6ba4fdb4ffbd22e98acb1 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Fri, 16 Feb 2024 20:09:51 -0500 Subject: [PATCH 16/37] Add comment explaining GT_SWIFT_ERROR dest reg --- src/coreclr/jit/codegenarmarch.cpp | 20 -------------------- src/coreclr/jit/codegencommon.cpp | 28 ++++++++++++++++++++++++++++ src/coreclr/jit/codegenxarch.cpp | 27 --------------------------- src/coreclr/jit/lsraarm64.cpp | 6 ++++++ src/coreclr/jit/lsraxarch.cpp | 6 ++++++ 5 files changed, 40 insertions(+), 47 deletions(-) diff --git a/src/coreclr/jit/codegenarmarch.cpp b/src/coreclr/jit/codegenarmarch.cpp index 7e2e15f8fbaed8..bd3d70b9e3278a 100644 --- a/src/coreclr/jit/codegenarmarch.cpp +++ b/src/coreclr/jit/codegenarmarch.cpp @@ -1524,26 +1524,6 @@ void CodeGen::genCodeForPhysReg(GenTreePhysReg* tree) genProduceReg(tree); } -#ifdef SWIFT_SUPPORT -//--------------------------------------------------------------------- -// genCodeForSwiftErrorReg - generate code for a GT_SWIFT_ERROR node -// -// Arguments -// tree - the GT_SWIFT_ERROR node -// -// Return value: -// None -// -void CodeGen::genCodeForSwiftErrorReg(GenTree* tree) -{ - assert(tree->OperIs(GT_SWIFT_ERROR)); - - tree->SetRegNum(REG_SWIFT_ERROR); - - genProduceReg(tree); -} -#endif // SWIFT_SUPPORT - //--------------------------------------------------------------------- // genCodeForNullCheck - generate code for a GT_NULLCHECK node // diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index c468473067b6e1..8da9f7dd486719 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -8515,3 +8515,31 @@ void CodeGen::genCodeForReuseVal(GenTree* treeNode) genDefineTempLabel(genCreateTempLabel()); } } + +#ifdef SWIFT_SUPPORT +//--------------------------------------------------------------------- +// genCodeForSwiftErrorReg - generate code for a GT_SWIFT_ERROR node +// +// Arguments +// tree - the GT_SWIFT_ERROR node +// +// Return value: +// None +// +void CodeGen::genCodeForSwiftErrorReg(GenTree* tree) +{ + assert(tree->OperIs(GT_SWIFT_ERROR)); + + var_types targetType = tree->TypeGet(); + regNumber targetReg = tree->GetRegNum(); + + // LSRA should have picked REG_SWIFT_ERROR as the destination register, too + // (see LinearScan::BuildNode for an explanation of why we want this) + assert(targetReg == REG_SWIFT_ERROR); + + inst_Mov(targetType, targetReg, REG_SWIFT_ERROR, /* canSkip */ true); + genTransferRegGCState(targetReg, REG_SWIFT_ERROR); + + genProduceReg(tree); +} +#endif // SWIFT_SUPPORT diff --git a/src/coreclr/jit/codegenxarch.cpp b/src/coreclr/jit/codegenxarch.cpp index d1d7448de1d6d0..83b978219e60ec 100644 --- a/src/coreclr/jit/codegenxarch.cpp +++ b/src/coreclr/jit/codegenxarch.cpp @@ -4683,33 +4683,6 @@ void CodeGen::genCodeForPhysReg(GenTreePhysReg* tree) genProduceReg(tree); } -#ifdef SWIFT_SUPPORT -//--------------------------------------------------------------------- -// genCodeForSwiftErrorReg - generate code for a GT_SWIFT_ERROR node -// -// Arguments -// tree - the GT_SWIFT_ERROR node -// -// Return value: -// None -// -void CodeGen::genCodeForSwiftErrorReg(GenTree* tree) -{ - assert(tree->OperIs(GT_SWIFT_ERROR)); - - var_types targetType = tree->TypeGet(); - regNumber targetReg = tree->GetRegNum(); - - - assert(targetReg == REG_SWIFT_ERROR); - - inst_Mov(targetType, targetReg, REG_SWIFT_ERROR, /* canSkip */ true); - genTransferRegGCState(targetReg, REG_SWIFT_ERROR); - - genProduceReg(tree); -} -#endif // SWIFT_SUPPORT - //--------------------------------------------------------------------- // genCodeForNullCheck - generate code for a GT_NULLCHECK node // diff --git a/src/coreclr/jit/lsraarm64.cpp b/src/coreclr/jit/lsraarm64.cpp index 84e88ed36d5a0d..7ad64b2652b7e4 100644 --- a/src/coreclr/jit/lsraarm64.cpp +++ b/src/coreclr/jit/lsraarm64.cpp @@ -1290,6 +1290,12 @@ int LinearScan::BuildNode(GenTree* tree) // After a Swift call potentially trashes the error register, // the register can be used again only if there is a GT_SWIFT_ERROR node to consume it // (i.e. the register's value is saved to a SwiftError) + + // Any register should do here, but the error register value should immediately + // be moved from GT_SWIFT_ERROR's destination register to the SwiftError struct, + // and we know REG_SWIFT_ERROR should be busy up to this point, anyway. + // By forcing LSRA to use REG_SWIFT_ERROR as both the source and destination register, + // we can ensure the redundant move is elided. addRefsForPhysRegMask(RBM_SWIFT_ERROR, currentLoc, RefTypeKill, true); BuildDef(tree, RBM_SWIFT_ERROR); break; diff --git a/src/coreclr/jit/lsraxarch.cpp b/src/coreclr/jit/lsraxarch.cpp index 66e7fccbd0bc51..f20a5617a9a06b 100644 --- a/src/coreclr/jit/lsraxarch.cpp +++ b/src/coreclr/jit/lsraxarch.cpp @@ -641,6 +641,12 @@ int LinearScan::BuildNode(GenTree* tree) // After a Swift call potentially trashes the error register, // the register can be used again only if there is a GT_SWIFT_ERROR node to consume it // (i.e. the register's value is saved to a SwiftError) + + // Any register should do here, but the error register value should immediately + // be moved from GT_SWIFT_ERROR's destination register to the SwiftError struct, + // and we know REG_SWIFT_ERROR should be busy up to this point, anyway. + // By forcing LSRA to use REG_SWIFT_ERROR as both the source and destination register, + // we can ensure the redundant move is elided. addRefsForPhysRegMask(RBM_SWIFT_ERROR, currentLoc, RefTypeKill, true); BuildDef(tree, RBM_SWIFT_ERROR); break; From 0a0b556e28426097e21646322a16ad708241119f Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Fri, 16 Feb 2024 20:11:21 -0500 Subject: [PATCH 17/37] Style --- src/coreclr/jit/importercalls.cpp | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index cf2b38131f36ec..5888604f6720e0 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -101,7 +101,7 @@ var_types Compiler::impImportCall(OPCODE opcode, // Swift calls may use special register types that require additional IR to handle, // so if we're importing a Swift call, look for these types in the signature CallArg* swiftErrorArg = nullptr; - CallArg* swiftSelfArg = nullptr; + CallArg* swiftSelfArg = nullptr; /*------------------------------------------------------------------------- * First create the call node @@ -1838,7 +1838,10 @@ GenTreeCall* Compiler::impImportIndirectCall(CORINFO_SIG_INFO* sig, const DebugI /*****************************************************************************/ -void Compiler::impPopArgsForUnmanagedCall(GenTreeCall* call, CORINFO_SIG_INFO* sig, /* OUT */ CallArg** swiftErrorArg, /* OUT */ CallArg** swiftSelfArg) +void Compiler::impPopArgsForUnmanagedCall(GenTreeCall* call, + CORINFO_SIG_INFO* sig, + /* OUT */ CallArg** swiftErrorArg, + /* OUT */ CallArg** swiftSelfArg) { assert(call->gtFlags & GTF_CALL_UNMANAGED); @@ -1869,7 +1872,7 @@ void Compiler::impPopArgsForUnmanagedCall(GenTreeCall* call, CORINFO_SIG_INFO* s if (call->unmgdCallConv == CorInfoCallConvExtension::Swift) { bool spillAllArgs = false; - + // Check the signature of the Swift call for the special types CORINFO_ARG_LIST_HANDLE sigArg = sig->args; @@ -1919,8 +1922,8 @@ void Compiler::impPopArgsForUnmanagedCall(GenTreeCall* call, CORINFO_SIG_INFO* s { for (unsigned level = 0; level < verCurrentState.esStackDepth; level++) { - impSpillStackEntry(level, - BAD_VAR_NUM DEBUGARG(false) DEBUGARG("impPopArgsForUnmanagedCall - spillAllArgs=true")); + impSpillStackEntry(level, BAD_VAR_NUM DEBUGARG(false) + DEBUGARG("impPopArgsForUnmanagedCall - spillAllArgs=true")); } } } @@ -2003,7 +2006,7 @@ void Compiler::impPopArgsForUnmanagedCall(GenTreeCall* call, CORINFO_SIG_INFO* s assert(swiftErrorArg != nullptr); *swiftErrorArg = &arg; } - // TODO: SwiftSelf, SwiftAsync +// TODO: SwiftSelf, SwiftAsync #endif // SWIFT_SUPPORT argIndex++; @@ -2024,7 +2027,7 @@ void Compiler::impAppendSwiftErrorStore(GenTreeCall* call, CallArg* const swiftE assert(call != nullptr); assert(call->unmgdCallConv == CorInfoCallConvExtension::Swift); assert(swiftErrorArg != nullptr); - + GenTree* const argNode = swiftErrorArg->GetNode(); assert(argNode != nullptr); @@ -5741,8 +5744,8 @@ void Compiler::impCheckForPInvokeCall( // return here without inlining the native call. if (unmanagedCallConv == CorInfoCallConvExtension::Managed || unmanagedCallConv == CorInfoCallConvExtension::Fastcall || - unmanagedCallConv == CorInfoCallConvExtension::FastcallMemberFunction)// || - //unmanagedCallConv == CorInfoCallConvExtension::Swift) + unmanagedCallConv == CorInfoCallConvExtension::FastcallMemberFunction) // || + // unmanagedCallConv == CorInfoCallConvExtension::Swift) { return; } From f61d406136021b9575e41d71b05352754c83d717 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Fri, 16 Feb 2024 20:55:13 -0500 Subject: [PATCH 18/37] Disable --- src/coreclr/jit/importercalls.cpp | 4 ++-- src/coreclr/jit/targetamd64.h | 8 ++++---- src/coreclr/jit/targetarm64.h | 8 ++++---- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 5888604f6720e0..8bd60ee96b19d0 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -5744,8 +5744,8 @@ void Compiler::impCheckForPInvokeCall( // return here without inlining the native call. if (unmanagedCallConv == CorInfoCallConvExtension::Managed || unmanagedCallConv == CorInfoCallConvExtension::Fastcall || - unmanagedCallConv == CorInfoCallConvExtension::FastcallMemberFunction) // || - // unmanagedCallConv == CorInfoCallConvExtension::Swift) + unmanagedCallConv == CorInfoCallConvExtension::FastcallMemberFunction || + unmanagedCallConv == CorInfoCallConvExtension::Swift) { return; } diff --git a/src/coreclr/jit/targetamd64.h b/src/coreclr/jit/targetamd64.h index 147355c2474a24..ddd46664262d14 100644 --- a/src/coreclr/jit/targetamd64.h +++ b/src/coreclr/jit/targetamd64.h @@ -563,9 +563,9 @@ #define RBM_STACK_PROBE_HELPER_TRASH RBM_RAX #endif // !UNIX_AMD64_ABI - #define SWIFT_SUPPORT - #define REG_SWIFT_ERROR REG_R12 - #define RBM_SWIFT_ERROR RBM_R12 - #define SWIFT_SELF_REG REG_R13 + // #define SWIFT_SUPPORT + // #define REG_SWIFT_ERROR REG_R12 + // #define RBM_SWIFT_ERROR RBM_R12 + // #define SWIFT_SELF_REG REG_R13 // clang-format on diff --git a/src/coreclr/jit/targetarm64.h b/src/coreclr/jit/targetarm64.h index 74a14535f15075..32e45928fd4596 100644 --- a/src/coreclr/jit/targetarm64.h +++ b/src/coreclr/jit/targetarm64.h @@ -370,9 +370,9 @@ #define REG_ZERO_INIT_FRAME_REG2 REG_R10 #define REG_ZERO_INIT_FRAME_SIMD REG_V16 - #define SWIFT_SUPPORT - #define REG_SWIFT_ERROR REG_R21 - #define RBM_SWIFT_ERROR RBM_R21 - #define SWIFT_SELF_REG REG_R20 + // #define SWIFT_SUPPORT + // #define REG_SWIFT_ERROR REG_R21 + // #define RBM_SWIFT_ERROR RBM_R21 + // #define SWIFT_SELF_REG REG_R20 // clang-format on From 33ff3df066bb8b41069236f0f0d14517589636bd Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Mon, 19 Feb 2024 10:09:53 -0500 Subject: [PATCH 19/37] Fix comment --- src/coreclr/jit/importercalls.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 8bd60ee96b19d0..8b269a30540773 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -1871,7 +1871,7 @@ void Compiler::impPopArgsForUnmanagedCall(GenTreeCall* call, // We are importing an unmanaged Swift call, which might require special parameter handling if (call->unmgdCallConv == CorInfoCallConvExtension::Swift) { - bool spillAllArgs = false; + bool checkEntireStack = false; // Check the signature of the Swift call for the special types CORINFO_ARG_LIST_HANDLE sigArg = sig->args; @@ -1908,8 +1908,8 @@ void Compiler::impPopArgsForUnmanagedCall(GenTreeCall* call, BADCODE("Duplicate SwiftError* parameter"); } - swiftErrorIndex = argIndex; - spillAllArgs = true; + swiftErrorIndex = argIndex; + checkEntireStack = true; } // TODO: Handle SwiftSelf, SwiftAsync } @@ -1917,13 +1917,13 @@ void Compiler::impPopArgsForUnmanagedCall(GenTreeCall* call, // Don't need to reverse args for Swift calls argsToReverse = 0; - // If using one of the Swift register types, spill all args to the stack - if (spillAllArgs) + // If using one of the Swift register types, check entire stack for side effects + if (checkEntireStack) { for (unsigned level = 0; level < verCurrentState.esStackDepth; level++) { impSpillStackEntry(level, BAD_VAR_NUM DEBUGARG(false) - DEBUGARG("impPopArgsForUnmanagedCall - spillAllArgs=true")); + DEBUGARG("impPopArgsForUnmanagedCall - checkEntireStack=true")); } } } From 8b574285dedb5471dfb50c118cb8cc31f47a8668 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Mon, 19 Feb 2024 10:15:30 -0500 Subject: [PATCH 20/37] Throw exception for passing SwiftError struct --- src/coreclr/jit/importercalls.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 8b269a30540773..00112b2875898c 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -1901,8 +1901,12 @@ void Compiler::impPopArgsForUnmanagedCall(GenTreeCall* call, if ((strcmp(className, "SwiftError") == 0) && (strcmp(namespaceName, "System.Runtime.InteropServices.Swift") == 0)) { - // For error handling purposes, we expect a pointer to a SwiftError to be passed - assert(argIsByrefOrPtr); + // For error handling purposes, we expect a pointer/reference to a SwiftError to be passed + if (!argIsByrefOrPtr) + { + BADCODE("Expected SwiftError pointer/reference, got value class"); + } + if (swiftErrorIndex != sig->numArgs) { BADCODE("Duplicate SwiftError* parameter"); From 7ce0adb95d024c4f64b3736b295454a873dadc01 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Mon, 19 Feb 2024 10:32:01 -0500 Subject: [PATCH 21/37] Fix value num --- src/coreclr/jit/importercalls.cpp | 2 +- src/coreclr/jit/valuenum.cpp | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 00112b2875898c..e660d0d0d6545e 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -1904,7 +1904,7 @@ void Compiler::impPopArgsForUnmanagedCall(GenTreeCall* call, // For error handling purposes, we expect a pointer/reference to a SwiftError to be passed if (!argIsByrefOrPtr) { - BADCODE("Expected SwiftError pointer/reference, got value class"); + BADCODE("Expected SwiftError pointer/reference, got struct"); } if (swiftErrorIndex != sig->numArgs) diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 6d0fb2d624e5a9..a2914370958b8d 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -11245,7 +11245,9 @@ void Compiler::fgValueNumberTree(GenTree* tree) break; case GT_CATCH_ARG: + case GT_SWIFT_ERROR: // We know nothing about the value of a caught expression. + // We also know nothing about the error register's value post-Swift call. tree->gtVNPair.SetBoth(vnStore->VNForExpr(compCurBB, tree->TypeGet())); break; @@ -11256,7 +11258,6 @@ void Compiler::fgValueNumberTree(GenTree* tree) break; // These do not represent values. - case GT_SWIFT_ERROR: case GT_NO_OP: case GT_NOP: case GT_JMP: // Control flow From 7692346ca7db8a9b7d8c1eea0412671972cad24e Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Tue, 20 Feb 2024 11:37:56 -0500 Subject: [PATCH 22/37] Mark error reg as busy until GT_SWIFT_ERROR consumes it --- src/coreclr/jit/lsra.cpp | 10 ++++++++++ src/coreclr/jit/lsra.h | 4 ++++ src/coreclr/jit/lsrabuild.cpp | 22 ++++++++++++++++++++++ 3 files changed, 36 insertions(+) diff --git a/src/coreclr/jit/lsra.cpp b/src/coreclr/jit/lsra.cpp index 04ca6149c9fc36..7b80bcc080c6a9 100644 --- a/src/coreclr/jit/lsra.cpp +++ b/src/coreclr/jit/lsra.cpp @@ -5121,6 +5121,11 @@ void LinearScan::allocateRegistersMinimal() } clearRegBusyUntilKill(regRecord->regNum); INDEBUG(dumpLsraAllocationEvent(LSRA_EVENT_KEPT_ALLOCATION, nullptr, regRecord->regNum)); + + if (currentRefPosition.busyUntilNextKill) + { + setRegBusyUntilKill(regRecord->regNum, regRecord->registerType); + } } continue; } @@ -5830,6 +5835,11 @@ void LinearScan::allocateRegisters() } clearRegBusyUntilKill(regRecord->regNum); INDEBUG(dumpLsraAllocationEvent(LSRA_EVENT_KEPT_ALLOCATION, nullptr, regRecord->regNum)); + + if (currentRefPosition.busyUntilNextKill) + { + setRegBusyUntilKill(regRecord->regNum, regRecord->registerType); + } } continue; } diff --git a/src/coreclr/jit/lsra.h b/src/coreclr/jit/lsra.h index c0e0f5d2fdbd34..2aedc80d054922 100644 --- a/src/coreclr/jit/lsra.h +++ b/src/coreclr/jit/lsra.h @@ -2519,6 +2519,9 @@ class RefPosition unsigned char skipSaveRestore : 1; #endif + // For a phys reg, indicates that it should be marked as busy until the next time it is killed. + unsigned char busyUntilNextKill : 1; + #ifdef DEBUG // Minimum number registers that needs to be ensured while // constraining candidates for this ref position under @@ -2562,6 +2565,7 @@ class RefPosition , isLocalDefUse(false) , delayRegFree(false) , outOfOrder(false) + , busyUntilNextKill(false) #ifdef DEBUG , minRegCandidateCount(1) , rpNum(0) diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index 83aa30020e9ea0..bfa78236f9a99f 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -1221,6 +1221,28 @@ bool LinearScan::buildKillPositionsForNode(GenTree* tree, LsraLocation currentLo insertedKills = true; } +#ifdef SWIFT_SUPPORT + if (tree->IsCall() && ((tree->AsCall()->gtCallMoreFlags & GTF_CALL_M_SWIFT_ERROR_HANDLING) != 0)) + { + // Tree is a Swift call with error handling; error register should have been killed + assert(tree->AsCall()->unmgdCallConv == CorInfoCallConvExtension::Swift); + assert((killMask & RBM_SWIFT_ERROR) != 0); + + // After a Swift call that might throw returns, we expect the error register to be consumed + // by a GT_SWIFT_ERROR node. However, we want to ensure the error register won't be trashed + // before GT_SWIFT_ERROR can consume it + // (for example, the PInvoke epilog comes before the error register store). + // To do so, mark the register as "busy until next kill". + // This means the register will remain busy from the Swift call until the GT_SWIFT_ERROR kills it. + + RegRecord* swiftErrorRegRec = getRegisterRecord(REG_SWIFT_ERROR); + RefPosition* pos = swiftErrorRegRec->lastRefPosition; + assert(pos != nullptr); + assert(pos->refType = RefTypeKill); + pos->busyUntilNextKill = true; + } +#endif // SWIFT_SUPPORT + return insertedKills; } From 24b4beca0ddeb539a05a860cc658ce82e4417491 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Tue, 20 Feb 2024 11:39:33 -0500 Subject: [PATCH 23/37] enable Swift call conv in JIT --- src/coreclr/jit/importercalls.cpp | 3 +-- src/coreclr/jit/targetamd64.h | 8 ++++---- src/coreclr/jit/targetarm64.h | 8 ++++---- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index e660d0d0d6545e..0b0454e959414a 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -5748,8 +5748,7 @@ void Compiler::impCheckForPInvokeCall( // return here without inlining the native call. if (unmanagedCallConv == CorInfoCallConvExtension::Managed || unmanagedCallConv == CorInfoCallConvExtension::Fastcall || - unmanagedCallConv == CorInfoCallConvExtension::FastcallMemberFunction || - unmanagedCallConv == CorInfoCallConvExtension::Swift) + unmanagedCallConv == CorInfoCallConvExtension::FastcallMemberFunction) { return; } diff --git a/src/coreclr/jit/targetamd64.h b/src/coreclr/jit/targetamd64.h index ddd46664262d14..147355c2474a24 100644 --- a/src/coreclr/jit/targetamd64.h +++ b/src/coreclr/jit/targetamd64.h @@ -563,9 +563,9 @@ #define RBM_STACK_PROBE_HELPER_TRASH RBM_RAX #endif // !UNIX_AMD64_ABI - // #define SWIFT_SUPPORT - // #define REG_SWIFT_ERROR REG_R12 - // #define RBM_SWIFT_ERROR RBM_R12 - // #define SWIFT_SELF_REG REG_R13 + #define SWIFT_SUPPORT + #define REG_SWIFT_ERROR REG_R12 + #define RBM_SWIFT_ERROR RBM_R12 + #define SWIFT_SELF_REG REG_R13 // clang-format on diff --git a/src/coreclr/jit/targetarm64.h b/src/coreclr/jit/targetarm64.h index 32e45928fd4596..74a14535f15075 100644 --- a/src/coreclr/jit/targetarm64.h +++ b/src/coreclr/jit/targetarm64.h @@ -370,9 +370,9 @@ #define REG_ZERO_INIT_FRAME_REG2 REG_R10 #define REG_ZERO_INIT_FRAME_SIMD REG_V16 - // #define SWIFT_SUPPORT - // #define REG_SWIFT_ERROR REG_R21 - // #define RBM_SWIFT_ERROR RBM_R21 - // #define SWIFT_SELF_REG REG_R20 + #define SWIFT_SUPPORT + #define REG_SWIFT_ERROR REG_R21 + #define RBM_SWIFT_ERROR RBM_R21 + #define SWIFT_SELF_REG REG_R20 // clang-format on From ad8aa75e8276ea00eac95491f6af84702a03d07a Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Tue, 20 Feb 2024 11:54:31 -0500 Subject: [PATCH 24/37] Fix assert --- src/coreclr/jit/gentree.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index aa695c466a0654..787b38732c02a0 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -7365,6 +7365,7 @@ bool GenTree::OperRequiresGlobRefFlag(Compiler* comp) const case GT_CMPXCHG: case GT_MEMORYBARRIER: case GT_KEEPALIVE: + case GT_SWIFT_ERROR: return true; case GT_CALL: From 0edff77c921fb6ab0984bee5ba215df22b910f0d Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Tue, 20 Feb 2024 11:57:49 -0500 Subject: [PATCH 25/37] Allow Swift call conv in dllimport.cpp --- src/coreclr/vm/dllimport.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index d395b8e32cf5c4..542ec14ad198d2 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -4263,8 +4263,7 @@ static void CreateNDirectStubAccessMetadata( { if (unmgdCallConv == CorInfoCallConvExtension::Managed || unmgdCallConv == CorInfoCallConvExtension::Fastcall || - unmgdCallConv == CorInfoCallConvExtension::FastcallMemberFunction || - unmgdCallConv == CorInfoCallConvExtension::Swift) + unmgdCallConv == CorInfoCallConvExtension::FastcallMemberFunction) { COMPlusThrow(kTypeLoadException, IDS_INVALID_PINVOKE_CALLCONV); } From 5ec97c29a15a40b6f5d19222e9057ef4910aaf54 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Tue, 20 Feb 2024 13:33:29 -0500 Subject: [PATCH 26/37] Improve codegen for storing error reg --- src/coreclr/jit/importercalls.cpp | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 0b0454e959414a..71031fbdcc0f07 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -1926,8 +1926,7 @@ void Compiler::impPopArgsForUnmanagedCall(GenTreeCall* call, { for (unsigned level = 0; level < verCurrentState.esStackDepth; level++) { - impSpillStackEntry(level, BAD_VAR_NUM DEBUGARG(false) - DEBUGARG("impPopArgsForUnmanagedCall - checkEntireStack=true")); + impSpillSideEffects(true, CHECK_SPILL_ALL DEBUGARG("Spill for swift calls")); } } } @@ -2035,16 +2034,12 @@ void Compiler::impAppendSwiftErrorStore(GenTreeCall* call, CallArg* const swiftE GenTree* const argNode = swiftErrorArg->GetNode(); assert(argNode != nullptr); - // SwiftError* arg should have been spilled to a local temp variable - assert(argNode->OperIs(GT_LCL_VAR)); - // Store the error register value to where the SwiftError* points to GenTree* errorRegNode = new (this, GT_SWIFT_ERROR) GenTree(GT_SWIFT_ERROR, TYP_I_IMPL); errorRegNode->SetHasOrderingSideEffect(); errorRegNode->gtFlags |= (GTF_CALL | GTF_GLOB_REF); - GenTree* argNodeCopy = gtNewLclvNode(argNode->AsLclVar()->GetLclNum(), argNode->TypeGet()); - GenTreeStoreInd* swiftErrorStore = gtNewStoreIndNode(argNodeCopy->TypeGet(), argNodeCopy, errorRegNode); + GenTreeStoreInd* swiftErrorStore = gtNewStoreIndNode(argNode->TypeGet(), argNode, errorRegNode); impAppendTree(swiftErrorStore, CHECK_SPILL_ALL, impCurStmtDI, false); // Indicate the error register will be checked after this call returns From cc6e2dc718b199d8d0e31d64c607e57f2c75aa36 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Tue, 20 Feb 2024 13:35:38 -0500 Subject: [PATCH 27/37] Remove loop --- src/coreclr/jit/importercalls.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 71031fbdcc0f07..171fa2504b46a7 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -1924,10 +1924,7 @@ void Compiler::impPopArgsForUnmanagedCall(GenTreeCall* call, // If using one of the Swift register types, check entire stack for side effects if (checkEntireStack) { - for (unsigned level = 0; level < verCurrentState.esStackDepth; level++) - { - impSpillSideEffects(true, CHECK_SPILL_ALL DEBUGARG("Spill for swift calls")); - } + impSpillSideEffects(true, CHECK_SPILL_ALL DEBUGARG("Spill for swift calls")); } } #endif // SWIFT_SUPPORT From 5a0a60f04cdc40eb7b04eb532b903e2896787b8e Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Wed, 21 Feb 2024 09:01:33 -0500 Subject: [PATCH 28/37] ifdef lsra changes --- src/coreclr/jit/lsra.cpp | 4 ++++ src/coreclr/jit/lsra.h | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/src/coreclr/jit/lsra.cpp b/src/coreclr/jit/lsra.cpp index 7b80bcc080c6a9..5b86bda766d15d 100644 --- a/src/coreclr/jit/lsra.cpp +++ b/src/coreclr/jit/lsra.cpp @@ -5122,10 +5122,12 @@ void LinearScan::allocateRegistersMinimal() clearRegBusyUntilKill(regRecord->regNum); INDEBUG(dumpLsraAllocationEvent(LSRA_EVENT_KEPT_ALLOCATION, nullptr, regRecord->regNum)); +#ifdef SWIFT_SUPPORT if (currentRefPosition.busyUntilNextKill) { setRegBusyUntilKill(regRecord->regNum, regRecord->registerType); } +#endif // SWIFT_SUPPORT } continue; } @@ -5836,10 +5838,12 @@ void LinearScan::allocateRegisters() clearRegBusyUntilKill(regRecord->regNum); INDEBUG(dumpLsraAllocationEvent(LSRA_EVENT_KEPT_ALLOCATION, nullptr, regRecord->regNum)); +#ifdef SWIFT_SUPPORT if (currentRefPosition.busyUntilNextKill) { setRegBusyUntilKill(regRecord->regNum, regRecord->registerType); } +#endif // SWIFT_SUPPORT } continue; } diff --git a/src/coreclr/jit/lsra.h b/src/coreclr/jit/lsra.h index 2aedc80d054922..47ddaed404f1e4 100644 --- a/src/coreclr/jit/lsra.h +++ b/src/coreclr/jit/lsra.h @@ -2519,8 +2519,10 @@ class RefPosition unsigned char skipSaveRestore : 1; #endif +#ifdef SWIFT_SUPPORT // For a phys reg, indicates that it should be marked as busy until the next time it is killed. unsigned char busyUntilNextKill : 1; +#endif // SWIFT_SUPPORT #ifdef DEBUG // Minimum number registers that needs to be ensured while @@ -2565,7 +2567,9 @@ class RefPosition , isLocalDefUse(false) , delayRegFree(false) , outOfOrder(false) +#ifdef SWIFT_SUPPORT , busyUntilNextKill(false) +#endif // SWIFT_SUPPORT #ifdef DEBUG , minRegCandidateCount(1) , rpNum(0) From f38e916baba367f72bd1a0408b3f47db632a5906 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Wed, 21 Feb 2024 12:29:00 -0500 Subject: [PATCH 29/37] Move GT_SWIFT_ERROR to right after GT_CALL node during lowering --- src/coreclr/jit/lower.cpp | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 591db3a78a22c0..c0feb187bd409c 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -5588,6 +5588,26 @@ void Lowering::InsertPInvokeCallEpilog(GenTreeCall* call) { JITDUMP("======= Inserting PInvoke call epilog\n"); + GenTree* insertionPoint = call; + +#ifdef SWIFT_SUPPORT + // For Swift calls that require error handling, ensure the GT_SWIFT_ERROR node + // that consumes the error register is the call node's successor. + // This is to simplify logic for marking the error register as busy in LSRA. + if ((call->gtCallMoreFlags & GTF_CALL_M_SWIFT_ERROR_HANDLING) != 0) + { + do + { + insertionPoint = insertionPoint->gtNext; + assert(insertionPoint != nullptr); + } + while (!insertionPoint->OperIs(GT_SWIFT_ERROR)); + + BlockRange().Remove(insertionPoint); + BlockRange().InsertAfter(call, LIR::SeqTree(comp, insertionPoint)); + } +#endif // SWIFT_SUPPORT + if (comp->opts.ShouldUsePInvokeHelpers()) { noway_assert(comp->lvaInlinedPInvokeFrameVar != BAD_VAR_NUM); @@ -5604,13 +5624,13 @@ void Lowering::InsertPInvokeCallEpilog(GenTreeCall* call) GenTreeCall* helperCall = comp->gtNewHelperCallNode(CORINFO_HELP_JIT_PINVOKE_END, TYP_VOID, frameAddr); comp->fgMorphTree(helperCall); - BlockRange().InsertAfter(call, LIR::SeqTree(comp, helperCall)); + BlockRange().InsertAfter(insertionPoint, LIR::SeqTree(comp, helperCall)); ContainCheckCallOperands(helperCall); return; } // gcstate = 1 - GenTree* insertionPoint = call->gtNext; + insertionPoint = insertionPoint->gtNext; GenTree* tree = SetGCState(1); BlockRange().InsertBefore(insertionPoint, LIR::SeqTree(comp, tree)); From b08a4e004586b330dc136afc5b2261d37b154462 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Wed, 21 Feb 2024 14:00:01 -0500 Subject: [PATCH 30/37] Use delayRegFree to mark error register as busy --- src/coreclr/jit/lsra.cpp | 28 ++++++++++++++-------------- src/coreclr/jit/lsra.h | 8 -------- src/coreclr/jit/lsraarm64.cpp | 5 ----- src/coreclr/jit/lsrabuild.cpp | 22 ++++++++++++---------- src/coreclr/jit/lsraxarch.cpp | 5 ----- 5 files changed, 26 insertions(+), 42 deletions(-) diff --git a/src/coreclr/jit/lsra.cpp b/src/coreclr/jit/lsra.cpp index 5b86bda766d15d..3dcc755d1f8cff 100644 --- a/src/coreclr/jit/lsra.cpp +++ b/src/coreclr/jit/lsra.cpp @@ -5109,6 +5109,13 @@ void LinearScan::allocateRegistersMinimal() } regsInUseThisLocation |= currentRefPosition.registerAssignment; INDEBUG(dumpLsraAllocationEvent(LSRA_EVENT_FIXED_REG, nullptr, currentRefPosition.assignedReg())); + +#ifdef SWIFT_SUPPORT + if (currentRefPosition.delayRegFree) + { + regsInUseNextLocation |= currentRefPosition.registerAssignment; + } +#endif // SWIFT_SUPPORT } else { @@ -5121,13 +5128,6 @@ void LinearScan::allocateRegistersMinimal() } clearRegBusyUntilKill(regRecord->regNum); INDEBUG(dumpLsraAllocationEvent(LSRA_EVENT_KEPT_ALLOCATION, nullptr, regRecord->regNum)); - -#ifdef SWIFT_SUPPORT - if (currentRefPosition.busyUntilNextKill) - { - setRegBusyUntilKill(regRecord->regNum, regRecord->registerType); - } -#endif // SWIFT_SUPPORT } continue; } @@ -5825,6 +5825,13 @@ void LinearScan::allocateRegisters() } regsInUseThisLocation |= currentRefPosition.registerAssignment; INDEBUG(dumpLsraAllocationEvent(LSRA_EVENT_FIXED_REG, nullptr, currentRefPosition.assignedReg())); + +#ifdef SWIFT_SUPPORT + if (currentRefPosition.delayRegFree) + { + regsInUseNextLocation |= currentRefPosition.registerAssignment; + } +#endif // SWIFT_SUPPORT } else { @@ -5837,13 +5844,6 @@ void LinearScan::allocateRegisters() } clearRegBusyUntilKill(regRecord->regNum); INDEBUG(dumpLsraAllocationEvent(LSRA_EVENT_KEPT_ALLOCATION, nullptr, regRecord->regNum)); - -#ifdef SWIFT_SUPPORT - if (currentRefPosition.busyUntilNextKill) - { - setRegBusyUntilKill(regRecord->regNum, regRecord->registerType); - } -#endif // SWIFT_SUPPORT } continue; } diff --git a/src/coreclr/jit/lsra.h b/src/coreclr/jit/lsra.h index 47ddaed404f1e4..c0e0f5d2fdbd34 100644 --- a/src/coreclr/jit/lsra.h +++ b/src/coreclr/jit/lsra.h @@ -2519,11 +2519,6 @@ class RefPosition unsigned char skipSaveRestore : 1; #endif -#ifdef SWIFT_SUPPORT - // For a phys reg, indicates that it should be marked as busy until the next time it is killed. - unsigned char busyUntilNextKill : 1; -#endif // SWIFT_SUPPORT - #ifdef DEBUG // Minimum number registers that needs to be ensured while // constraining candidates for this ref position under @@ -2567,9 +2562,6 @@ class RefPosition , isLocalDefUse(false) , delayRegFree(false) , outOfOrder(false) -#ifdef SWIFT_SUPPORT - , busyUntilNextKill(false) -#endif // SWIFT_SUPPORT #ifdef DEBUG , minRegCandidateCount(1) , rpNum(0) diff --git a/src/coreclr/jit/lsraarm64.cpp b/src/coreclr/jit/lsraarm64.cpp index 7ad64b2652b7e4..066b9d4e2b7bb3 100644 --- a/src/coreclr/jit/lsraarm64.cpp +++ b/src/coreclr/jit/lsraarm64.cpp @@ -1287,16 +1287,11 @@ int LinearScan::BuildNode(GenTree* tree) srcCount = 0; assert(dstCount == 1); - // After a Swift call potentially trashes the error register, - // the register can be used again only if there is a GT_SWIFT_ERROR node to consume it - // (i.e. the register's value is saved to a SwiftError) - // Any register should do here, but the error register value should immediately // be moved from GT_SWIFT_ERROR's destination register to the SwiftError struct, // and we know REG_SWIFT_ERROR should be busy up to this point, anyway. // By forcing LSRA to use REG_SWIFT_ERROR as both the source and destination register, // we can ensure the redundant move is elided. - addRefsForPhysRegMask(RBM_SWIFT_ERROR, currentLoc, RefTypeKill, true); BuildDef(tree, RBM_SWIFT_ERROR); break; #endif // SWIFT_SUPPORT diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index bfa78236f9a99f..af615e110bb67d 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -1230,16 +1230,18 @@ bool LinearScan::buildKillPositionsForNode(GenTree* tree, LsraLocation currentLo // After a Swift call that might throw returns, we expect the error register to be consumed // by a GT_SWIFT_ERROR node. However, we want to ensure the error register won't be trashed - // before GT_SWIFT_ERROR can consume it - // (for example, the PInvoke epilog comes before the error register store). - // To do so, mark the register as "busy until next kill". - // This means the register will remain busy from the Swift call until the GT_SWIFT_ERROR kills it. - - RegRecord* swiftErrorRegRec = getRegisterRecord(REG_SWIFT_ERROR); - RefPosition* pos = swiftErrorRegRec->lastRefPosition; - assert(pos != nullptr); - assert(pos->refType = RefTypeKill); - pos->busyUntilNextKill = true; + // before GT_SWIFT_ERROR can consume it. + // (For example, the PInvoke epilog comes before the error register store.) + // To do so, delay the freeing of the error register until the next node. + // This only works if the next node after the call is the GT_SWIFT_ERROR node. + // (InsertPInvokeCallEpilog should have moved the GT_SWIFT_ERROR node during lowering.) + assert(tree->gtNext != nullptr); + assert(tree->gtNext->OperIs(GT_SWIFT_ERROR)); + + // We could use RefTypeKill, but RefTypeFixedReg is used less commonly, so the check for delayRegFree + // during register allocation should be cheaper in terms of TP. + RefPosition* pos = newRefPosition(REG_SWIFT_ERROR, currentLoc, RefTypeFixedReg, tree, RBM_SWIFT_ERROR); + pos->delayRegFree = true; } #endif // SWIFT_SUPPORT diff --git a/src/coreclr/jit/lsraxarch.cpp b/src/coreclr/jit/lsraxarch.cpp index f20a5617a9a06b..39299a575ac721 100644 --- a/src/coreclr/jit/lsraxarch.cpp +++ b/src/coreclr/jit/lsraxarch.cpp @@ -638,16 +638,11 @@ int LinearScan::BuildNode(GenTree* tree) srcCount = 0; assert(dstCount == 1); - // After a Swift call potentially trashes the error register, - // the register can be used again only if there is a GT_SWIFT_ERROR node to consume it - // (i.e. the register's value is saved to a SwiftError) - // Any register should do here, but the error register value should immediately // be moved from GT_SWIFT_ERROR's destination register to the SwiftError struct, // and we know REG_SWIFT_ERROR should be busy up to this point, anyway. // By forcing LSRA to use REG_SWIFT_ERROR as both the source and destination register, // we can ensure the redundant move is elided. - addRefsForPhysRegMask(RBM_SWIFT_ERROR, currentLoc, RefTypeKill, true); BuildDef(tree, RBM_SWIFT_ERROR); break; #endif // SWIFT_SUPPORT From 276b9784633fcb8b4735be56dd21685ece3498cb Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Wed, 21 Feb 2024 14:00:08 -0500 Subject: [PATCH 31/37] Style --- src/coreclr/jit/lower.cpp | 3 +-- src/coreclr/jit/lsrabuild.cpp | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index c0feb187bd409c..99b30172c4060e 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -5600,8 +5600,7 @@ void Lowering::InsertPInvokeCallEpilog(GenTreeCall* call) { insertionPoint = insertionPoint->gtNext; assert(insertionPoint != nullptr); - } - while (!insertionPoint->OperIs(GT_SWIFT_ERROR)); + } while (!insertionPoint->OperIs(GT_SWIFT_ERROR)); BlockRange().Remove(insertionPoint); BlockRange().InsertAfter(call, LIR::SeqTree(comp, insertionPoint)); diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index af615e110bb67d..d256ba129cee2b 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -1240,7 +1240,7 @@ bool LinearScan::buildKillPositionsForNode(GenTree* tree, LsraLocation currentLo // We could use RefTypeKill, but RefTypeFixedReg is used less commonly, so the check for delayRegFree // during register allocation should be cheaper in terms of TP. - RefPosition* pos = newRefPosition(REG_SWIFT_ERROR, currentLoc, RefTypeFixedReg, tree, RBM_SWIFT_ERROR); + RefPosition* pos = newRefPosition(REG_SWIFT_ERROR, currentLoc, RefTypeFixedReg, tree, RBM_SWIFT_ERROR); pos->delayRegFree = true; } #endif // SWIFT_SUPPORT From b619bae9aeb9140f6e2350046d29c57b3f292ccf Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Thu, 22 Feb 2024 00:02:05 -0500 Subject: [PATCH 32/37] Move GT_SWIFT_ERROR move logic to LowerNonvirtPinvokeCall --- src/coreclr/jit/lower.cpp | 43 ++++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 99b30172c4060e..428c1ab78805cb 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -5588,25 +5588,6 @@ void Lowering::InsertPInvokeCallEpilog(GenTreeCall* call) { JITDUMP("======= Inserting PInvoke call epilog\n"); - GenTree* insertionPoint = call; - -#ifdef SWIFT_SUPPORT - // For Swift calls that require error handling, ensure the GT_SWIFT_ERROR node - // that consumes the error register is the call node's successor. - // This is to simplify logic for marking the error register as busy in LSRA. - if ((call->gtCallMoreFlags & GTF_CALL_M_SWIFT_ERROR_HANDLING) != 0) - { - do - { - insertionPoint = insertionPoint->gtNext; - assert(insertionPoint != nullptr); - } while (!insertionPoint->OperIs(GT_SWIFT_ERROR)); - - BlockRange().Remove(insertionPoint); - BlockRange().InsertAfter(call, LIR::SeqTree(comp, insertionPoint)); - } -#endif // SWIFT_SUPPORT - if (comp->opts.ShouldUsePInvokeHelpers()) { noway_assert(comp->lvaInlinedPInvokeFrameVar != BAD_VAR_NUM); @@ -5623,13 +5604,13 @@ void Lowering::InsertPInvokeCallEpilog(GenTreeCall* call) GenTreeCall* helperCall = comp->gtNewHelperCallNode(CORINFO_HELP_JIT_PINVOKE_END, TYP_VOID, frameAddr); comp->fgMorphTree(helperCall); - BlockRange().InsertAfter(insertionPoint, LIR::SeqTree(comp, helperCall)); + BlockRange().InsertAfter(call, LIR::SeqTree(comp, helperCall)); ContainCheckCallOperands(helperCall); return; } // gcstate = 1 - insertionPoint = insertionPoint->gtNext; + GenTree* insertionPoint = call->gtNext; GenTree* tree = SetGCState(1); BlockRange().InsertBefore(insertionPoint, LIR::SeqTree(comp, tree)); @@ -5812,6 +5793,26 @@ GenTree* Lowering::LowerNonvirtPinvokeCall(GenTreeCall* call) InsertPInvokeCallEpilog(call); } +#ifdef SWIFT_SUPPORT + // For Swift calls that require error handling, ensure the GT_SWIFT_ERROR node + // that consumes the error register is the call node's successor. + // This is to simplify logic for marking the error register as busy in LSRA. + if ((call->gtCallMoreFlags & GTF_CALL_M_SWIFT_ERROR_HANDLING) != 0) + { + GenTree* swiftErrorNode = call->gtNext; + assert(swiftErrorNode != nullptr); + + while (!swiftErrorNode->OperIs(GT_SWIFT_ERROR)) + { + swiftErrorNode = swiftErrorNode->gtNext; + assert(swiftErrorNode != nullptr); + } + + BlockRange().Remove(swiftErrorNode); + BlockRange().InsertAfter(call, swiftErrorNode); + } +#endif // SWIFT_SUPPORT + return result; } From b7b8e578a1595c63607b5ca304ecba11d893cdc1 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Thu, 22 Feb 2024 00:09:27 -0500 Subject: [PATCH 33/37] Move delayRegFree logic to LinearScan::BuildCall --- src/coreclr/jit/lsraarmarch.cpp | 24 ++++++++++++++++++++++++ src/coreclr/jit/lsrabuild.cpp | 24 ------------------------ src/coreclr/jit/lsraxarch.cpp | 24 ++++++++++++++++++++++++ 3 files changed, 48 insertions(+), 24 deletions(-) diff --git a/src/coreclr/jit/lsraarmarch.cpp b/src/coreclr/jit/lsraarmarch.cpp index a0f27ba65b3058..203ce5a9b5e5f6 100644 --- a/src/coreclr/jit/lsraarmarch.cpp +++ b/src/coreclr/jit/lsraarmarch.cpp @@ -393,6 +393,30 @@ int LinearScan::BuildCall(GenTreeCall* call) regMaskTP killMask = getKillSetForCall(call); BuildDefsWithKills(call, dstCount, dstCandidates, killMask); +#ifdef SWIFT_SUPPORT + if ((call->gtCallMoreFlags & GTF_CALL_M_SWIFT_ERROR_HANDLING) != 0) + { + // Tree is a Swift call with error handling; error register should have been killed + assert(call->unmgdCallConv == CorInfoCallConvExtension::Swift); + assert((killMask & RBM_SWIFT_ERROR) != 0); + + // After a Swift call that might throw returns, we expect the error register to be consumed + // by a GT_SWIFT_ERROR node. However, we want to ensure the error register won't be trashed + // before GT_SWIFT_ERROR can consume it. + // (For example, the PInvoke epilog comes before the error register store.) + // To do so, delay the freeing of the error register until the next node. + // This only works if the next node after the call is the GT_SWIFT_ERROR node. + // (InsertPInvokeCallEpilog should have moved the GT_SWIFT_ERROR node during lowering.) + assert(call->gtNext != nullptr); + assert(call->gtNext->OperIs(GT_SWIFT_ERROR)); + + // We could use RefTypeKill, but RefTypeFixedReg is used less commonly, so the check for delayRegFree + // during register allocation should be cheaper in terms of TP. + RefPosition* pos = newRefPosition(REG_SWIFT_ERROR, currentLoc, RefTypeFixedReg, call, RBM_SWIFT_ERROR); + pos->delayRegFree = true; + } +#endif // SWIFT_SUPPORT + // No args are placed in registers anymore. placedArgRegs = RBM_NONE; numPlacedArgLocals = 0; diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index d256ba129cee2b..83aa30020e9ea0 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -1221,30 +1221,6 @@ bool LinearScan::buildKillPositionsForNode(GenTree* tree, LsraLocation currentLo insertedKills = true; } -#ifdef SWIFT_SUPPORT - if (tree->IsCall() && ((tree->AsCall()->gtCallMoreFlags & GTF_CALL_M_SWIFT_ERROR_HANDLING) != 0)) - { - // Tree is a Swift call with error handling; error register should have been killed - assert(tree->AsCall()->unmgdCallConv == CorInfoCallConvExtension::Swift); - assert((killMask & RBM_SWIFT_ERROR) != 0); - - // After a Swift call that might throw returns, we expect the error register to be consumed - // by a GT_SWIFT_ERROR node. However, we want to ensure the error register won't be trashed - // before GT_SWIFT_ERROR can consume it. - // (For example, the PInvoke epilog comes before the error register store.) - // To do so, delay the freeing of the error register until the next node. - // This only works if the next node after the call is the GT_SWIFT_ERROR node. - // (InsertPInvokeCallEpilog should have moved the GT_SWIFT_ERROR node during lowering.) - assert(tree->gtNext != nullptr); - assert(tree->gtNext->OperIs(GT_SWIFT_ERROR)); - - // We could use RefTypeKill, but RefTypeFixedReg is used less commonly, so the check for delayRegFree - // during register allocation should be cheaper in terms of TP. - RefPosition* pos = newRefPosition(REG_SWIFT_ERROR, currentLoc, RefTypeFixedReg, tree, RBM_SWIFT_ERROR); - pos->delayRegFree = true; - } -#endif // SWIFT_SUPPORT - return insertedKills; } diff --git a/src/coreclr/jit/lsraxarch.cpp b/src/coreclr/jit/lsraxarch.cpp index 39299a575ac721..d48170612fd93c 100644 --- a/src/coreclr/jit/lsraxarch.cpp +++ b/src/coreclr/jit/lsraxarch.cpp @@ -1371,6 +1371,30 @@ int LinearScan::BuildCall(GenTreeCall* call) regMaskTP killMask = getKillSetForCall(call); BuildDefsWithKills(call, dstCount, dstCandidates, killMask); +#ifdef SWIFT_SUPPORT + if ((call->gtCallMoreFlags & GTF_CALL_M_SWIFT_ERROR_HANDLING) != 0) + { + // Tree is a Swift call with error handling; error register should have been killed + assert(call->unmgdCallConv == CorInfoCallConvExtension::Swift); + assert((killMask & RBM_SWIFT_ERROR) != 0); + + // After a Swift call that might throw returns, we expect the error register to be consumed + // by a GT_SWIFT_ERROR node. However, we want to ensure the error register won't be trashed + // before GT_SWIFT_ERROR can consume it. + // (For example, the PInvoke epilog comes before the error register store.) + // To do so, delay the freeing of the error register until the next node. + // This only works if the next node after the call is the GT_SWIFT_ERROR node. + // (InsertPInvokeCallEpilog should have moved the GT_SWIFT_ERROR node during lowering.) + assert(call->gtNext != nullptr); + assert(call->gtNext->OperIs(GT_SWIFT_ERROR)); + + // We could use RefTypeKill, but RefTypeFixedReg is used less commonly, so the check for delayRegFree + // during register allocation should be cheaper in terms of TP. + RefPosition* pos = newRefPosition(REG_SWIFT_ERROR, currentLoc, RefTypeFixedReg, call, RBM_SWIFT_ERROR); + pos->delayRegFree = true; + } +#endif // SWIFT_SUPPORT + // No args are placed in registers anymore. placedArgRegs = RBM_NONE; numPlacedArgLocals = 0; From 991ddce2fafec320e6c41bb0ad9bc2816b62d396 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Thu, 22 Feb 2024 10:37:05 -0500 Subject: [PATCH 34/37] Use setDelayFree() --- src/coreclr/jit/lsraarmarch.cpp | 4 ++-- src/coreclr/jit/lsraxarch.cpp | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/lsraarmarch.cpp b/src/coreclr/jit/lsraarmarch.cpp index 203ce5a9b5e5f6..af15157f49c912 100644 --- a/src/coreclr/jit/lsraarmarch.cpp +++ b/src/coreclr/jit/lsraarmarch.cpp @@ -412,8 +412,8 @@ int LinearScan::BuildCall(GenTreeCall* call) // We could use RefTypeKill, but RefTypeFixedReg is used less commonly, so the check for delayRegFree // during register allocation should be cheaper in terms of TP. - RefPosition* pos = newRefPosition(REG_SWIFT_ERROR, currentLoc, RefTypeFixedReg, call, RBM_SWIFT_ERROR); - pos->delayRegFree = true; + RefPosition* pos = newRefPosition(REG_SWIFT_ERROR, currentLoc, RefTypeFixedReg, call, RBM_SWIFT_ERROR); + pos->setDelayFree(); } #endif // SWIFT_SUPPORT diff --git a/src/coreclr/jit/lsraxarch.cpp b/src/coreclr/jit/lsraxarch.cpp index d48170612fd93c..4320dade934ae1 100644 --- a/src/coreclr/jit/lsraxarch.cpp +++ b/src/coreclr/jit/lsraxarch.cpp @@ -1390,8 +1390,8 @@ int LinearScan::BuildCall(GenTreeCall* call) // We could use RefTypeKill, but RefTypeFixedReg is used less commonly, so the check for delayRegFree // during register allocation should be cheaper in terms of TP. - RefPosition* pos = newRefPosition(REG_SWIFT_ERROR, currentLoc, RefTypeFixedReg, call, RBM_SWIFT_ERROR); - pos->delayRegFree = true; + RefPosition* pos = newRefPosition(REG_SWIFT_ERROR, currentLoc, RefTypeFixedReg, call, RBM_SWIFT_ERROR); + pos->setDelayFree(); } #endif // SWIFT_SUPPORT From 9048ab2927279bb80a776faf0cdd423222607a04 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Thu, 22 Feb 2024 10:37:38 -0500 Subject: [PATCH 35/37] Fix --- src/coreclr/jit/lsraarmarch.cpp | 2 +- src/coreclr/jit/lsraxarch.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/lsraarmarch.cpp b/src/coreclr/jit/lsraarmarch.cpp index af15157f49c912..b363ced4e5cae7 100644 --- a/src/coreclr/jit/lsraarmarch.cpp +++ b/src/coreclr/jit/lsraarmarch.cpp @@ -413,7 +413,7 @@ int LinearScan::BuildCall(GenTreeCall* call) // We could use RefTypeKill, but RefTypeFixedReg is used less commonly, so the check for delayRegFree // during register allocation should be cheaper in terms of TP. RefPosition* pos = newRefPosition(REG_SWIFT_ERROR, currentLoc, RefTypeFixedReg, call, RBM_SWIFT_ERROR); - pos->setDelayFree(); + setDelayFree(pos); } #endif // SWIFT_SUPPORT diff --git a/src/coreclr/jit/lsraxarch.cpp b/src/coreclr/jit/lsraxarch.cpp index 4320dade934ae1..5690f84f44edc2 100644 --- a/src/coreclr/jit/lsraxarch.cpp +++ b/src/coreclr/jit/lsraxarch.cpp @@ -1391,7 +1391,7 @@ int LinearScan::BuildCall(GenTreeCall* call) // We could use RefTypeKill, but RefTypeFixedReg is used less commonly, so the check for delayRegFree // during register allocation should be cheaper in terms of TP. RefPosition* pos = newRefPosition(REG_SWIFT_ERROR, currentLoc, RefTypeFixedReg, call, RBM_SWIFT_ERROR); - pos->setDelayFree(); + setDelayFree(pos); } #endif // SWIFT_SUPPORT From 764f872fe24e61d8f97324712fc07c6bf46c8092 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Thu, 22 Feb 2024 15:38:14 -0500 Subject: [PATCH 36/37] Fix and enable error handling tests --- .../Interop/Swift/SwiftErrorHandling/SwiftErrorHandling.cs | 5 ++++- .../Swift/SwiftErrorHandling/SwiftErrorHandling.swift | 4 ++++ src/tests/issues.targets | 5 ++++- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/tests/Interop/Swift/SwiftErrorHandling/SwiftErrorHandling.cs b/src/tests/Interop/Swift/SwiftErrorHandling/SwiftErrorHandling.cs index 67a398d357e113..d4b81bafcd4c8a 100644 --- a/src/tests/Interop/Swift/SwiftErrorHandling/SwiftErrorHandling.cs +++ b/src/tests/Interop/Swift/SwiftErrorHandling/SwiftErrorHandling.cs @@ -26,6 +26,9 @@ public class ErrorHandlingTests [DllImport(SwiftLib, EntryPoint = "$s18SwiftErrorHandling05getMyB7Message4from13messageLengthSPys6UInt16VGSgs0B0_p_s5Int32VztF")] public unsafe static extern void* GetErrorMessage(void* handle, out int length); + [DllImport(SwiftLib, EntryPoint = "$s18SwiftErrorHandling16freeStringBuffer6bufferySpys6UInt16VG_tF")] + public unsafe static extern void FreeErrorMessageBuffer(void* stringPtr); + [Fact] public unsafe static void TestSwiftErrorThrown() { @@ -99,7 +102,7 @@ private unsafe static string GetErrorMessageFromSwift(SwiftError error) { void* pointer = GetErrorMessage(error.Value, out int messageLength); string errorMessage = Marshal.PtrToStringUni((IntPtr)pointer, messageLength); - NativeMemory.Free((void*)pointer); + FreeErrorMessageBuffer(pointer); return errorMessage; } } diff --git a/src/tests/Interop/Swift/SwiftErrorHandling/SwiftErrorHandling.swift b/src/tests/Interop/Swift/SwiftErrorHandling/SwiftErrorHandling.swift index 20022c0dba3e28..5058014a42ce3a 100644 --- a/src/tests/Interop/Swift/SwiftErrorHandling/SwiftErrorHandling.swift +++ b/src/tests/Interop/Swift/SwiftErrorHandling/SwiftErrorHandling.swift @@ -33,3 +33,7 @@ public func getMyErrorMessage(from error: Error, messageLength: inout Int32) -> } return nil } + +public func freeStringBuffer(buffer: UnsafeMutablePointer) { + buffer.deallocate() +} diff --git a/src/tests/issues.targets b/src/tests/issues.targets index 2737556054ab7d..a6c4b64be6a4be 100644 --- a/src/tests/issues.targets +++ b/src/tests/issues.targets @@ -75,7 +75,10 @@ https://github.com/dotnet/runtime/issues/88586 - + + https://github.com/dotnet/runtime/issues/93631 + + https://github.com/dotnet/runtime/issues/93631 From 2c0eab87ad0faaf9dd0b0d395e4d5765b2072af2 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Fri, 23 Feb 2024 09:37:11 -0500 Subject: [PATCH 37/37] Enable test for coreclr --- .../Interop/Swift/SwiftErrorHandling/SwiftErrorHandling.csproj | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/tests/Interop/Swift/SwiftErrorHandling/SwiftErrorHandling.csproj b/src/tests/Interop/Swift/SwiftErrorHandling/SwiftErrorHandling.csproj index 49be10b9393911..89eda99352fd20 100644 --- a/src/tests/Interop/Swift/SwiftErrorHandling/SwiftErrorHandling.csproj +++ b/src/tests/Interop/Swift/SwiftErrorHandling/SwiftErrorHandling.csproj @@ -5,8 +5,6 @@ true true - - true