Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JIT: Support storing Swift error register value to SwiftError struct #98586

Merged
merged 39 commits into from
Feb 26, 2024
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
d66b6a8
Add error reg support
amanasifkhalid Feb 16, 2024
0da12b3
Kill error reg if calling a Swift method
amanasifkhalid Feb 16, 2024
56eb2a4
Style
amanasifkhalid Feb 16, 2024
2c533aa
Don't forget arm64 lsra
amanasifkhalid Feb 16, 2024
49bf639
Remove space
amanasifkhalid Feb 16, 2024
0d35921
Re-disable Swift call conv
amanasifkhalid Feb 16, 2024
d1618c3
Fix comment
amanasifkhalid Feb 16, 2024
1966828
Fix another comment
amanasifkhalid Feb 16, 2024
62fb26a
Kill error reg only if Swift method has error handling
amanasifkhalid Feb 16, 2024
197f704
Add GTF_CALL, GTF_GLOB_REF flags to errorRegNode
amanasifkhalid Feb 16, 2024
83b0524
Use TYP_I_IMPL
amanasifkhalid Feb 16, 2024
984edf6
add genCodeForSwiftErrorReg
amanasifkhalid Feb 17, 2024
8152cad
Remove SwiftError* call arg after adding IR for it
amanasifkhalid Feb 17, 2024
dc2268d
Remove sig parsing to impPopArgsForUnmanagedCall
amanasifkhalid Feb 17, 2024
8f92c30
Add impAppendSwiftErrorStore
amanasifkhalid Feb 17, 2024
61851e1
Add comment explaining GT_SWIFT_ERROR dest reg
amanasifkhalid Feb 17, 2024
0a0b556
Style
amanasifkhalid Feb 17, 2024
ced9ed3
pull
amanasifkhalid Feb 17, 2024
f61d406
Disable
amanasifkhalid Feb 17, 2024
33ff3df
Fix comment
amanasifkhalid Feb 19, 2024
8b57428
Throw exception for passing SwiftError struct
amanasifkhalid Feb 19, 2024
7ce0adb
Fix value num
amanasifkhalid Feb 19, 2024
7692346
Mark error reg as busy until GT_SWIFT_ERROR consumes it
amanasifkhalid Feb 20, 2024
24b4bec
enable Swift call conv in JIT
amanasifkhalid Feb 20, 2024
ad8aa75
Fix assert
amanasifkhalid Feb 20, 2024
0edff77
Allow Swift call conv in dllimport.cpp
amanasifkhalid Feb 20, 2024
5ec97c2
Improve codegen for storing error reg
amanasifkhalid Feb 20, 2024
cc6e2dc
Remove loop
amanasifkhalid Feb 20, 2024
5a0a60f
ifdef lsra changes
amanasifkhalid Feb 21, 2024
f38e916
Move GT_SWIFT_ERROR to right after GT_CALL node during lowering
amanasifkhalid Feb 21, 2024
b08a4e0
Use delayRegFree to mark error register as busy
amanasifkhalid Feb 21, 2024
276b978
Style
amanasifkhalid Feb 21, 2024
b619bae
Move GT_SWIFT_ERROR move logic to LowerNonvirtPinvokeCall
amanasifkhalid Feb 22, 2024
b7b8e57
Move delayRegFree logic to LinearScan::BuildCall
amanasifkhalid Feb 22, 2024
991ddce
Use setDelayFree()
amanasifkhalid Feb 22, 2024
9048ab2
Fix
amanasifkhalid Feb 22, 2024
764f872
Fix and enable error handling tests
amanasifkhalid Feb 22, 2024
f125610
Merge branch 'main' into swift-error-reg
amanasifkhalid Feb 23, 2024
2c0eab8
Enable test for coreclr
amanasifkhalid Feb 23, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/coreclr/jit/codegen.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
17 changes: 17 additions & 0 deletions src/coreclr/jit/codegenarmarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,12 @@ void CodeGen::genCodeForTreeNode(GenTree* treeNode)
break;
#endif // TARGET_ARM64

#ifdef SWIFT_SUPPORT
case GT_SWIFT_ERROR:
genCodeForSwiftErrorReg(treeNode);
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
Expand Down Expand Up @@ -3425,6 +3431,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, REG_SWIFT_ERROR);
}
#endif // SWIFT_SUPPORT

genCallInstruction(call);

genDefinePendingCallLabel(call);
Expand Down
28 changes: 28 additions & 0 deletions src/coreclr/jit/codegencommon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
17 changes: 17 additions & 0 deletions src/coreclr/jit/codegenxarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2107,6 +2107,12 @@ void CodeGen::genCodeForTreeNode(GenTree* treeNode)
case GT_NOP:
break;

#ifdef SWIFT_SUPPORT
case GT_SWIFT_ERROR:
genCodeForSwiftErrorReg(treeNode);
break;
#endif // SWIFT_SUPPORT

case GT_KEEPALIVE:
genConsumeRegs(treeNode->AsOp()->gtOp1);
break;
Expand Down Expand Up @@ -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, REG_SWIFT_ERROR);
kotlarmilos marked this conversation as resolved.
Show resolved Hide resolved
}
#endif // SWIFT_SUPPORT

genCallInstruction(call X86_ARG(stackArgBytes));

genDefinePendingCallLabel(call);
Expand Down
7 changes: 6 additions & 1 deletion src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -4363,7 +4363,11 @@ 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, /* 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);
Expand Down Expand Up @@ -11315,6 +11319,7 @@ class GenTreeVisitor
case GT_PINVOKE_EPILOG:
case GT_IL_OFFSET:
case GT_NOP:
case GT_SWIFT_ERROR:
break;

// Lclvar unary operators
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/compiler.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 7 additions & 1 deletion src/coreclr/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -7420,6 +7423,7 @@ bool GenTree::OperSupportsOrderingSideEffect() const
case GT_CMPXCHG:
case GT_MEMORYBARRIER:
case GT_CATCH_ARG:
case GT_SWIFT_ERROR:
return true;
default:
return false;
Expand Down Expand Up @@ -8758,7 +8762,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
//
Expand Down Expand Up @@ -10304,6 +10308,7 @@ GenTreeUseEdgeIterator::GenTreeUseEdgeIterator(GenTree* node)
case GT_PINVOKE_EPILOG:
case GT_IL_OFFSET:
case GT_NOP:
case GT_SWIFT_ERROR:
m_state = -1;
return;

Expand Down Expand Up @@ -12410,6 +12415,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:
Expand Down
4 changes: 4 additions & 0 deletions src/coreclr/jit/gentree.h
Original file line number Diff line number Diff line change
Expand Up @@ -4116,6 +4116,10 @@ enum GenTreeCallFlags : unsigned int
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)
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/gtlist.h
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
140 changes: 137 additions & 3 deletions src/coreclr/jit/importercalls.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,11 @@ var_types Compiler::impImportCall(OPCODE opcode,
CORINFO_SIG_INFO calliSig;
NewCallArg extraArg;

// 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;

/*-------------------------------------------------------------------------
* First create the call node
*/
Expand Down Expand Up @@ -651,6 +656,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
Expand All @@ -663,7 +670,7 @@ var_types Compiler::impImportCall(OPCODE opcode,

checkForSmallType = true;

impPopArgsForUnmanagedCall(call->AsCall(), sig);
impPopArgsForUnmanagedCall(call->AsCall(), sig, &swiftErrorArg, &swiftSelfArg);

goto DONE;
}
Expand Down Expand Up @@ -1485,6 +1492,15 @@ var_types Compiler::impImportCall(OPCODE opcode,
impPushOnStack(call, tiRetVal);
}

#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)
{
impAppendSwiftErrorStore(call->AsCall(), swiftErrorArg);
}
#endif // SWIFT_SUPPORT
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps introduce a method for this just to avoid growing impImportCall even more


return callRetTyp;
}
#ifdef _PREFAST_
Expand Down Expand Up @@ -1822,7 +1838,10 @@ 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,
/* OUT */ CallArg** swiftErrorArg,
/* OUT */ CallArg** swiftSelfArg)
{
assert(call->gtFlags & GTF_CALL_UNMANAGED);

Expand All @@ -1842,10 +1861,74 @@ void Compiler::impPopArgsForUnmanagedCall(GenTreeCall* call, CORINFO_SIG_INFO* s

if (call->unmgdCallConv == CorInfoCallConvExtension::Thiscall)
{
assert(argsToReverse);
assert(argsToReverse != 0);
argsToReverse--;
}

#ifdef SWIFT_SUPPORT
unsigned short swiftErrorIndex = sig->numArgs;

// 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
CORINFO_ARG_LIST_HANDLE sigArg = sig->args;

for (unsigned short argIndex = 0; argIndex < sig->numArgs;
kotlarmilos marked this conversation as resolved.
Show resolved Hide resolved
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) &&
kotlarmilos marked this conversation as resolved.
Show resolved Hide resolved
(strcmp(namespaceName, "System.Runtime.InteropServices.Swift") == 0))
{
// For error handling purposes, we expect a pointer to a SwiftError to be passed
assert(argIsByrefOrPtr);
kotlarmilos marked this conversation as resolved.
Show resolved Hide resolved
if (swiftErrorIndex != sig->numArgs)
{
BADCODE("Duplicate SwiftError* parameter");
kotlarmilos marked this conversation as resolved.
Show resolved Hide resolved
}

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)
kotlarmilos marked this conversation as resolved.
Show resolved Hide resolved
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;
Expand Down Expand Up @@ -1892,6 +1975,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();
Expand All @@ -1914,9 +1998,59 @@ 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++;
}
}

#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.
Expand Down
Loading