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.

Sign up for GitHub

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: Add GT_SWIFT_ERROR_RET to represent loading error register upon return #100692

Merged
merged 29 commits into from
Apr 12, 2024
Merged

JIT: Add GT_SWIFT_ERROR_RET to represent loading error register upon return #100692

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
a9646c1
Add GT_SWIFT_ERROR_RET
amanasifkhalid Apr 5, 2024
b328337
Cleanup
amanasifkhalid Apr 5, 2024
63eb15c
Style
amanasifkhalid Apr 5, 2024
674d8f1
Style; comments
amanasifkhalid Apr 5, 2024
7d2e8fa
Binop implementation
amanasifkhalid Apr 8, 2024
a342e00
Update comment
amanasifkhalid Apr 8, 2024
6e50ea7
Merge from main
amanasifkhalid Apr 8, 2024
aa8c83c
Comments
amanasifkhalid Apr 8, 2024
9e310ec
Style
amanasifkhalid Apr 8, 2024
15d39bf
Add GetReturnValue helper
amanasifkhalid Apr 8, 2024
873e15a
Handle GT_SWIFT_ERROR_RET in LSRA
amanasifkhalid Apr 8, 2024
5d96892
Overzealous opt
amanasifkhalid Apr 9, 2024
24e8d0d
Feedback
amanasifkhalid Apr 9, 2024
bf186b7
Store error to local during return merging
amanasifkhalid Apr 10, 2024
c381c48
Handle GT_SWIFT_ERROR_RET in morph
amanasifkhalid Apr 10, 2024
70a69d7
Move GT_SWIFT_ERROR_RET creation to phase
amanasifkhalid Apr 10, 2024
de4457b
Handle remaining GT_RETURN-specific code
amanasifkhalid Apr 10, 2024
7096e23
Style
amanasifkhalid Apr 10, 2024
927a55e
Add comment
amanasifkhalid Apr 10, 2024
848704b
Remove ifdef
amanasifkhalid Apr 10, 2024
502f471
Fix store ordering
amanasifkhalid Apr 10, 2024
e8b1dbc
Style
amanasifkhalid Apr 10, 2024
2dce428
Do SwiftError local conversion in phase
amanasifkhalid Apr 10, 2024
38ad8a7
Delete assert
amanasifkhalid Apr 10, 2024
18df9c7
Fix build
amanasifkhalid Apr 10, 2024
86a0825
Fix codegen on arm64
amanasifkhalid Apr 11, 2024
41ec8f7
Feedback
amanasifkhalid Apr 11, 2024
6d9d360
Update BADCODE message
amanasifkhalid Apr 11, 2024
0cd339d
Feedback
amanasifkhalid Apr 12, 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
7 changes: 6 additions & 1 deletion src/coreclr/jit/flowgraph.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2596,8 +2596,13 @@ PhaseStatus Compiler::fgAddSwiftErrorReturns()

fgWalkResult PreOrderVisit(GenTree** use, GenTree* user)
{
if ((*use)->OperIs(GT_LCL_VAR) && ((*use)->AsLclVarCommon()->GetLclNum() == m_compiler->lvaSwiftErrorArg))
if ((*use)->AsLclVarCommon()->GetLclNum() == m_compiler->lvaSwiftErrorArg)
{
if (!(*use)->OperIs(GT_LCL_VAR))
{
BADCODE("Expected GT_LCL_VAR of SwiftError* parameter, got other GenTreeLclVarCommon node");
Copy link
Member

Choose a reason for hiding this comment

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

I think the message should be phrased in terms of IL, not IR constructs

Copy link
Member Author

Choose a reason for hiding this comment

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

Would something like "Found invalid use of SwiftError* parameter" be too vague?

Copy link
Member

Choose a reason for hiding this comment

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

That sounds good to me

}

*use = m_compiler->gtNewLclVarAddrNode(m_compiler->lvaSwiftErrorLocal, genActualType(*use));
}

Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/jit/ifconversion.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,9 +295,9 @@ bool OptIfConversionDsc::IfConvertCheckStmts(BasicBlock* fromBlock, IfConvertOpe
}

case GT_RETURN:
case GT_SWIFT_ERROR_RET:
{
GenTree* const retVal = tree->AsOp()->GetReturnValue();
// GT_SWIFT_ERROR_RET not supported
GenTree* const retVal = tree->gtGetOp1();

// Only allow RETURNs if else conversion is being used.
if (!m_doElseConversion)
Expand Down
7 changes: 3 additions & 4 deletions src/coreclr/jit/morph.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14116,10 +14116,9 @@ void Compiler::fgMergeBlockReturn(BasicBlock* block)
if ((ret != nullptr) && ret->OperIs(GT_SWIFT_ERROR_RET))
{
assert(genReturnErrorLocal != BAD_VAR_NUM);
const DebugInfo& di = lastStmt->GetDebugInfo();
GenTree* swiftErrorStore =
gtNewTempStore(genReturnErrorLocal, ret->gtGetOp1(), CHECK_SPILL_NONE, nullptr, di, block);
Statement* const newStmt = gtNewStmt(swiftErrorStore, di);
const DebugInfo& di = lastStmt->GetDebugInfo();
GenTree* swiftErrorStore = gtNewTempStore(genReturnErrorLocal, ret->gtGetOp1());
Statement* const newStmt = gtNewStmt(swiftErrorStore, di);
fgInsertStmtBefore(block, lastStmt, newStmt);
}
#endif // SWIFT_SUPPORT
Expand Down
16 changes: 13 additions & 3 deletions src/coreclr/jit/valuenum.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11697,15 +11697,25 @@ void Compiler::fgValueNumberTree(GenTree* tree)
}
break;

// GT_SWIFT_ERROR_RET is similar to GT_RETURN, but it's a binary node, and its return value is op2.
case GT_SWIFT_ERROR_RET:
if (tree->gtGetOp2() != nullptr)
{
tree->gtVNPair = vnStore->VNPWithExc(vnStore->VNPForVoid(),
vnStore->VNPExceptionSet(tree->gtGetOp2()->gtVNPair));
// We have a return value and an error value.
ValueNumPair vnp;
ValueNumPair op1Xvnp;
ValueNumPair op2Xvnp;
vnStore->VNPUnpackExc(tree->gtGetOp1()->gtVNPair, &vnp, &op1Xvnp);
vnStore->VNPUnpackExc(tree->gtGetOp2()->gtVNPair, &vnp, &op2Xvnp);

const ValueNumPair excSetPair = vnStore->VNPExcSetUnion(op1Xvnp, op2Xvnp);
tree->gtVNPair = vnStore->VNPWithExc(vnStore->VNPForVoid(), excSetPair);
}
else
{
tree->gtVNPair = vnStore->VNPForVoid();
// We only have the error value.
tree->gtVNPair = vnStore->VNPWithExc(vnStore->VNPForVoid(),
vnStore->VNPExceptionSet(tree->gtGetOp1()->gtVNPair));
}
break;

Expand Down
Loading