From a9646c1d410329e7366fa490c5d670039971715f Mon Sep 17 00:00:00 2001
From: Aman Khalid <amankhalid@microsoft.com>
Date: Fri, 5 Apr 2024 14:59:56 +0000
Subject: [PATCH 01/28] Add GT_SWIFT_ERROR_RET

---
 src/coreclr/jit/codegencommon.cpp | 11 -----------
 src/coreclr/jit/codegenxarch.cpp  | 10 ++++++++++
 src/coreclr/jit/compiler.h        |  1 +
 src/coreclr/jit/compiler.hpp      |  1 +
 src/coreclr/jit/fgstmt.cpp        |  1 +
 src/coreclr/jit/gentree.cpp       |  3 +++
 src/coreclr/jit/gtlist.h          |  3 ++-
 src/coreclr/jit/importer.cpp      | 12 ++++++++++++
 src/coreclr/jit/liveness.cpp      |  1 +
 src/coreclr/jit/lower.cpp         |  9 +++++++++
 10 files changed, 40 insertions(+), 12 deletions(-)

diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp
index 2cee578a47bdc3..d8988a9fef061f 100644
--- a/src/coreclr/jit/codegencommon.cpp
+++ b/src/coreclr/jit/codegencommon.cpp
@@ -7935,17 +7935,6 @@ void CodeGen::genReturn(GenTree* treeNode)
 
     genStackPointerCheck(doStackPointerCheck, compiler->lvaReturnSpCheck);
 #endif // defined(DEBUG) && defined(TARGET_XARCH)
-
-#ifdef SWIFT_SUPPORT
-    // If this method has a SwiftError* out parameter, load the SwiftError pseudolocal value into the error register.
-    // TODO-CQ: Introduce GenTree node that models returning a normal and Swift error value.
-    if (compiler->lvaSwiftErrorArg != BAD_VAR_NUM)
-    {
-        assert(compiler->info.compCallConv == CorInfoCallConvExtension::Swift);
-        assert(compiler->lvaSwiftErrorLocal != BAD_VAR_NUM);
-        GetEmitter()->emitIns_R_S(ins_Load(TYP_I_IMPL), EA_PTRSIZE, REG_SWIFT_ERROR, compiler->lvaSwiftErrorLocal, 0);
-    }
-#endif // SWIFT_SUPPORT
 }
 
 //------------------------------------------------------------------------
diff --git a/src/coreclr/jit/codegenxarch.cpp b/src/coreclr/jit/codegenxarch.cpp
index 6f68063d0b4fa6..77f0e767d6e9dc 100644
--- a/src/coreclr/jit/codegenxarch.cpp
+++ b/src/coreclr/jit/codegenxarch.cpp
@@ -1962,6 +1962,16 @@ void CodeGen::genCodeForTreeNode(GenTree* treeNode)
             genReturn(treeNode);
             break;
 
+#ifdef SWIFT_SUPPORT
+        case GT_SWIFT_ERROR_RET:
+        {
+            GenTree* swiftErrorNode = treeNode->gtGetOp1();
+            const regNumber srcReg = genConsumeReg(swiftErrorNode);
+            inst_Mov(swiftErrorNode->TypeGet(), REG_SWIFT_ERROR, srcReg, true, EA_PTRSIZE);
+            break;
+        }
+#endif // SWIFT_SUPPORT
+
         case GT_LEA:
             // If we are here, it is the case where there is an LEA that cannot be folded into a parent instruction.
             genLeaInstruction(treeNode->AsAddrMode());
diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h
index f27618af986f09..426faa73adace2 100644
--- a/src/coreclr/jit/compiler.h
+++ b/src/coreclr/jit/compiler.h
@@ -11465,6 +11465,7 @@ class GenTreeVisitor
             case GT_FIELD_ADDR:
             case GT_RETURN:
             case GT_RETFILT:
+            case GT_SWIFT_ERROR_RET:
             case GT_RUNTIMELOOKUP:
             case GT_ARR_ADDR:
             case GT_KEEPALIVE:
diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp
index 62efd5282a16a1..2934db7d53d0b8 100644
--- a/src/coreclr/jit/compiler.hpp
+++ b/src/coreclr/jit/compiler.hpp
@@ -4291,6 +4291,7 @@ void GenTree::VisitOperands(TVisitor visitor)
         case GT_RETURNTRAP:
         case GT_KEEPALIVE:
         case GT_INC_SATURATE:
+        case GT_SWIFT_ERROR_RET:
             visitor(this->AsUnOp()->gtOp1);
             return;
 
diff --git a/src/coreclr/jit/fgstmt.cpp b/src/coreclr/jit/fgstmt.cpp
index fead5b82e0b347..cd23733da25ab1 100644
--- a/src/coreclr/jit/fgstmt.cpp
+++ b/src/coreclr/jit/fgstmt.cpp
@@ -538,6 +538,7 @@ inline bool OperIsControlFlow(genTreeOps oper)
 
         case GT_RETURN:
         case GT_RETFILT:
+        case GT_SWIFT_ERROR_RET:
 #if !defined(FEATURE_EH_FUNCLETS)
         case GT_END_LFIN:
 #endif // !FEATURE_EH_FUNCLETS
diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp
index 71d0d0e2dfe904..947035a8998592 100644
--- a/src/coreclr/jit/gentree.cpp
+++ b/src/coreclr/jit/gentree.cpp
@@ -6719,6 +6719,7 @@ bool GenTree::TryGetUse(GenTree* operand, GenTree*** pUse)
         case GT_BSWAP16:
         case GT_KEEPALIVE:
         case GT_INC_SATURATE:
+        case GT_SWIFT_ERROR_RET:
             if (operand == this->AsUnOp()->gtOp1)
             {
                 *pUse = &this->AsUnOp()->gtOp1;
@@ -7370,6 +7371,7 @@ bool GenTree::OperSupportsOrderingSideEffect() const
         case GT_MEMORYBARRIER:
         case GT_CATCH_ARG:
         case GT_SWIFT_ERROR:
+        case GT_SWIFT_ERROR_RET:
             return true;
         default:
             return false;
@@ -10289,6 +10291,7 @@ GenTreeUseEdgeIterator::GenTreeUseEdgeIterator(GenTree* node)
         case GT_PUTARG_SPLIT:
 #endif // FEATURE_ARG_SPLIT
         case GT_RETURNTRAP:
+        case GT_SWIFT_ERROR_RET:
             m_edge = &m_node->AsUnOp()->gtOp1;
             assert(*m_edge != nullptr);
             m_advance = &GenTreeUseEdgeIterator::Terminate;
diff --git a/src/coreclr/jit/gtlist.h b/src/coreclr/jit/gtlist.h
index 817b27a936a561..0e46e7a2671fc8 100644
--- a/src/coreclr/jit/gtlist.h
+++ b/src/coreclr/jit/gtlist.h
@@ -290,7 +290,8 @@ GTNODE(END_LFIN         , GenTreeVal         ,0,0,GTK_LEAF|GTK_NOVALUE) // End l
 //  Swift interop-specific nodes:
 //-----------------------------------------------------------------------------
 
-GTNODE(SWIFT_ERROR      , GenTree            ,0,0,GTK_LEAF)             // Error register value post-Swift call
+GTNODE(SWIFT_ERROR      , GenTree            ,0,0,GTK_LEAF)              // Error register value post-Swift call
+GTNODE(SWIFT_ERROR_RET  , GenTreeOp          ,0,0,GTK_UNOP|GTK_NOVALUE) // Returns SwiftError pseudolocal's value in REG_SWIFT_ERROR
 
 //-----------------------------------------------------------------------------
 //  Nodes used by Lower to generate a closer CPU representation of other nodes
diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp
index 0d1df79812f03a..9f9c09dd62c239 100644
--- a/src/coreclr/jit/importer.cpp
+++ b/src/coreclr/jit/importer.cpp
@@ -10879,6 +10879,18 @@ bool Compiler::impReturnInstruction(int prefixFlags, OPCODE& opcode)
         }
     }
 
+#ifdef SWIFT_SUPPORT
+    if (lvaSwiftErrorArg != BAD_VAR_NUM)
+    {
+        assert(info.compCallConv == CorInfoCallConvExtension::Swift);
+        assert(lvaSwiftErrorArg != BAD_VAR_NUM);
+        GenTree* const swiftErrorNode = gtNewLclFldNode(lvaSwiftErrorLocal, TYP_I_IMPL, 0);
+        GenTree* const swiftErrorRet  = gtNewOperNode(GT_SWIFT_ERROR_RET, op1->TypeGet(), swiftErrorNode);
+        swiftErrorRet->SetHasOrderingSideEffect();
+        impAppendTree(swiftErrorRet, CHECK_SPILL_NONE, impCurStmtDI);
+    }
+#endif // SWIFT_SUPPORT
+
     impAppendTree(op1, CHECK_SPILL_NONE, impCurStmtDI);
 #ifdef DEBUG
     // Remember at which BC offset the tree was finished
diff --git a/src/coreclr/jit/liveness.cpp b/src/coreclr/jit/liveness.cpp
index 05c65d2de3450c..08dcd500acb022 100644
--- a/src/coreclr/jit/liveness.cpp
+++ b/src/coreclr/jit/liveness.cpp
@@ -1955,6 +1955,7 @@ void Compiler::fgComputeLifeLIR(VARSET_TP& life, BasicBlock* block, VARSET_VALAR
             case GT_PUTARG_STK:
             case GT_IL_OFFSET:
             case GT_KEEPALIVE:
+            case GT_SWIFT_ERROR_RET:
                 // Never remove these nodes, as they are always side-effecting.
                 //
                 // NOTE: the only side-effect of some of these nodes (GT_CMP, GT_SUB_HI) is a write to the flags
diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp
index be0dde669e34be..0fcdc9450338d9 100644
--- a/src/coreclr/jit/lower.cpp
+++ b/src/coreclr/jit/lower.cpp
@@ -524,6 +524,15 @@ GenTree* Lowering::LowerNode(GenTree* node)
             LowerRet(node->AsUnOp());
             break;
 
+        case GT_SWIFT_ERROR_RET:
+        {
+            GenTree* const nextNode = node->gtNext;
+            LIR::Range& blockRange = BlockRange();
+            blockRange.Remove(node);
+            blockRange.InsertAtEnd(node);
+            return nextNode;
+        }
+
         case GT_RETURNTRAP:
             ContainCheckReturnTrap(node->AsOp());
             break;

From b328337986b9337d611fbb80ec6cfc120f147c99 Mon Sep 17 00:00:00 2001
From: Aman Khalid <amankhalid@microsoft.com>
Date: Fri, 5 Apr 2024 16:36:17 +0000
Subject: [PATCH 02/28] Cleanup

---
 src/coreclr/jit/codegen.h          |  4 ++++
 src/coreclr/jit/codegenarmarch.cpp |  6 ++++++
 src/coreclr/jit/codegencommon.cpp  | 19 +++++++++++++++++++
 src/coreclr/jit/codegenxarch.cpp   |  6 +-----
 src/coreclr/jit/importer.cpp       |  1 -
 src/coreclr/jit/lclvars.cpp        |  1 -
 src/coreclr/jit/lower.cpp          | 29 ++++++++++++++++++++++-------
 src/coreclr/jit/lower.h            |  1 +
 8 files changed, 53 insertions(+), 14 deletions(-)

diff --git a/src/coreclr/jit/codegen.h b/src/coreclr/jit/codegen.h
index ae97be76fbe5a2..90224e2fabbcb6 100644
--- a/src/coreclr/jit/codegen.h
+++ b/src/coreclr/jit/codegen.h
@@ -1324,6 +1324,10 @@ class CodeGen final : public CodeGenInterface
 
     void genReturn(GenTree* treeNode);
 
+#ifdef SWIFT_SUPPORT
+    void genSwiftErrorReturn(GenTree* treeNode);
+#endif // SWIFT_SUPPORT
+
 #ifdef TARGET_XARCH
     void           genStackPointerConstantAdjustment(ssize_t spDelta, bool trackSpAdjustments);
     void           genStackPointerConstantAdjustmentWithProbe(ssize_t spDelta, bool trackSpAdjustments);
diff --git a/src/coreclr/jit/codegenarmarch.cpp b/src/coreclr/jit/codegenarmarch.cpp
index a9e2a41f73f945..b53ddba5e192fd 100644
--- a/src/coreclr/jit/codegenarmarch.cpp
+++ b/src/coreclr/jit/codegenarmarch.cpp
@@ -282,6 +282,12 @@ void CodeGen::genCodeForTreeNode(GenTree* treeNode)
             genReturn(treeNode);
             break;
 
+#ifdef SWIFT_SUPPORT
+        case GT_SWIFT_ERROR_RET:
+            genSwiftErrorReturn(treeNode);
+            break;
+#endif // SWIFT_SUPPORT
+
         case GT_LEA:
             // If we are here, it is the case where there is an LEA that cannot be folded into a parent instruction.
             genLeaInstruction(treeNode->AsAddrMode());
diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp
index d8988a9fef061f..e7815f8387a8dd 100644
--- a/src/coreclr/jit/codegencommon.cpp
+++ b/src/coreclr/jit/codegencommon.cpp
@@ -7937,6 +7937,25 @@ void CodeGen::genReturn(GenTree* treeNode)
 #endif // defined(DEBUG) && defined(TARGET_XARCH)
 }
 
+#ifdef SWIFT_SUPPORT
+//------------------------------------------------------------------------
+// genSwiftErrorReturn: Generates code for loading the SwiftError pseudolocal value into the error register.
+//
+// Arguments:
+//    treeNode - The GT_SWIFT_ERROR_RET tree node.
+//
+// Return Value:
+//    None
+//
+void CodeGen::genSwiftErrorReturn(GenTree* treeNode)
+{
+    assert(treeNode->OperIs(GT_SWIFT_ERROR_RET));
+    GenTree* swiftErrorNode = treeNode->gtGetOp1();
+    const regNumber srcReg = genConsumeReg(swiftErrorNode);
+    inst_Mov(swiftErrorNode->TypeGet(), REG_SWIFT_ERROR, srcReg, true, EA_PTRSIZE);
+}
+#endif // SWIFT_SUPPORT
+
 //------------------------------------------------------------------------
 // isStructReturn: Returns whether the 'treeNode' is returning a struct.
 //
diff --git a/src/coreclr/jit/codegenxarch.cpp b/src/coreclr/jit/codegenxarch.cpp
index 77f0e767d6e9dc..be3c4beca93a70 100644
--- a/src/coreclr/jit/codegenxarch.cpp
+++ b/src/coreclr/jit/codegenxarch.cpp
@@ -1964,12 +1964,8 @@ void CodeGen::genCodeForTreeNode(GenTree* treeNode)
 
 #ifdef SWIFT_SUPPORT
         case GT_SWIFT_ERROR_RET:
-        {
-            GenTree* swiftErrorNode = treeNode->gtGetOp1();
-            const regNumber srcReg = genConsumeReg(swiftErrorNode);
-            inst_Mov(swiftErrorNode->TypeGet(), REG_SWIFT_ERROR, srcReg, true, EA_PTRSIZE);
+            genSwiftErrorReturn(treeNode);
             break;
-        }
 #endif // SWIFT_SUPPORT
 
         case GT_LEA:
diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp
index 9f9c09dd62c239..4b3b667436ff7e 100644
--- a/src/coreclr/jit/importer.cpp
+++ b/src/coreclr/jit/importer.cpp
@@ -10886,7 +10886,6 @@ bool Compiler::impReturnInstruction(int prefixFlags, OPCODE& opcode)
         assert(lvaSwiftErrorArg != BAD_VAR_NUM);
         GenTree* const swiftErrorNode = gtNewLclFldNode(lvaSwiftErrorLocal, TYP_I_IMPL, 0);
         GenTree* const swiftErrorRet  = gtNewOperNode(GT_SWIFT_ERROR_RET, op1->TypeGet(), swiftErrorNode);
-        swiftErrorRet->SetHasOrderingSideEffect();
         impAppendTree(swiftErrorRet, CHECK_SPILL_NONE, impCurStmtDI);
     }
 #endif // SWIFT_SUPPORT
diff --git a/src/coreclr/jit/lclvars.cpp b/src/coreclr/jit/lclvars.cpp
index 3e182e0820a1c5..a9777f71d68e12 100644
--- a/src/coreclr/jit/lclvars.cpp
+++ b/src/coreclr/jit/lclvars.cpp
@@ -1469,7 +1469,6 @@ bool Compiler::lvaInitSpecialSwiftParam(CORINFO_ARG_LIST_HANDLE argHnd,
         // Instead, all usages of the SwiftError* parameter will be redirected to this pseudolocal.
         lvaSwiftErrorLocal = lvaGrabTempWithImplicitUse(false DEBUGARG("SwiftError pseudolocal"));
         lvaSetStruct(lvaSwiftErrorLocal, typeHnd, false);
-        lvaSetVarAddrExposed(lvaSwiftErrorLocal DEBUGARG(AddressExposedReason::ESCAPE_ADDRESS));
         return true;
     }
 
diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp
index 0fcdc9450338d9..beaa473ece51bf 100644
--- a/src/coreclr/jit/lower.cpp
+++ b/src/coreclr/jit/lower.cpp
@@ -525,13 +525,7 @@ GenTree* Lowering::LowerNode(GenTree* node)
             break;
 
         case GT_SWIFT_ERROR_RET:
-        {
-            GenTree* const nextNode = node->gtNext;
-            LIR::Range& blockRange = BlockRange();
-            blockRange.Remove(node);
-            blockRange.InsertAtEnd(node);
-            return nextNode;
-        }
+            return LowerSwiftErrorRet(node);
 
         case GT_RETURNTRAP:
             ContainCheckReturnTrap(node->AsOp());
@@ -4634,6 +4628,27 @@ void Lowering::LowerRet(GenTreeUnOp* ret)
     ContainCheckRet(ret);
 }
 
+//----------------------------------------------------------------------------------------------
+// LowerSwiftErrorRet: Lower swift error return node by moving it to the end of its BBJ_RETURN block.
+//                     We wait to do this late in the JIT's phases,
+//                     as we typically expect BBJ_RETURN blocks to end with a GT_RETURN/GT_RETFILT node.
+//
+// Arguments:
+//     swiftErrorRet - The GT_SWIFT_ERROR_RET node.
+//
+// Return Value:
+//     The next node to be lowered.
+//
+GenTree* Lowering::LowerSwiftErrorRet(GenTree* swiftErrorRet)
+{
+    assert(swiftErrorRet->OperIs(GT_SWIFT_ERROR_RET));
+    GenTree* const nextNode = swiftErrorRet->gtNext;
+    LIR::Range& blockRange = BlockRange();
+    blockRange.Remove(swiftErrorRet);
+    blockRange.InsertAtEnd(swiftErrorRet);
+    return nextNode;
+}
+
 //----------------------------------------------------------------------------------------------
 // LowerStoreLocCommon: platform independent part of local var or field store lowering.
 //
diff --git a/src/coreclr/jit/lower.h b/src/coreclr/jit/lower.h
index 5d4cc1ac080a19..7a3eab44b4eb6e 100644
--- a/src/coreclr/jit/lower.h
+++ b/src/coreclr/jit/lower.h
@@ -155,6 +155,7 @@ class Lowering final : public Phase
     GenTreeCC* LowerNodeCC(GenTree* node, GenCondition condition);
     void       LowerJmpMethod(GenTree* jmp);
     void       LowerRet(GenTreeUnOp* ret);
+    GenTree*   LowerSwiftErrorRet(GenTree* ret);
     void       LowerStoreLocCommon(GenTreeLclVarCommon* lclVar);
     void       LowerRetStruct(GenTreeUnOp* ret);
     void       LowerRetSingleRegStructLclVar(GenTreeUnOp* ret);

From 63eb15c672c804f774af245f77b2a0558703b094 Mon Sep 17 00:00:00 2001
From: Aman Khalid <amankhalid@microsoft.com>
Date: Fri, 5 Apr 2024 16:40:53 +0000
Subject: [PATCH 03/28] Style

---
 src/coreclr/jit/codegencommon.cpp | 4 ++--
 src/coreclr/jit/lower.cpp         | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp
index e7815f8387a8dd..2580c94ad70c94 100644
--- a/src/coreclr/jit/codegencommon.cpp
+++ b/src/coreclr/jit/codegencommon.cpp
@@ -7950,8 +7950,8 @@ void CodeGen::genReturn(GenTree* treeNode)
 void CodeGen::genSwiftErrorReturn(GenTree* treeNode)
 {
     assert(treeNode->OperIs(GT_SWIFT_ERROR_RET));
-    GenTree* swiftErrorNode = treeNode->gtGetOp1();
-    const regNumber srcReg = genConsumeReg(swiftErrorNode);
+    GenTree*        swiftErrorNode = treeNode->gtGetOp1();
+    const regNumber srcReg         = genConsumeReg(swiftErrorNode);
     inst_Mov(swiftErrorNode->TypeGet(), REG_SWIFT_ERROR, srcReg, true, EA_PTRSIZE);
 }
 #endif // SWIFT_SUPPORT
diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp
index beaa473ece51bf..7441359bb59ffb 100644
--- a/src/coreclr/jit/lower.cpp
+++ b/src/coreclr/jit/lower.cpp
@@ -4642,8 +4642,8 @@ void Lowering::LowerRet(GenTreeUnOp* ret)
 GenTree* Lowering::LowerSwiftErrorRet(GenTree* swiftErrorRet)
 {
     assert(swiftErrorRet->OperIs(GT_SWIFT_ERROR_RET));
-    GenTree* const nextNode = swiftErrorRet->gtNext;
-    LIR::Range& blockRange = BlockRange();
+    GenTree* const nextNode   = swiftErrorRet->gtNext;
+    LIR::Range&    blockRange = BlockRange();
     blockRange.Remove(swiftErrorRet);
     blockRange.InsertAtEnd(swiftErrorRet);
     return nextNode;

From 674d8f143c97d2d38ce667b91242bc8f95e7f4af Mon Sep 17 00:00:00 2001
From: Aman Khalid <amankhalid@microsoft.com>
Date: Fri, 5 Apr 2024 16:48:02 +0000
Subject: [PATCH 04/28] Style; comments

---
 src/coreclr/jit/gtlist.h     | 2 +-
 src/coreclr/jit/importer.cpp | 7 ++++++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/coreclr/jit/gtlist.h b/src/coreclr/jit/gtlist.h
index 0e46e7a2671fc8..440e21740ab878 100644
--- a/src/coreclr/jit/gtlist.h
+++ b/src/coreclr/jit/gtlist.h
@@ -290,7 +290,7 @@ GTNODE(END_LFIN         , GenTreeVal         ,0,0,GTK_LEAF|GTK_NOVALUE) // End l
 //  Swift interop-specific nodes:
 //-----------------------------------------------------------------------------
 
-GTNODE(SWIFT_ERROR      , GenTree            ,0,0,GTK_LEAF)              // Error register value post-Swift call
+GTNODE(SWIFT_ERROR      , GenTree            ,0,0,GTK_LEAF)             // Error register value post-Swift call
 GTNODE(SWIFT_ERROR_RET  , GenTreeOp          ,0,0,GTK_UNOP|GTK_NOVALUE) // Returns SwiftError pseudolocal's value in REG_SWIFT_ERROR
 
 //-----------------------------------------------------------------------------
diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp
index 4b3b667436ff7e..6c6f5d9d52af7f 100644
--- a/src/coreclr/jit/importer.cpp
+++ b/src/coreclr/jit/importer.cpp
@@ -10880,10 +10880,15 @@ bool Compiler::impReturnInstruction(int prefixFlags, OPCODE& opcode)
     }
 
 #ifdef SWIFT_SUPPORT
+    // If this method has a SwiftError* out parameter, we need to set the error register upon returning.
+    // By placing the GT_SWIFT_ERROR_RET node before the GT_RETURN node,
+    // we don't have to teach the JIT that GT_SWIFT_ERROR_RET is a valid terminator for BBJ_RETURN blocks.
+    // During lowering, we will move the GT_SWIFT_ERROR_RET to the end of the block
+    // so the error register won't get trashed when returning the normal value.
     if (lvaSwiftErrorArg != BAD_VAR_NUM)
     {
         assert(info.compCallConv == CorInfoCallConvExtension::Swift);
-        assert(lvaSwiftErrorArg != BAD_VAR_NUM);
+        assert(lvaSwiftErrorLocal != BAD_VAR_NUM);
         GenTree* const swiftErrorNode = gtNewLclFldNode(lvaSwiftErrorLocal, TYP_I_IMPL, 0);
         GenTree* const swiftErrorRet  = gtNewOperNode(GT_SWIFT_ERROR_RET, op1->TypeGet(), swiftErrorNode);
         impAppendTree(swiftErrorRet, CHECK_SPILL_NONE, impCurStmtDI);

From 7d2e8fab1ce4772255fa1916557b4e01dfb0d84e Mon Sep 17 00:00:00 2001
From: Aman Khalid <amankhalid@microsoft.com>
Date: Mon, 8 Apr 2024 14:15:36 +0000
Subject: [PATCH 05/28] Binop implementation

---
 src/coreclr/jit/codegenarm64.cpp  |  4 ++--
 src/coreclr/jit/codegencommon.cpp | 27 +++++++++++++++------------
 src/coreclr/jit/codegenxarch.cpp  |  2 +-
 src/coreclr/jit/compiler.h        |  1 -
 src/coreclr/jit/compiler.hpp      |  1 -
 src/coreclr/jit/fgstmt.cpp        |  2 +-
 src/coreclr/jit/flowgraph.cpp     | 11 +++++++++++
 src/coreclr/jit/gentree.cpp       |  7 ++++---
 src/coreclr/jit/gentree.h         |  3 +++
 src/coreclr/jit/gtlist.h          |  4 ++--
 src/coreclr/jit/importer.cpp      |  4 ++--
 src/coreclr/jit/lower.cpp         | 13 ++++++++-----
 src/coreclr/jit/lsraxarch.cpp     |  1 +
 src/coreclr/jit/morph.cpp         | 31 +++++++++++++++++++++----------
 src/coreclr/jit/valuenum.cpp      | 15 ++++++++++++++-
 15 files changed, 85 insertions(+), 41 deletions(-)

diff --git a/src/coreclr/jit/codegenarm64.cpp b/src/coreclr/jit/codegenarm64.cpp
index cd1b1558d93e64..32c944c69870ae 100644
--- a/src/coreclr/jit/codegenarm64.cpp
+++ b/src/coreclr/jit/codegenarm64.cpp
@@ -2990,14 +2990,14 @@ void CodeGen::genCodeForStoreLclVar(GenTreeLclVar* lclNode)
 // Note: treeNode's and op1's registers are already consumed.
 //
 // Arguments:
-//    treeNode - The GT_RETURN or GT_RETFILT tree node with non-struct and non-void type
+//    treeNode - The GT_RETURN/GT_RETFILT/GT_SWIFT_ERROR_RET tree node with non-struct and non-void type
 //
 // Return Value:
 //    None
 //
 void CodeGen::genSimpleReturn(GenTree* treeNode)
 {
-    assert(treeNode->OperGet() == GT_RETURN || treeNode->OperGet() == GT_RETFILT);
+    assert(treeNode->OperIs(GT_RETURN, GT_RETFILT, GT_SWIFT_ERROR_RET));
     GenTree*  op1        = treeNode->gtGetOp1();
     var_types targetType = treeNode->TypeGet();
 
diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp
index 2580c94ad70c94..7bfbcc7210f960 100644
--- a/src/coreclr/jit/codegencommon.cpp
+++ b/src/coreclr/jit/codegencommon.cpp
@@ -7759,14 +7759,14 @@ void CodeGen::genLongReturn(GenTree* treeNode)
 //            In case of LONG return on 32-bit, delegates to the genLongReturn method.
 //
 // Arguments:
-//    treeNode - The GT_RETURN or GT_RETFILT tree node.
+//    treeNode - The GT_RETURN/GT_RETFILT/GT_SWIFT_ERROR_RET tree node.
 //
 // Return Value:
 //    None
 //
 void CodeGen::genReturn(GenTree* treeNode)
 {
-    assert(treeNode->OperIs(GT_RETURN, GT_RETFILT));
+    assert(treeNode->OperIs(GT_RETURN, GT_RETFILT, GT_SWIFT_ERROR_RET));
 
     GenTree*  op1        = treeNode->gtGetOp1();
     var_types targetType = treeNode->TypeGet();
@@ -7870,7 +7870,7 @@ void CodeGen::genReturn(GenTree* treeNode)
     //
     // There should be a single GT_RETURN while generating profiler ELT callbacks.
     //
-    if (treeNode->OperIs(GT_RETURN) && compiler->compIsProfilerHookNeeded())
+    if (treeNode->OperIs(GT_RETURN, GT_SWIFT_ERROR_RET) && compiler->compIsProfilerHookNeeded())
     {
         // !! NOTE !!
         // Since we are invalidating the assumption that we would slip into the epilog
@@ -7939,7 +7939,8 @@ void CodeGen::genReturn(GenTree* treeNode)
 
 #ifdef SWIFT_SUPPORT
 //------------------------------------------------------------------------
-// genSwiftErrorReturn: Generates code for loading the SwiftError pseudolocal value into the error register.
+// genSwiftErrorReturn: Generates code for returning the normal return value,
+//                      and loading the SwiftError pseudolocal value in the error register.
 //
 // Arguments:
 //    treeNode - The GT_SWIFT_ERROR_RET tree node.
@@ -7950,9 +7951,11 @@ void CodeGen::genReturn(GenTree* treeNode)
 void CodeGen::genSwiftErrorReturn(GenTree* treeNode)
 {
     assert(treeNode->OperIs(GT_SWIFT_ERROR_RET));
-    GenTree*        swiftErrorNode = treeNode->gtGetOp1();
-    const regNumber srcReg         = genConsumeReg(swiftErrorNode);
-    inst_Mov(swiftErrorNode->TypeGet(), REG_SWIFT_ERROR, srcReg, true, EA_PTRSIZE);
+    std::swap(treeNode->AsOp()->gtOp1, treeNode->AsOp()->gtOp2);
+    GenTree*        swiftErrorNode = treeNode->gtGetOp2();
+    const regNumber errorSrcReg    = genConsumeReg(swiftErrorNode);
+    genReturn(treeNode);
+    inst_Mov(swiftErrorNode->TypeGet(), REG_SWIFT_ERROR, errorSrcReg, true, EA_PTRSIZE);
 }
 #endif // SWIFT_SUPPORT
 
@@ -7963,15 +7966,15 @@ void CodeGen::genSwiftErrorReturn(GenTree* treeNode)
 //    treeNode - The tree node to evaluate whether is a struct return.
 //
 // Return Value:
-//    Returns true if the 'treeNode" is a GT_RETURN node of type struct.
+//    Returns true if the 'treeNode" is a GT_RETURN/GT_SWIFT_ERROR_RET node of type struct.
 //    Otherwise returns false.
 //
 bool CodeGen::isStructReturn(GenTree* treeNode)
 {
-    // This method could be called for 'treeNode' of GT_RET_FILT or GT_RETURN.
+    // This method could be called for 'treeNode' of GT_RET_FILT/GT_RETURN/GT_SWIFT_ERROR_RET.
     // For the GT_RET_FILT, the return is always a bool or a void, for the end of a finally block.
-    noway_assert(treeNode->OperGet() == GT_RETURN || treeNode->OperGet() == GT_RETFILT);
-    if (treeNode->OperGet() != GT_RETURN)
+    noway_assert(treeNode->OperIs(GT_RETURN, GT_RETFILT, GT_SWIFT_ERROR_RET));
+    if (!treeNode->OperIs(GT_RETURN, GT_SWIFT_ERROR_RET))
     {
         return false;
     }
@@ -7998,7 +8001,7 @@ bool CodeGen::isStructReturn(GenTree* treeNode)
 //
 void CodeGen::genStructReturn(GenTree* treeNode)
 {
-    assert(treeNode->OperGet() == GT_RETURN);
+    assert(treeNode->OperIs(GT_RETURN, GT_SWIFT_ERROR_RET));
 
     genConsumeRegs(treeNode->gtGetOp1());
 
diff --git a/src/coreclr/jit/codegenxarch.cpp b/src/coreclr/jit/codegenxarch.cpp
index be3c4beca93a70..27b7cee27d80a6 100644
--- a/src/coreclr/jit/codegenxarch.cpp
+++ b/src/coreclr/jit/codegenxarch.cpp
@@ -1404,7 +1404,7 @@ void CodeGen::genSIMDSplitReturn(GenTree* src, ReturnTypeDesc* retTypeDesc)
 //
 void CodeGen::genFloatReturn(GenTree* treeNode)
 {
-    assert(treeNode->OperGet() == GT_RETURN || treeNode->OperGet() == GT_RETFILT);
+    assert(treeNode->OperIs(GT_RETURN, GT_RETFILT));
     assert(varTypeIsFloating(treeNode));
 
     GenTree* op1 = treeNode->gtGetOp1();
diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h
index 426faa73adace2..f27618af986f09 100644
--- a/src/coreclr/jit/compiler.h
+++ b/src/coreclr/jit/compiler.h
@@ -11465,7 +11465,6 @@ class GenTreeVisitor
             case GT_FIELD_ADDR:
             case GT_RETURN:
             case GT_RETFILT:
-            case GT_SWIFT_ERROR_RET:
             case GT_RUNTIMELOOKUP:
             case GT_ARR_ADDR:
             case GT_KEEPALIVE:
diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp
index 2934db7d53d0b8..62efd5282a16a1 100644
--- a/src/coreclr/jit/compiler.hpp
+++ b/src/coreclr/jit/compiler.hpp
@@ -4291,7 +4291,6 @@ void GenTree::VisitOperands(TVisitor visitor)
         case GT_RETURNTRAP:
         case GT_KEEPALIVE:
         case GT_INC_SATURATE:
-        case GT_SWIFT_ERROR_RET:
             visitor(this->AsUnOp()->gtOp1);
             return;
 
diff --git a/src/coreclr/jit/fgstmt.cpp b/src/coreclr/jit/fgstmt.cpp
index cd23733da25ab1..b8cc6afd885f2e 100644
--- a/src/coreclr/jit/fgstmt.cpp
+++ b/src/coreclr/jit/fgstmt.cpp
@@ -197,7 +197,7 @@ void Compiler::fgInsertStmtNearEnd(BasicBlock* block, Statement* stmt)
         }
         else if (block->KindIs(BBJ_RETURN))
         {
-            assert((lastStmt->GetRootNode()->OperIs(GT_RETURN, GT_JMP)) ||
+            assert((lastStmt->GetRootNode()->OperIs(GT_RETURN, GT_SWIFT_ERROR_RET, GT_JMP)) ||
                    // BBJ_RETURN blocks in functions returning void do not get a GT_RETURN node if they
                    // have a .tail prefix (even if canTailCall returns false for these calls)
                    // code:Compiler::impImportBlockCode (search for the RET: label)
diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp
index 7cedd596b12e8b..09dedf991038e6 100644
--- a/src/coreclr/jit/flowgraph.cpp
+++ b/src/coreclr/jit/flowgraph.cpp
@@ -2028,6 +2028,17 @@ class MergedReturns
             returnExpr = new (comp, GT_RETURN) GenTreeOp(GT_RETURN, TYP_VOID);
         }
 
+#ifdef SWIFT_SUPPORT
+        if (comp->lvaSwiftErrorArg != BAD_VAR_NUM)
+        {
+            assert(comp->info.compCallConv == CorInfoCallConvExtension::Swift);
+            assert(comp->lvaSwiftErrorLocal != BAD_VAR_NUM);
+            GenTree* const swiftErrorNode = comp->gtNewLclFldNode(comp->lvaSwiftErrorLocal, TYP_I_IMPL, 0);
+            returnExpr                    = new (comp, GT_SWIFT_ERROR_RET)
+                GenTreeOp(GT_SWIFT_ERROR_RET, returnExpr->TypeGet(), swiftErrorNode, returnExpr->gtGetOp1());
+        }
+#endif // SWIFT_SUPPORT
+
         // Add 'return' expression to the return block
         comp->fgNewStmtAtEnd(newReturnBB, returnExpr);
         // Flag that this 'return' was generated by return merging so that subsequent
diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp
index 947035a8998592..e71b4825f14ae0 100644
--- a/src/coreclr/jit/gentree.cpp
+++ b/src/coreclr/jit/gentree.cpp
@@ -6719,7 +6719,6 @@ bool GenTree::TryGetUse(GenTree* operand, GenTree*** pUse)
         case GT_BSWAP16:
         case GT_KEEPALIVE:
         case GT_INC_SATURATE:
-        case GT_SWIFT_ERROR_RET:
             if (operand == this->AsUnOp()->gtOp1)
             {
                 *pUse = &this->AsUnOp()->gtOp1;
@@ -7371,7 +7370,6 @@ bool GenTree::OperSupportsOrderingSideEffect() const
         case GT_MEMORYBARRIER:
         case GT_CATCH_ARG:
         case GT_SWIFT_ERROR:
-        case GT_SWIFT_ERROR_RET:
             return true;
         default:
             return false;
@@ -9633,6 +9631,10 @@ GenTree* Compiler::gtCloneExpr(GenTree* tree)
             }
             break;
 
+            case GT_SWIFT_ERROR_RET:
+                copy = new (this, oper) GenTreeOp(oper, tree->TypeGet(), tree->gtGetOp1(), tree->gtGetOp2());
+                break;
+
             default:
                 assert(!GenTree::IsExOp(tree->OperKind()) && tree->OperIsSimple());
                 // We're in the SimpleOp case, so it's always unary or binary.
@@ -10291,7 +10293,6 @@ GenTreeUseEdgeIterator::GenTreeUseEdgeIterator(GenTree* node)
         case GT_PUTARG_SPLIT:
 #endif // FEATURE_ARG_SPLIT
         case GT_RETURNTRAP:
-        case GT_SWIFT_ERROR_RET:
             m_edge = &m_node->AsUnOp()->gtOp1;
             assert(*m_edge != nullptr);
             m_advance = &GenTreeUseEdgeIterator::Terminate;
diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h
index a0e8eb7242e6cf..2f935b061eb776 100644
--- a/src/coreclr/jit/gentree.h
+++ b/src/coreclr/jit/gentree.h
@@ -1739,6 +1739,9 @@ struct GenTree
 #endif // defined(TARGET_ARM64)
 
                 return true;
+
+            case GT_SWIFT_ERROR_RET:
+                return (gtType == TYP_VOID);
             default:
                 return false;
         }
diff --git a/src/coreclr/jit/gtlist.h b/src/coreclr/jit/gtlist.h
index 440e21740ab878..feba3e1e383960 100644
--- a/src/coreclr/jit/gtlist.h
+++ b/src/coreclr/jit/gtlist.h
@@ -290,8 +290,8 @@ GTNODE(END_LFIN         , GenTreeVal         ,0,0,GTK_LEAF|GTK_NOVALUE) // End l
 //  Swift interop-specific nodes:
 //-----------------------------------------------------------------------------
 
-GTNODE(SWIFT_ERROR      , GenTree            ,0,0,GTK_LEAF)             // Error register value post-Swift call
-GTNODE(SWIFT_ERROR_RET  , GenTreeOp          ,0,0,GTK_UNOP|GTK_NOVALUE) // Returns SwiftError pseudolocal's value in REG_SWIFT_ERROR
+GTNODE(SWIFT_ERROR      , GenTree            ,0,0,GTK_LEAF)              // Error register value post-Swift call
+GTNODE(SWIFT_ERROR_RET  , GenTreeOp          ,0,1,GTK_BINOP|GTK_NOVALUE) // Returns SwiftError pseudolocal's value in REG_SWIFT_ERROR
 
 //-----------------------------------------------------------------------------
 //  Nodes used by Lower to generate a closer CPU representation of other nodes
diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp
index 6c6f5d9d52af7f..53f22a56a9f820 100644
--- a/src/coreclr/jit/importer.cpp
+++ b/src/coreclr/jit/importer.cpp
@@ -10890,8 +10890,8 @@ bool Compiler::impReturnInstruction(int prefixFlags, OPCODE& opcode)
         assert(info.compCallConv == CorInfoCallConvExtension::Swift);
         assert(lvaSwiftErrorLocal != BAD_VAR_NUM);
         GenTree* const swiftErrorNode = gtNewLclFldNode(lvaSwiftErrorLocal, TYP_I_IMPL, 0);
-        GenTree* const swiftErrorRet  = gtNewOperNode(GT_SWIFT_ERROR_RET, op1->TypeGet(), swiftErrorNode);
-        impAppendTree(swiftErrorRet, CHECK_SPILL_NONE, impCurStmtDI);
+        op1                           = new (this, GT_SWIFT_ERROR_RET)
+            GenTreeOp(GT_SWIFT_ERROR_RET, op1->TypeGet(), swiftErrorNode, op1->gtGetOp1());
     }
 #endif // SWIFT_SUPPORT
 
diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp
index 7441359bb59ffb..6aff5b84ed1a04 100644
--- a/src/coreclr/jit/lower.cpp
+++ b/src/coreclr/jit/lower.cpp
@@ -525,7 +525,10 @@ GenTree* Lowering::LowerNode(GenTree* node)
             break;
 
         case GT_SWIFT_ERROR_RET:
-            return LowerSwiftErrorRet(node);
+            std::swap(node->AsOp()->gtOp1, node->AsOp()->gtOp2);
+            LowerRet(node->AsUnOp());
+            std::swap(node->AsOp()->gtOp1, node->AsOp()->gtOp2);
+            break;
 
         case GT_RETURNTRAP:
             ContainCheckReturnTrap(node->AsOp());
@@ -4547,7 +4550,7 @@ void Lowering::LowerJmpMethod(GenTree* jmp)
 // Lower GT_RETURN node to insert PInvoke method epilog if required.
 void Lowering::LowerRet(GenTreeUnOp* ret)
 {
-    assert(ret->OperGet() == GT_RETURN);
+    assert(ret->OperIs(GT_RETURN, GT_SWIFT_ERROR_RET));
 
     JITDUMP("lowering GT_RETURN\n");
     DISPNODE(ret);
@@ -4896,7 +4899,7 @@ void Lowering::LowerRetStruct(GenTreeUnOp* ret)
         return;
     }
 
-    assert(ret->OperIs(GT_RETURN));
+    assert(ret->OperIs(GT_RETURN, GT_SWIFT_ERROR_RET));
     assert(varTypeIsStruct(ret));
 
     GenTree*  retVal           = ret->gtGetOp1();
@@ -4988,7 +4991,7 @@ void Lowering::LowerRetStruct(GenTreeUnOp* ret)
 void Lowering::LowerRetSingleRegStructLclVar(GenTreeUnOp* ret)
 {
     assert(!comp->compMethodReturnsMultiRegRetType());
-    assert(ret->OperIs(GT_RETURN));
+    assert(ret->OperIs(GT_RETURN, GT_SWIFT_ERROR_RET));
     GenTreeLclVarCommon* lclVar = ret->gtGetOp1()->AsLclVar();
     assert(lclVar->OperIs(GT_LCL_VAR));
     unsigned   lclNum = lclVar->GetLclNum();
@@ -8172,7 +8175,7 @@ void Lowering::ContainCheckLclHeap(GenTreeOp* node)
 //
 void Lowering::ContainCheckRet(GenTreeUnOp* ret)
 {
-    assert(ret->OperIs(GT_RETURN));
+    assert(ret->OperIs(GT_RETURN, GT_SWIFT_ERROR_RET));
 
 #if !defined(TARGET_64BIT)
     if (ret->TypeGet() == TYP_LONG)
diff --git a/src/coreclr/jit/lsraxarch.cpp b/src/coreclr/jit/lsraxarch.cpp
index f380daeab59ac2..8481593f34dada 100644
--- a/src/coreclr/jit/lsraxarch.cpp
+++ b/src/coreclr/jit/lsraxarch.cpp
@@ -733,6 +733,7 @@ bool LinearScan::isRMWRegOper(GenTree* tree)
 #ifdef TARGET_X86
         case GT_LONG:
 #endif
+        case GT_SWIFT_ERROR_RET:
             return false;
 
         case GT_ADD:
diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp
index eb22f9d8f9ce5a..d77f9ffdd0fe0a 100644
--- a/src/coreclr/jit/morph.cpp
+++ b/src/coreclr/jit/morph.cpp
@@ -14069,7 +14069,7 @@ void Compiler::fgMergeBlockReturn(BasicBlock* block)
     Statement* lastStmt = block->lastStmt();
     GenTree*   ret      = (lastStmt != nullptr) ? lastStmt->GetRootNode() : nullptr;
 
-    if ((ret != nullptr) && (ret->OperGet() == GT_RETURN) && ((ret->gtFlags & GTF_RET_MERGED) != 0))
+    if ((ret != nullptr) && ret->OperIs(GT_RETURN, GT_SWIFT_ERROR_RET) && ((ret->gtFlags & GTF_RET_MERGED) != 0))
     {
         // This return was generated during epilog merging, so leave it alone
     }
@@ -14092,7 +14092,8 @@ void Compiler::fgMergeBlockReturn(BasicBlock* block)
 
         if (genReturnLocal != BAD_VAR_NUM)
         {
-            // replace the GT_RETURN node to be a STORE_LCL_VAR that stores the return value into genReturnLocal.
+            // replace the GT_RETURN/GT_SWIFT_ERROR_RET node to be a STORE_LCL_VAR that stores the return value into
+            // genReturnLocal.
 
             // Method must be returning a value other than TYP_VOID.
             noway_assert(compMethodHasRetVal());
@@ -14104,13 +14105,22 @@ void Compiler::fgMergeBlockReturn(BasicBlock* block)
 
             // GT_RETURN must have non-null operand as the method is returning the value assigned to
             // genReturnLocal
-            noway_assert(ret->OperGet() == GT_RETURN);
-            noway_assert(ret->gtGetOp1() != nullptr);
+            GenTree* retVal;
+            if (ret->OperIs(GT_SWIFT_ERROR_RET))
+            {
+                retVal = ret->gtGetOp2();
+            }
+            else
+            {
+                noway_assert(ret->OperIs(GT_RETURN));
+                retVal = ret->gtGetOp1();
+            }
+
+            noway_assert(retVal != nullptr);
 
             Statement*       pAfterStatement = lastStmt;
             const DebugInfo& di              = lastStmt->GetDebugInfo();
-            GenTree*         tree =
-                gtNewTempStore(genReturnLocal, ret->gtGetOp1(), CHECK_SPILL_NONE, &pAfterStatement, di, block);
+            GenTree* tree = gtNewTempStore(genReturnLocal, retVal, CHECK_SPILL_NONE, &pAfterStatement, di, block);
             if (tree->OperIsCopyBlkOp())
             {
                 tree = fgMorphCopyBlock(tree);
@@ -14133,16 +14143,17 @@ void Compiler::fgMergeBlockReturn(BasicBlock* block)
                 lastStmt = newStmt;
             }
         }
-        else if (ret != nullptr && ret->OperGet() == GT_RETURN)
+        else if ((ret != nullptr) && ret->OperIs(GT_RETURN, GT_SWIFT_ERROR_RET))
         {
             // This block ends with a GT_RETURN
             noway_assert(lastStmt != nullptr);
             noway_assert(lastStmt->GetNextStmt() == nullptr);
 
-            // Must be a void GT_RETURN with null operand; delete it as this block branches to oneReturn
-            // block
+            // Must be a void GT_RETURN/GT_SWIFT_ERROR_RET with null operand; delete it as this block branches to
+            // oneReturn block
+            GenTree* const retVal = ret->OperIs(GT_RETURN) ? ret->gtGetOp1() : ret->gtGetOp2();
             noway_assert(ret->TypeGet() == TYP_VOID);
-            noway_assert(ret->gtGetOp1() == nullptr);
+            noway_assert(retVal == nullptr);
 
             if (opts.compDbgCode && lastStmt->GetDebugInfo().IsValid())
             {
diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp
index 889e0227e992d9..48e9d2adfeed47 100644
--- a/src/coreclr/jit/valuenum.cpp
+++ b/src/coreclr/jit/valuenum.cpp
@@ -9569,7 +9569,8 @@ static genTreeOps genTreeOpsIllegalAsVNFunc[] = {GT_IND, // When we do heap memo
                                                  GT_NOP,
 
                                                  // These control-flow operations need no values.
-                                                 GT_JTRUE, GT_RETURN, GT_SWITCH, GT_RETFILT, GT_CKFINITE};
+                                                 GT_JTRUE, GT_RETURN, GT_SWITCH, GT_RETFILT, GT_CKFINITE,
+                                                 GT_SWIFT_ERROR_RET};
 
 void ValueNumStore::ValidateValueNumStoreStatics()
 {
@@ -11703,6 +11704,18 @@ void Compiler::fgValueNumberTree(GenTree* tree)
                         }
                         break;
 
+                    case GT_SWIFT_ERROR_RET:
+                        if (tree->gtGetOp2() != nullptr)
+                        {
+                            tree->gtVNPair = vnStore->VNPWithExc(vnStore->VNPForVoid(),
+                                                                 vnStore->VNPExceptionSet(tree->gtGetOp2()->gtVNPair));
+                        }
+                        else
+                        {
+                            tree->gtVNPair = vnStore->VNPForVoid();
+                        }
+                        break;
+
                     // BOX and CKFINITE are passthrough nodes (like NOP). We'll add the exception for the latter later.
                     case GT_BOX:
                     case GT_CKFINITE:

From a342e0008e9b7f2d3d205a86924d2e79a9d5a085 Mon Sep 17 00:00:00 2001
From: Aman Khalid <amankhalid@microsoft.com>
Date: Mon, 8 Apr 2024 14:16:17 +0000
Subject: [PATCH 06/28] Update comment

---
 src/coreclr/jit/gtlist.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/coreclr/jit/gtlist.h b/src/coreclr/jit/gtlist.h
index feba3e1e383960..de0c995a1bb043 100644
--- a/src/coreclr/jit/gtlist.h
+++ b/src/coreclr/jit/gtlist.h
@@ -291,7 +291,7 @@ GTNODE(END_LFIN         , GenTreeVal         ,0,0,GTK_LEAF|GTK_NOVALUE) // End l
 //-----------------------------------------------------------------------------
 
 GTNODE(SWIFT_ERROR      , GenTree            ,0,0,GTK_LEAF)              // Error register value post-Swift call
-GTNODE(SWIFT_ERROR_RET  , GenTreeOp          ,0,1,GTK_BINOP|GTK_NOVALUE) // Returns SwiftError pseudolocal's value in REG_SWIFT_ERROR
+GTNODE(SWIFT_ERROR_RET  , GenTreeOp          ,0,1,GTK_BINOP|GTK_NOVALUE) // Returns normal return value, and SwiftError pseudolocal's value in REG_SWIFT_ERROR
 
 //-----------------------------------------------------------------------------
 //  Nodes used by Lower to generate a closer CPU representation of other nodes

From aa8c83c2b461a48e2a29c43a119eb8c2f150c707 Mon Sep 17 00:00:00 2001
From: Aman Khalid <amankhalid@microsoft.com>
Date: Mon, 8 Apr 2024 14:33:31 +0000
Subject: [PATCH 07/28] Comments

---
 src/coreclr/jit/codegencommon.cpp | 4 +++-
 src/coreclr/jit/codegenxarch.cpp  | 2 ++
 src/coreclr/jit/lower.cpp         | 2 ++
 src/coreclr/jit/lsra.cpp          | 5 ++---
 4 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp
index cb24d3fa5a0a9c..efb02d56a7b3d7 100644
--- a/src/coreclr/jit/codegencommon.cpp
+++ b/src/coreclr/jit/codegencommon.cpp
@@ -7955,6 +7955,8 @@ void CodeGen::genReturn(GenTree* treeNode)
 void CodeGen::genSwiftErrorReturn(GenTree* treeNode)
 {
     assert(treeNode->OperIs(GT_SWIFT_ERROR_RET));
+    // The first operand is the error value, but genReturn expects it to be the normal return value,
+    // so swap them.
     std::swap(treeNode->AsOp()->gtOp1, treeNode->AsOp()->gtOp2);
     GenTree*        swiftErrorNode = treeNode->gtGetOp2();
     const regNumber errorSrcReg    = genConsumeReg(swiftErrorNode);
@@ -7970,7 +7972,7 @@ void CodeGen::genSwiftErrorReturn(GenTree* treeNode)
 //    treeNode - The tree node to evaluate whether is a struct return.
 //
 // Return Value:
-//    Returns true if the 'treeNode" is a GT_RETURN/GT_SWIFT_ERROR_RET node of type struct.
+//    Returns true if the 'treeNode' is a GT_RETURN/GT_SWIFT_ERROR_RET node of type struct.
 //    Otherwise returns false.
 //
 bool CodeGen::isStructReturn(GenTree* treeNode)
diff --git a/src/coreclr/jit/codegenxarch.cpp b/src/coreclr/jit/codegenxarch.cpp
index 65ac843f2378e3..79b76a6c8cc25b 100644
--- a/src/coreclr/jit/codegenxarch.cpp
+++ b/src/coreclr/jit/codegenxarch.cpp
@@ -1402,6 +1402,8 @@ void CodeGen::genSIMDSplitReturn(GenTree* src, ReturnTypeDesc* retTypeDesc)
 //
 // Arguments:
 //    treeNode - The GT_RETURN or GT_RETFILT tree node with float type.
+//    (We don't expect treeNode to be a GT_SWIFT_ERROR_RET node,
+//    as Swift interop isn't supported on x86.)
 //
 // Return Value:
 //    None
diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp
index f330a51977d4b1..a46a5b7349f1cc 100644
--- a/src/coreclr/jit/lower.cpp
+++ b/src/coreclr/jit/lower.cpp
@@ -525,6 +525,8 @@ GenTree* Lowering::LowerNode(GenTree* node)
             break;
 
         case GT_SWIFT_ERROR_RET:
+            // The first operand is the error value, but LowerRet expects it to be the normal return value,
+            // so swap them.
             std::swap(node->AsOp()->gtOp1, node->AsOp()->gtOp2);
             LowerRet(node->AsUnOp());
             std::swap(node->AsOp()->gtOp1, node->AsOp()->gtOp2);
diff --git a/src/coreclr/jit/lsra.cpp b/src/coreclr/jit/lsra.cpp
index 831f6c7ac194cf..510a2c80f29a9d 100644
--- a/src/coreclr/jit/lsra.cpp
+++ b/src/coreclr/jit/lsra.cpp
@@ -8515,8 +8515,7 @@ void LinearScan::insertMove(
             noway_assert(!blockRange.IsEmpty());
 
             GenTree* branch = lastNode;
-            assert(branch->OperIsConditionalJump() || branch->OperGet() == GT_SWITCH_TABLE ||
-                   branch->OperGet() == GT_SWITCH);
+            assert(branch->OperIsConditionalJump() || branch->OperIs(GT_SWITCH_TABLE, GT_SWITCH));
 
             blockRange.InsertBefore(branch, std::move(treeRange));
         }
@@ -8524,7 +8523,7 @@ void LinearScan::insertMove(
         {
             // These block kinds don't have a branch at the end.
             assert((lastNode == nullptr) || (!lastNode->OperIsConditionalJump() &&
-                                             !lastNode->OperIs(GT_SWITCH_TABLE, GT_SWITCH, GT_RETURN, GT_RETFILT)));
+                                             !lastNode->OperIs(GT_SWITCH_TABLE, GT_SWITCH, GT_RETURN, GT_RETFILT, GT_SWIFT_ERROR_RET)));
             blockRange.InsertAtEnd(std::move(treeRange));
         }
     }

From 9e310ecd3272388d3e0a5f5d5be7d3598b8446ab Mon Sep 17 00:00:00 2001
From: Aman Khalid <amankhalid@microsoft.com>
Date: Mon, 8 Apr 2024 14:36:11 +0000
Subject: [PATCH 08/28] Style

---
 src/coreclr/jit/lsra.cpp | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/coreclr/jit/lsra.cpp b/src/coreclr/jit/lsra.cpp
index 510a2c80f29a9d..af29eef1eb22a5 100644
--- a/src/coreclr/jit/lsra.cpp
+++ b/src/coreclr/jit/lsra.cpp
@@ -8522,8 +8522,9 @@ void LinearScan::insertMove(
         else
         {
             // These block kinds don't have a branch at the end.
-            assert((lastNode == nullptr) || (!lastNode->OperIsConditionalJump() &&
-                                             !lastNode->OperIs(GT_SWITCH_TABLE, GT_SWITCH, GT_RETURN, GT_RETFILT, GT_SWIFT_ERROR_RET)));
+            assert((lastNode == nullptr) ||
+                   (!lastNode->OperIsConditionalJump() &&
+                    !lastNode->OperIs(GT_SWITCH_TABLE, GT_SWITCH, GT_RETURN, GT_RETFILT, GT_SWIFT_ERROR_RET)));
             blockRange.InsertAtEnd(std::move(treeRange));
         }
     }

From 15d39bff2f378fb7d44b88921d9d385be905fba6 Mon Sep 17 00:00:00 2001
From: Aman Khalid <amankhalid@microsoft.com>
Date: Mon, 8 Apr 2024 18:38:59 +0000
Subject: [PATCH 09/28] Add GetReturnValue helper

---
 src/coreclr/jit/codegencommon.cpp |  7 ++-----
 src/coreclr/jit/gentree.h         | 13 +++++++++++++
 src/coreclr/jit/lower.cpp         | 11 ++---------
 3 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp
index efb02d56a7b3d7..15fdec607b8d9c 100644
--- a/src/coreclr/jit/codegencommon.cpp
+++ b/src/coreclr/jit/codegencommon.cpp
@@ -7767,7 +7767,7 @@ void CodeGen::genReturn(GenTree* treeNode)
 {
     assert(treeNode->OperIs(GT_RETURN, GT_RETFILT, GT_SWIFT_ERROR_RET));
 
-    GenTree*  op1        = treeNode->gtGetOp1();
+    GenTree*  op1        = treeNode->AsOp()->GetReturnValue();
     var_types targetType = treeNode->TypeGet();
 
     // A void GT_RETFILT is the end of a finally. For non-void filter returns we need to load the result in the return
@@ -7955,10 +7955,7 @@ void CodeGen::genReturn(GenTree* treeNode)
 void CodeGen::genSwiftErrorReturn(GenTree* treeNode)
 {
     assert(treeNode->OperIs(GT_SWIFT_ERROR_RET));
-    // The first operand is the error value, but genReturn expects it to be the normal return value,
-    // so swap them.
-    std::swap(treeNode->AsOp()->gtOp1, treeNode->AsOp()->gtOp2);
-    GenTree*        swiftErrorNode = treeNode->gtGetOp2();
+    GenTree*        swiftErrorNode = treeNode->gtGetOp1();
     const regNumber errorSrcReg    = genConsumeReg(swiftErrorNode);
     genReturn(treeNode);
     inst_Mov(swiftErrorNode->TypeGet(), REG_SWIFT_ERROR, errorSrcReg, true, EA_PTRSIZE);
diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h
index b37f4810bc2598..9da96ec025390f 100644
--- a/src/coreclr/jit/gentree.h
+++ b/src/coreclr/jit/gentree.h
@@ -3040,6 +3040,19 @@ struct GenTreeOp : public GenTreeUnOp
     // then sets the flag GTF_DIV_BY_CNS_OPT and GTF_DONT_CSE on the constant
     void CheckDivideByConstOptimized(Compiler* comp);
 
+    GenTree* GetReturnValue() const
+    {
+        assert(OperIs(GT_RETURN, GT_RETFILT, GT_SWIFT_ERROR_RET));
+#ifdef SWIFT_SUPPORT
+        if (OperIs(GT_SWIFT_ERROR_RET))
+        {
+            return gtOp2;
+        }
+#endif // SWIFT_SUPPORT
+
+        return gtOp1;
+    }
+
 #if !defined(TARGET_64BIT) || defined(TARGET_ARM64)
     bool IsValidLongMul();
 #endif
diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp
index a46a5b7349f1cc..1447c76269bf19 100644
--- a/src/coreclr/jit/lower.cpp
+++ b/src/coreclr/jit/lower.cpp
@@ -521,15 +521,8 @@ GenTree* Lowering::LowerNode(GenTree* node)
             break;
 
         case GT_RETURN:
-            LowerRet(node->AsUnOp());
-            break;
-
         case GT_SWIFT_ERROR_RET:
-            // The first operand is the error value, but LowerRet expects it to be the normal return value,
-            // so swap them.
-            std::swap(node->AsOp()->gtOp1, node->AsOp()->gtOp2);
             LowerRet(node->AsUnOp());
-            std::swap(node->AsOp()->gtOp1, node->AsOp()->gtOp2);
             break;
 
         case GT_RETURNTRAP:
@@ -4558,13 +4551,13 @@ void Lowering::LowerRet(GenTreeUnOp* ret)
     DISPNODE(ret);
     JITDUMP("============\n");
 
-    GenTree* retVal = ret->gtGetOp1();
+    GenTree* retVal = ret->AsOp()->GetReturnValue();
     // There are two kinds of retyping:
     // - A simple bitcast can be inserted when:
     //   - We're returning a floating type as an integral type or vice-versa, or
     // - If we're returning a struct as a primitive type, we change the type of
     // 'retval' in 'LowerRetStructLclVar()'
-    bool needBitcast        = (ret->TypeGet() != TYP_VOID) && !varTypeUsesSameRegType(ret, ret->gtGetOp1());
+    bool needBitcast        = (ret->TypeGet() != TYP_VOID) && !varTypeUsesSameRegType(ret, retVal);
     bool doPrimitiveBitcast = false;
     if (needBitcast)
     {

From 873e15a3ea3c2d772c8aee97ca760e8a68ebe4f4 Mon Sep 17 00:00:00 2001
From: Aman Khalid <amankhalid@microsoft.com>
Date: Mon, 8 Apr 2024 22:11:25 +0000
Subject: [PATCH 10/28] Handle GT_SWIFT_ERROR_RET in LSRA

---
 src/coreclr/jit/lsraarm64.cpp | 17 ++++++++++++++++-
 src/coreclr/jit/lsrabuild.cpp |  2 +-
 src/coreclr/jit/lsraxarch.cpp | 17 ++++++++++++++++-
 3 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/src/coreclr/jit/lsraarm64.cpp b/src/coreclr/jit/lsraarm64.cpp
index 1096d7f11701c5..e2617fdcc6cc55 100644
--- a/src/coreclr/jit/lsraarm64.cpp
+++ b/src/coreclr/jit/lsraarm64.cpp
@@ -582,7 +582,7 @@ int LinearScan::BuildNode(GenTree* tree)
 {
     assert(!tree->isContained());
     int       srcCount;
-    int       dstCount      = 0;
+    int       dstCount;
     regMaskTP killMask      = RBM_NONE;
     bool      isLocalDefUse = false;
 
@@ -741,6 +741,21 @@ int LinearScan::BuildNode(GenTree* tree)
             BuildDefsWithKills(tree, 0, RBM_NONE, killMask);
             break;
 
+#ifdef SWIFT_SUPPORT
+        case GT_SWIFT_ERROR_RET:
+            // Plus one for error register
+            srcCount = BuildReturn(tree) + 1;
+            killMask = getKillSetForReturn();
+            BuildDefsWithKills(tree, 0, RBM_NONE, killMask);
+
+            // Any register should do here, but the error register's last use should be for storing the error value
+            // before returning. Instead of loading the SwiftError pseudolocal's value into a temp register,
+            // and then into the error register, use the error register directly.
+            // This will allow us to elide a move instruction during codegen.
+            BuildUse(tree->gtGetOp1(), RBM_SWIFT_ERROR);
+            break;
+#endif // SWIFT_SUPPORT
+
         case GT_RETFILT:
             assert(dstCount == 0);
             if (tree->TypeGet() == TYP_VOID)
diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp
index 6c11e00aa57cb6..dff38bc06f1c5f 100644
--- a/src/coreclr/jit/lsrabuild.cpp
+++ b/src/coreclr/jit/lsrabuild.cpp
@@ -3989,7 +3989,7 @@ int LinearScan::BuildSimple(GenTree* tree)
 //
 int LinearScan::BuildReturn(GenTree* tree)
 {
-    GenTree* op1 = tree->gtGetOp1();
+    GenTree* op1 = tree->AsOp()->GetReturnValue();
 
 #if !defined(TARGET_64BIT)
     if (tree->TypeGet() == TYP_LONG)
diff --git a/src/coreclr/jit/lsraxarch.cpp b/src/coreclr/jit/lsraxarch.cpp
index c0ffad8de3a238..8b25c4721473ca 100644
--- a/src/coreclr/jit/lsraxarch.cpp
+++ b/src/coreclr/jit/lsraxarch.cpp
@@ -46,7 +46,7 @@ int LinearScan::BuildNode(GenTree* tree)
 {
     assert(!tree->isContained());
     int       srcCount;
-    int       dstCount      = 0;
+    int       dstCount;
     regMaskTP killMask      = RBM_NONE;
     bool      isLocalDefUse = false;
 
@@ -193,6 +193,21 @@ int LinearScan::BuildNode(GenTree* tree)
             BuildDefsWithKills(tree, 0, RBM_NONE, killMask);
             break;
 
+#ifdef SWIFT_SUPPORT
+        case GT_SWIFT_ERROR_RET:
+            // Plus one for error register
+            srcCount = BuildReturn(tree) + 1;
+            killMask = getKillSetForReturn();
+            BuildDefsWithKills(tree, 0, RBM_NONE, killMask);
+
+            // Any register should do here, but the error register's last use should be for storing the error value
+            // before returning. Instead of loading the SwiftError pseudolocal's value into a temp register,
+            // and then into the error register, use the error register directly.
+            // This will allow us to elide a move instruction during codegen.
+            BuildUse(tree->gtGetOp1(), RBM_SWIFT_ERROR);
+            break;
+#endif // SWIFT_SUPPORT
+
         case GT_RETFILT:
             assert(dstCount == 0);
             if (tree->TypeGet() == TYP_VOID)

From 5d96892634995f6cb121c55544bb5c2900742a31 Mon Sep 17 00:00:00 2001
From: Aman Khalid <amankhalid@microsoft.com>
Date: Tue, 9 Apr 2024 00:48:53 +0000
Subject: [PATCH 11/28] Overzealous opt

---
 src/coreclr/jit/lsraarm64.cpp | 7 +------
 src/coreclr/jit/lsraxarch.cpp | 7 +------
 2 files changed, 2 insertions(+), 12 deletions(-)

diff --git a/src/coreclr/jit/lsraarm64.cpp b/src/coreclr/jit/lsraarm64.cpp
index e2617fdcc6cc55..a4935e0a331063 100644
--- a/src/coreclr/jit/lsraarm64.cpp
+++ b/src/coreclr/jit/lsraarm64.cpp
@@ -747,12 +747,7 @@ int LinearScan::BuildNode(GenTree* tree)
             srcCount = BuildReturn(tree) + 1;
             killMask = getKillSetForReturn();
             BuildDefsWithKills(tree, 0, RBM_NONE, killMask);
-
-            // Any register should do here, but the error register's last use should be for storing the error value
-            // before returning. Instead of loading the SwiftError pseudolocal's value into a temp register,
-            // and then into the error register, use the error register directly.
-            // This will allow us to elide a move instruction during codegen.
-            BuildUse(tree->gtGetOp1(), RBM_SWIFT_ERROR);
+            BuildUse(tree->gtGetOp1(), RBM_NONE);
             break;
 #endif // SWIFT_SUPPORT
 
diff --git a/src/coreclr/jit/lsraxarch.cpp b/src/coreclr/jit/lsraxarch.cpp
index 8b25c4721473ca..b5a80deb7f76cf 100644
--- a/src/coreclr/jit/lsraxarch.cpp
+++ b/src/coreclr/jit/lsraxarch.cpp
@@ -199,12 +199,7 @@ int LinearScan::BuildNode(GenTree* tree)
             srcCount = BuildReturn(tree) + 1;
             killMask = getKillSetForReturn();
             BuildDefsWithKills(tree, 0, RBM_NONE, killMask);
-
-            // Any register should do here, but the error register's last use should be for storing the error value
-            // before returning. Instead of loading the SwiftError pseudolocal's value into a temp register,
-            // and then into the error register, use the error register directly.
-            // This will allow us to elide a move instruction during codegen.
-            BuildUse(tree->gtGetOp1(), RBM_SWIFT_ERROR);
+            BuildUse(tree->gtGetOp1(), RBM_NONE);
             break;
 #endif // SWIFT_SUPPORT
 

From 24e8d0d89eba4efb1aab709d20a0b37f16285d71 Mon Sep 17 00:00:00 2001
From: Aman Khalid <amankhalid@microsoft.com>
Date: Tue, 9 Apr 2024 15:17:58 +0000
Subject: [PATCH 12/28] Feedback

---
 src/coreclr/jit/codegencommon.cpp |  2 +-
 src/coreclr/jit/gtlist.h          |  3 ++-
 src/coreclr/jit/importer.cpp      | 11 +++++------
 src/coreclr/jit/lower.cpp         | 21 ---------------------
 src/coreclr/jit/lower.h           |  1 -
 src/coreclr/jit/lsraarm64.cpp     |  2 +-
 src/coreclr/jit/lsraxarch.cpp     |  2 +-
 src/coreclr/jit/morph.cpp         |  2 +-
 8 files changed, 11 insertions(+), 33 deletions(-)

diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp
index 15fdec607b8d9c..f46ccdc4409808 100644
--- a/src/coreclr/jit/codegencommon.cpp
+++ b/src/coreclr/jit/codegencommon.cpp
@@ -7957,8 +7957,8 @@ void CodeGen::genSwiftErrorReturn(GenTree* treeNode)
     assert(treeNode->OperIs(GT_SWIFT_ERROR_RET));
     GenTree*        swiftErrorNode = treeNode->gtGetOp1();
     const regNumber errorSrcReg    = genConsumeReg(swiftErrorNode);
-    genReturn(treeNode);
     inst_Mov(swiftErrorNode->TypeGet(), REG_SWIFT_ERROR, errorSrcReg, true, EA_PTRSIZE);
+    genReturn(treeNode);
 }
 #endif // SWIFT_SUPPORT
 
diff --git a/src/coreclr/jit/gtlist.h b/src/coreclr/jit/gtlist.h
index 2a3c5e70a6c01d..2554bb22ef7040 100644
--- a/src/coreclr/jit/gtlist.h
+++ b/src/coreclr/jit/gtlist.h
@@ -291,7 +291,8 @@ GTNODE(END_LFIN         , GenTreeVal         ,0,0,GTK_LEAF|GTK_NOVALUE) // End l
 //-----------------------------------------------------------------------------
 
 GTNODE(SWIFT_ERROR      , GenTree            ,0,0,GTK_LEAF)              // Error register value post-Swift call
-GTNODE(SWIFT_ERROR_RET  , GenTreeOp          ,0,1,GTK_BINOP|GTK_NOVALUE) // Returns normal return value, and SwiftError pseudolocal's value in REG_SWIFT_ERROR
+GTNODE(SWIFT_ERROR_RET  , GenTreeOp          ,0,1,GTK_BINOP|GTK_NOVALUE) // Returns normal return value, and SwiftError pseudolocal's value in REG_SWIFT_ERROR.
+																		 // op1 is the error value, and op2 is the return value (or null if the method returns void).
 
 //-----------------------------------------------------------------------------
 //  Nodes used by Lower to generate a closer CPU representation of other nodes
diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp
index 4103938ccd2818..d9b48c7e8b55ad 100644
--- a/src/coreclr/jit/importer.cpp
+++ b/src/coreclr/jit/importer.cpp
@@ -10885,17 +10885,16 @@ bool Compiler::impReturnInstruction(int prefixFlags, OPCODE& opcode)
 
 #ifdef SWIFT_SUPPORT
     // If this method has a SwiftError* out parameter, we need to set the error register upon returning.
-    // By placing the GT_SWIFT_ERROR_RET node before the GT_RETURN node,
-    // we don't have to teach the JIT that GT_SWIFT_ERROR_RET is a valid terminator for BBJ_RETURN blocks.
-    // During lowering, we will move the GT_SWIFT_ERROR_RET to the end of the block
-    // so the error register won't get trashed when returning the normal value.
+    // GT_SWIFT_ERROR_RET handles returning both the normal and error value,
+    // and will be the terminator node for this block.
     if (lvaSwiftErrorArg != BAD_VAR_NUM)
     {
         assert(info.compCallConv == CorInfoCallConvExtension::Swift);
         assert(lvaSwiftErrorLocal != BAD_VAR_NUM);
         GenTree* const swiftErrorNode = gtNewLclFldNode(lvaSwiftErrorLocal, TYP_I_IMPL, 0);
-        op1                           = new (this, GT_SWIFT_ERROR_RET)
-            GenTreeOp(GT_SWIFT_ERROR_RET, op1->TypeGet(), swiftErrorNode, op1->gtGetOp1());
+        op1->SetOperRaw(GT_SWIFT_ERROR_RET);
+        op1->AsOp()->gtOp2 = op1->AsOp()->gtOp1;
+        op1->AsOp()->gtOp1 = swiftErrorNode;
     }
 #endif // SWIFT_SUPPORT
 
diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp
index 1447c76269bf19..219835a427f621 100644
--- a/src/coreclr/jit/lower.cpp
+++ b/src/coreclr/jit/lower.cpp
@@ -4626,27 +4626,6 @@ void Lowering::LowerRet(GenTreeUnOp* ret)
     ContainCheckRet(ret);
 }
 
-//----------------------------------------------------------------------------------------------
-// LowerSwiftErrorRet: Lower swift error return node by moving it to the end of its BBJ_RETURN block.
-//                     We wait to do this late in the JIT's phases,
-//                     as we typically expect BBJ_RETURN blocks to end with a GT_RETURN/GT_RETFILT node.
-//
-// Arguments:
-//     swiftErrorRet - The GT_SWIFT_ERROR_RET node.
-//
-// Return Value:
-//     The next node to be lowered.
-//
-GenTree* Lowering::LowerSwiftErrorRet(GenTree* swiftErrorRet)
-{
-    assert(swiftErrorRet->OperIs(GT_SWIFT_ERROR_RET));
-    GenTree* const nextNode   = swiftErrorRet->gtNext;
-    LIR::Range&    blockRange = BlockRange();
-    blockRange.Remove(swiftErrorRet);
-    blockRange.InsertAtEnd(swiftErrorRet);
-    return nextNode;
-}
-
 //----------------------------------------------------------------------------------------------
 // LowerStoreLocCommon: platform independent part of local var or field store lowering.
 //
diff --git a/src/coreclr/jit/lower.h b/src/coreclr/jit/lower.h
index 1827bdf9e3e29e..0538c661e16260 100644
--- a/src/coreclr/jit/lower.h
+++ b/src/coreclr/jit/lower.h
@@ -155,7 +155,6 @@ class Lowering final : public Phase
     GenTreeCC* LowerNodeCC(GenTree* node, GenCondition condition);
     void       LowerJmpMethod(GenTree* jmp);
     void       LowerRet(GenTreeUnOp* ret);
-    GenTree*   LowerSwiftErrorRet(GenTree* ret);
     void       LowerStoreLocCommon(GenTreeLclVarCommon* lclVar);
     void       LowerRetStruct(GenTreeUnOp* ret);
     void       LowerRetSingleRegStructLclVar(GenTreeUnOp* ret);
diff --git a/src/coreclr/jit/lsraarm64.cpp b/src/coreclr/jit/lsraarm64.cpp
index a4935e0a331063..dfcebf4392c53c 100644
--- a/src/coreclr/jit/lsraarm64.cpp
+++ b/src/coreclr/jit/lsraarm64.cpp
@@ -743,11 +743,11 @@ int LinearScan::BuildNode(GenTree* tree)
 
 #ifdef SWIFT_SUPPORT
         case GT_SWIFT_ERROR_RET:
+            BuildUse(tree->gtGetOp1(), RBM_SWIFT_ERROR);
             // Plus one for error register
             srcCount = BuildReturn(tree) + 1;
             killMask = getKillSetForReturn();
             BuildDefsWithKills(tree, 0, RBM_NONE, killMask);
-            BuildUse(tree->gtGetOp1(), RBM_NONE);
             break;
 #endif // SWIFT_SUPPORT
 
diff --git a/src/coreclr/jit/lsraxarch.cpp b/src/coreclr/jit/lsraxarch.cpp
index b5a80deb7f76cf..6abc86aab0ed5e 100644
--- a/src/coreclr/jit/lsraxarch.cpp
+++ b/src/coreclr/jit/lsraxarch.cpp
@@ -195,11 +195,11 @@ int LinearScan::BuildNode(GenTree* tree)
 
 #ifdef SWIFT_SUPPORT
         case GT_SWIFT_ERROR_RET:
+            BuildUse(tree->gtGetOp1(), RBM_SWIFT_ERROR);
             // Plus one for error register
             srcCount = BuildReturn(tree) + 1;
             killMask = getKillSetForReturn();
             BuildDefsWithKills(tree, 0, RBM_NONE, killMask);
-            BuildUse(tree->gtGetOp1(), RBM_NONE);
             break;
 #endif // SWIFT_SUPPORT
 
diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp
index cd1850a78fbc44..db76355c31b9a8 100644
--- a/src/coreclr/jit/morph.cpp
+++ b/src/coreclr/jit/morph.cpp
@@ -14153,7 +14153,7 @@ void Compiler::fgMergeBlockReturn(BasicBlock* block)
 
             // Must be a void GT_RETURN/GT_SWIFT_ERROR_RET with null operand; delete it as this block branches to
             // oneReturn block
-            GenTree* const retVal = ret->OperIs(GT_RETURN) ? ret->gtGetOp1() : ret->gtGetOp2();
+            GenTree* const retVal = ret->AsOp()->GetReturnValue();
             noway_assert(ret->TypeGet() == TYP_VOID);
             noway_assert(retVal == nullptr);
 

From bf186b74842ed2a2de29c489518a0321e98c7be1 Mon Sep 17 00:00:00 2001
From: Aman Khalid <amankhalid@microsoft.com>
Date: Wed, 10 Apr 2024 16:13:16 +0000
Subject: [PATCH 13/28] Store error to local during return merging

---
 src/coreclr/jit/compiler.h    |  4 ++++
 src/coreclr/jit/fgbasic.cpp   |  4 ++++
 src/coreclr/jit/flowgraph.cpp | 15 ++++++++++++---
 src/coreclr/jit/gentree.h     | 15 +++++++++++++++
 src/coreclr/jit/lower.cpp     | 17 ++++++++++-------
 src/coreclr/jit/morph.cpp     | 28 +++++++++++++++++-----------
 6 files changed, 62 insertions(+), 21 deletions(-)

diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h
index a9ec6f595ab5a7..c5234657f9f244 100644
--- a/src/coreclr/jit/compiler.h
+++ b/src/coreclr/jit/compiler.h
@@ -8406,6 +8406,10 @@ class Compiler
     unsigned    genReturnLocal; // Local number for the return value when applicable.
     BasicBlock* genReturnBB;    // jumped to when not optimizing for speed.
 
+#ifdef SWIFT_SUPPORT
+    unsigned genReturnErrorLocal; // Local number for the Swift error value when applicable.
+#endif // SWIFT_SUPPORT
+
     // The following properties are part of CodeGenContext.  Getters are provided here for
     // convenience and backward compatibility, but the properties can only be set by invoking
     // the setter on CodeGenContext directly.
diff --git a/src/coreclr/jit/fgbasic.cpp b/src/coreclr/jit/fgbasic.cpp
index 37683b188c303b..7c2ad719fb0296 100644
--- a/src/coreclr/jit/fgbasic.cpp
+++ b/src/coreclr/jit/fgbasic.cpp
@@ -70,6 +70,10 @@ void Compiler::fgInit()
     genReturnBB    = nullptr;
     genReturnLocal = BAD_VAR_NUM;
 
+#ifdef SWIFT_SUPPORT
+    genReturnErrorLocal = BAD_VAR_NUM;
+#endif // SWIFT_SUPPORT
+
     /* We haven't reached the global morphing phase */
     fgGlobalMorph     = false;
     fgGlobalMorphDone = false;
diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp
index a5fae4ab7bacb5..0368001fcb9e6c 100644
--- a/src/coreclr/jit/flowgraph.cpp
+++ b/src/coreclr/jit/flowgraph.cpp
@@ -2027,13 +2027,22 @@ class MergedReturns
         }
 
 #ifdef SWIFT_SUPPORT
+        // If this method has a SwiftError* out parameter, we need to set the error register upon returning.
+        // GT_SWIFT_ERROR_RET handles returning both the normal and error value,
+        // and will be the terminator node for this block.
         if (comp->lvaSwiftErrorArg != BAD_VAR_NUM)
         {
             assert(comp->info.compCallConv == CorInfoCallConvExtension::Swift);
             assert(comp->lvaSwiftErrorLocal != BAD_VAR_NUM);
-            GenTree* const swiftErrorNode = comp->gtNewLclFldNode(comp->lvaSwiftErrorLocal, TYP_I_IMPL, 0);
-            returnExpr                    = new (comp, GT_SWIFT_ERROR_RET)
-                GenTreeOp(GT_SWIFT_ERROR_RET, returnExpr->TypeGet(), swiftErrorNode, returnExpr->gtGetOp1());
+            assert(comp->genReturnErrorLocal == BAD_VAR_NUM);
+
+            const unsigned errorLclNum = comp->lvaGrabTemp(true DEBUGARG("Single return block SwiftError value"));
+            comp->genReturnErrorLocal = errorLclNum;
+            comp->lvaGetDesc(errorLclNum)->lvType = genActualType(TYP_I_IMPL);
+
+            returnExpr->SetOperRaw(GT_SWIFT_ERROR_RET);
+            returnExpr->AsOp()->gtOp2 = returnExpr->AsOp()->gtOp1;
+            returnExpr->AsOp()->gtOp1 = comp->gtNewLclvNode(errorLclNum, TYP_I_IMPL);
         }
 #endif // SWIFT_SUPPORT
 
diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h
index 9da96ec025390f..152499a032b272 100644
--- a/src/coreclr/jit/gentree.h
+++ b/src/coreclr/jit/gentree.h
@@ -3053,6 +3053,21 @@ struct GenTreeOp : public GenTreeUnOp
         return gtOp1;
     }
 
+    void SetReturnValue(GenTree* const retVal)
+    {
+        assert(OperIs(GT_RETURN, GT_RETFILT, GT_SWIFT_ERROR_RET));
+#ifdef SWIFT_SUPPORT
+        if (OperIs(GT_SWIFT_ERROR_RET))
+        {
+            gtOp2 = retVal;
+        }
+        else
+#endif // SWIFT_SUPPORT
+        {
+            gtOp1 = retVal;
+        }
+    }
+
 #if !defined(TARGET_64BIT) || defined(TARGET_ARM64)
     bool IsValidLongMul();
 #endif
diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp
index 219835a427f621..2e74e1069b6777 100644
--- a/src/coreclr/jit/lower.cpp
+++ b/src/coreclr/jit/lower.cpp
@@ -520,8 +520,11 @@ GenTree* Lowering::LowerNode(GenTree* node)
             LowerJmpMethod(node);
             break;
 
-        case GT_RETURN:
         case GT_SWIFT_ERROR_RET:
+            LowerNode(node->gtGetOp1());
+
+            FALLTHROUGH;
+        case GT_RETURN:
             LowerRet(node->AsUnOp());
             break;
 
@@ -4547,7 +4550,7 @@ void Lowering::LowerRet(GenTreeUnOp* ret)
 {
     assert(ret->OperIs(GT_RETURN, GT_SWIFT_ERROR_RET));
 
-    JITDUMP("lowering GT_RETURN\n");
+    JITDUMP("lowering return node\n");
     DISPNODE(ret);
     JITDUMP("============\n");
 
@@ -4573,7 +4576,7 @@ void Lowering::LowerRet(GenTreeUnOp* ret)
 #endif
 
         GenTree* bitcast = comp->gtNewBitCastNode(ret->TypeGet(), retVal);
-        ret->gtOp1       = bitcast;
+        ret->AsOp()->SetReturnValue(bitcast);
         BlockRange().InsertBefore(ret, bitcast);
         ContainCheckBitCast(bitcast);
     }
@@ -4965,7 +4968,7 @@ void Lowering::LowerRetSingleRegStructLclVar(GenTreeUnOp* ret)
 {
     assert(!comp->compMethodReturnsMultiRegRetType());
     assert(ret->OperIs(GT_RETURN, GT_SWIFT_ERROR_RET));
-    GenTreeLclVarCommon* lclVar = ret->gtGetOp1()->AsLclVar();
+    GenTreeLclVarCommon* lclVar = ret->AsOp()->GetReturnValue()->AsLclVar();
     assert(lclVar->OperIs(GT_LCL_VAR));
     unsigned   lclNum = lclVar->GetLclNum();
     LclVarDsc* varDsc = comp->lvaGetDesc(lclNum);
@@ -5013,7 +5016,7 @@ void Lowering::LowerRetSingleRegStructLclVar(GenTreeUnOp* ret)
         if (!varTypeUsesSameRegType(ret, lclVarType))
         {
             GenTree* bitcast = comp->gtNewBitCastNode(ret->TypeGet(), ret->gtOp1);
-            ret->gtOp1       = bitcast;
+            ret->AsOp()->SetReturnValue(bitcast);
             BlockRange().InsertBefore(ret, bitcast);
             ContainCheckBitCast(bitcast);
         }
@@ -8153,7 +8156,7 @@ void Lowering::ContainCheckRet(GenTreeUnOp* ret)
 #if !defined(TARGET_64BIT)
     if (ret->TypeGet() == TYP_LONG)
     {
-        GenTree* op1 = ret->gtGetOp1();
+        GenTree* op1 = ret->AsOp()->GetReturnValue();
         noway_assert(op1->OperGet() == GT_LONG);
         MakeSrcContained(ret, op1);
     }
@@ -8161,7 +8164,7 @@ void Lowering::ContainCheckRet(GenTreeUnOp* ret)
 #if FEATURE_MULTIREG_RET
     if (ret->TypeIs(TYP_STRUCT))
     {
-        GenTree* op1 = ret->gtGetOp1();
+        GenTree* op1 = ret->AsOp()->GetReturnValue();
         // op1 must be either a lclvar or a multi-reg returning call
         if (op1->OperGet() == GT_LCL_VAR)
         {
diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp
index db76355c31b9a8..01dd4705504894 100644
--- a/src/coreclr/jit/morph.cpp
+++ b/src/coreclr/jit/morph.cpp
@@ -14092,6 +14092,10 @@ void Compiler::fgMergeBlockReturn(BasicBlock* block)
             fgReturnCount--;
         }
 
+#ifdef SWIFT_SUPPORT
+        GenTree* const errorVal = ((ret != nullptr) && ret->OperIs(GT_SWIFT_ERROR_RET)) ? ret->gtGetOp1() : nullptr;
+#endif // SWIFT_SUPPORT
+
         if (genReturnLocal != BAD_VAR_NUM)
         {
             // replace the GT_RETURN/GT_SWIFT_ERROR_RET node to be a STORE_LCL_VAR that stores the return value into
@@ -14107,17 +14111,7 @@ void Compiler::fgMergeBlockReturn(BasicBlock* block)
 
             // GT_RETURN must have non-null operand as the method is returning the value assigned to
             // genReturnLocal
-            GenTree* retVal;
-            if (ret->OperIs(GT_SWIFT_ERROR_RET))
-            {
-                retVal = ret->gtGetOp2();
-            }
-            else
-            {
-                noway_assert(ret->OperIs(GT_RETURN));
-                retVal = ret->gtGetOp1();
-            }
-
+            GenTree* const retVal = ret->AsOp()->GetReturnValue();
             noway_assert(retVal != nullptr);
 
             Statement*       pAfterStatement = lastStmt;
@@ -14168,6 +14162,18 @@ void Compiler::fgMergeBlockReturn(BasicBlock* block)
             }
         }
 
+#ifdef SWIFT_SUPPORT
+        // If merging GT_SWIFT_ERROR_RET nodes, ensure the error operand is stored to the merged return error local,
+        // so the correct error value is retrieved in the merged return block.
+        if (errorVal != nullptr)
+        {
+            assert(genReturnErrorLocal != BAD_VAR_NUM);
+            Statement* insertAfterStmt = block->lastStmt();
+            GenTree* swiftErrorStore = gtNewTempStore(genReturnErrorLocal, errorVal, CHECK_SPILL_NONE, &insertAfterStmt, insertAfterStmt->GetDebugInfo(), block);
+            fgNewStmtAtEnd(block, swiftErrorStore);
+        }
+#endif // SWIFT_SUPPORT
+
         JITDUMP("\nUpdate " FMT_BB " to jump to common return block.\n", block->bbNum);
         DISPBLOCK(block);
 

From c381c485a05486547c51484c34de79ca02489ec5 Mon Sep 17 00:00:00 2001
From: Aman Khalid <amankhalid@microsoft.com>
Date: Wed, 10 Apr 2024 16:37:18 +0000
Subject: [PATCH 14/28] Handle GT_SWIFT_ERROR_RET in morph

---
 src/coreclr/jit/compiler.h |  2 +-
 src/coreclr/jit/morph.cpp  | 58 +++++++++++++++++++++++++-------------
 2 files changed, 39 insertions(+), 21 deletions(-)

diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h
index c5234657f9f244..bda57faab89fe0 100644
--- a/src/coreclr/jit/compiler.h
+++ b/src/coreclr/jit/compiler.h
@@ -6523,7 +6523,7 @@ class Compiler
     GenTree* fgOptimizeBitwiseAnd(GenTreeOp* andOp);
     GenTree* fgOptimizeBitwiseXor(GenTreeOp* xorOp);
     GenTree* fgPropagateCommaThrow(GenTree* parent, GenTreeOp* commaThrow, GenTreeFlags precedingSideEffects);
-    GenTree* fgMorphRetInd(GenTreeUnOp* tree);
+    GenTree* fgMorphRetInd(GenTreeOp* tree);
     GenTree* fgMorphModToZero(GenTreeOp* tree);
     GenTree* fgMorphModToSubMulDiv(GenTreeOp* tree);
     GenTree* fgMorphUModToAndSub(GenTreeOp* tree);
diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp
index 01dd4705504894..28f5d3816cf89e 100644
--- a/src/coreclr/jit/morph.cpp
+++ b/src/coreclr/jit/morph.cpp
@@ -8737,34 +8737,49 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA
             return fgMorphIntoHelperCall(tree, helper, true /* morphArgs */, op1, op2);
 
         case GT_RETURN:
+        case GT_SWIFT_ERROR_RET:
+        {
+            GenTree* retVal = tree->AsOp()->GetReturnValue();
+
             if (!tree->TypeIs(TYP_VOID))
             {
-                if (op1->OperIs(GT_LCL_FLD))
+                if (retVal->OperIs(GT_LCL_FLD))
                 {
-                    op1 = fgMorphRetInd(tree->AsUnOp());
+                    retVal = fgMorphRetInd(tree->AsOp());
                 }
 
-                fgTryReplaceStructLocalWithField(op1);
+                fgTryReplaceStructLocalWithField(retVal);
             }
 
             // normalize small integer return values
-            if (fgGlobalMorph && varTypeIsSmall(info.compRetType) && (op1 != nullptr) && !op1->TypeIs(TYP_VOID) &&
-                fgCastNeeded(op1, info.compRetType))
+            if (fgGlobalMorph && varTypeIsSmall(info.compRetType) && (retVal != nullptr) && !retVal->TypeIs(TYP_VOID) &&
+                fgCastNeeded(retVal, info.compRetType))
             {
                 // Small-typed return values are normalized by the callee
-                op1 = gtNewCastNode(TYP_INT, op1, false, info.compRetType);
+                retVal = gtNewCastNode(TYP_INT, retVal, false, info.compRetType);
 
                 // Propagate GTF_COLON_COND
-                op1->gtFlags |= (tree->gtFlags & GTF_COLON_COND);
+                retVal->gtFlags |= (tree->gtFlags & GTF_COLON_COND);
 
-                tree->AsOp()->gtOp1 = fgMorphTree(op1);
+                retVal = fgMorphTree(retVal);
+                tree->AsOp()->SetReturnValue(retVal);
 
                 // Propagate side effect flags
-                tree->SetAllEffectsFlags(tree->AsOp()->gtGetOp1());
+                tree->SetAllEffectsFlags(retVal);
 
                 return tree;
             }
+
+            if (tree->OperIs(GT_RETURN))
+            {
+                op1 = retVal;
+            }
+            else
+            {
+                op2 = retVal;
+            }
             break;
+        }
 
         case GT_EQ:
         case GT_NE:
@@ -9631,15 +9646,18 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA
             break;
 
         case GT_RETURN:
-
-            // Retry updating op1 to a field -- assertion
-            // prop done when morphing op1 changed the local.
+        case GT_SWIFT_ERROR_RET:
+        {
+            // Retry updating return operand to a field -- assertion
+            // prop done when morphing this operand changed the local.
             //
-            if (op1 != nullptr)
+            GenTree* const retVal = tree->AsOp()->GetReturnValue();
+            if (retVal != nullptr)
             {
-                fgTryReplaceStructLocalWithField(op1);
+                fgTryReplaceStructLocalWithField(retVal);
             }
             break;
+        }
 
         default:
             break;
@@ -11538,16 +11556,16 @@ GenTree* Compiler::fgPropagateCommaThrow(GenTree* parent, GenTreeOp* commaThrow,
 // fgMorphRetInd: Try to get rid of extra local indirections in a return tree.
 //
 // Arguments:
-//    node - The return node that uses an local field.
+//    node - The return node that uses a local field.
 //
 // Return Value:
-//    the original op1 of the ret if there was no optimization or an optimized new op1.
+//    the original return operand if there was no optimization, or an optimized new return operand.
 //
-GenTree* Compiler::fgMorphRetInd(GenTreeUnOp* ret)
+GenTree* Compiler::fgMorphRetInd(GenTreeOp* ret)
 {
-    assert(ret->OperIs(GT_RETURN));
-    assert(ret->gtGetOp1()->OperIs(GT_LCL_FLD));
-    GenTreeLclFld* lclFld = ret->gtGetOp1()->AsLclFld();
+    assert(ret->OperIs(GT_RETURN, GT_SWIFT_ERROR_RET));
+    assert(ret->GetReturnValue()->OperIs(GT_LCL_FLD));
+    GenTreeLclFld* lclFld = ret->GetReturnValue()->AsLclFld();
     unsigned       lclNum = lclFld->GetLclNum();
 
     if (fgGlobalMorph && varTypeIsStruct(lclFld) && !lvaIsImplicitByRefLocal(lclNum))

From 70a69d7bb20c3cb0eac12590e0ce4304bc8bd919 Mon Sep 17 00:00:00 2001
From: Aman Khalid <amankhalid@microsoft.com>
Date: Wed, 10 Apr 2024 17:03:56 +0000
Subject: [PATCH 15/28] Move GT_SWIFT_ERROR_RET creation to phase

---
 src/coreclr/jit/compiler.cpp  |  6 ++++
 src/coreclr/jit/compiler.h    |  4 +++
 src/coreclr/jit/compphases.h  |  3 ++
 src/coreclr/jit/flowgraph.cpp | 61 +++++++++++++++++++++++------------
 src/coreclr/jit/importer.cpp  | 15 ---------
 5 files changed, 54 insertions(+), 35 deletions(-)

diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp
index 1edc4294140189..e602a40ae05b44 100644
--- a/src/coreclr/jit/compiler.cpp
+++ b/src/coreclr/jit/compiler.cpp
@@ -4717,6 +4717,12 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
     //
     DoPhase(this, PHASE_MORPH_ADD_INTERNAL, &Compiler::fgAddInternal);
 
+#ifdef SWIFT_SUPPORT
+    // Transform GT_RETURN nodes into GT_SWIFT_ERROR_RET nodes if this method has Swift error handling
+    //
+    DoPhase(this, PHASE_SWIFT_ERROR_RET, &Compiler::fgAddSwiftErrorReturns);
+#endif // SWIFT_SUPPORT
+
     // Remove empty try regions
     //
     DoPhase(this, PHASE_EMPTY_TRY, &Compiler::fgRemoveEmptyTry);
diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h
index bda57faab89fe0..80eeaf18f5e77a 100644
--- a/src/coreclr/jit/compiler.h
+++ b/src/coreclr/jit/compiler.h
@@ -5279,6 +5279,10 @@ class Compiler
 
     PhaseStatus fgAddInternal();
 
+#ifdef SWIFT_SUPPORT
+    PhaseStatus fgAddSwiftErrorReturns();
+#endif // SWIFT_SUPPORT
+
     enum class FoldResult
     {
         FOLD_DID_NOTHING,
diff --git a/src/coreclr/jit/compphases.h b/src/coreclr/jit/compphases.h
index 4bd236ad7f1962..6ecf8a70a64852 100644
--- a/src/coreclr/jit/compphases.h
+++ b/src/coreclr/jit/compphases.h
@@ -34,6 +34,9 @@ CompPhaseNameMacro(PHASE_INCPROFILE,                 "Profile incorporation",
 CompPhaseNameMacro(PHASE_MORPH_INIT,                 "Morph - Init",                   false, -1, false)
 CompPhaseNameMacro(PHASE_MORPH_INLINE,               "Morph - Inlining",               false, -1, true)
 CompPhaseNameMacro(PHASE_MORPH_ADD_INTERNAL,         "Morph - Add internal blocks",    false, -1, true)
+#ifdef SWIFT_SUPPORT
+CompPhaseNameMacro(PHASE_SWIFT_ERROR_RET,            "Add Swift error returns",        false, -1, true)
+#endif // SWIFT_SUPPORT
 CompPhaseNameMacro(PHASE_ALLOCATE_OBJECTS,           "Allocate Objects",               false, -1, false)
 CompPhaseNameMacro(PHASE_EMPTY_TRY,                  "Remove empty try",               false, -1, false)
 CompPhaseNameMacro(PHASE_EMPTY_FINALLY,              "Remove empty finally",           false, -1, false)
diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp
index 0368001fcb9e6c..18471ce62c6c27 100644
--- a/src/coreclr/jit/flowgraph.cpp
+++ b/src/coreclr/jit/flowgraph.cpp
@@ -2026,26 +2026,6 @@ class MergedReturns
             returnExpr = new (comp, GT_RETURN) GenTreeOp(GT_RETURN, TYP_VOID);
         }
 
-#ifdef SWIFT_SUPPORT
-        // If this method has a SwiftError* out parameter, we need to set the error register upon returning.
-        // GT_SWIFT_ERROR_RET handles returning both the normal and error value,
-        // and will be the terminator node for this block.
-        if (comp->lvaSwiftErrorArg != BAD_VAR_NUM)
-        {
-            assert(comp->info.compCallConv == CorInfoCallConvExtension::Swift);
-            assert(comp->lvaSwiftErrorLocal != BAD_VAR_NUM);
-            assert(comp->genReturnErrorLocal == BAD_VAR_NUM);
-
-            const unsigned errorLclNum = comp->lvaGrabTemp(true DEBUGARG("Single return block SwiftError value"));
-            comp->genReturnErrorLocal = errorLclNum;
-            comp->lvaGetDesc(errorLclNum)->lvType = genActualType(TYP_I_IMPL);
-
-            returnExpr->SetOperRaw(GT_SWIFT_ERROR_RET);
-            returnExpr->AsOp()->gtOp2 = returnExpr->AsOp()->gtOp1;
-            returnExpr->AsOp()->gtOp1 = comp->gtNewLclvNode(errorLclNum, TYP_I_IMPL);
-        }
-#endif // SWIFT_SUPPORT
-
         // Add 'return' expression to the return block
         comp->fgNewStmtAtEnd(newReturnBB, returnExpr);
         // Flag that this 'return' was generated by return merging so that subsequent
@@ -2579,6 +2559,47 @@ PhaseStatus Compiler::fgAddInternal()
     return madeChanges ? PhaseStatus::MODIFIED_EVERYTHING : PhaseStatus::MODIFIED_NOTHING;
 }
 
+#ifdef SWIFT_SUPPORT
+PhaseStatus Compiler::fgAddSwiftErrorReturns()
+{
+    if (lvaSwiftErrorArg == BAD_VAR_NUM)
+    {
+        // No Swift error handling in this method
+        return PhaseStatus::MODIFIED_NOTHING;
+    }
+
+    assert(lvaSwiftErrorLocal != BAD_VAR_NUM);
+    assert(info.compCallConv == CorInfoCallConvExtension::Swift);
+
+    for (BasicBlock* block : Blocks())
+    {
+        if (block->KindIs(BBJ_RETURN))
+        {
+            GenTree* const ret = block->lastNode();
+            assert(ret->OperIs(GT_RETURN));
+            ret->SetOperRaw(GT_SWIFT_ERROR_RET);
+            ret->AsOp()->gtOp2 = ret->AsOp()->gtOp1;
+
+            // If this is the merged return block, use the merged return error local as the error operand.
+            // Else, load the error value from the SwiftError pseudolocal (this will probably get promoted, anyway).
+            if (block == genReturnBB)
+            {
+                assert(genReturnErrorLocal == BAD_VAR_NUM);
+                genReturnErrorLocal = lvaGrabTemp(true DEBUGARG("Single return block SwiftError value"));
+                lvaGetDesc(genReturnErrorLocal)->lvType = genActualType(TYP_I_IMPL);
+                ret->AsOp()->gtOp1 = gtNewLclvNode(genReturnErrorLocal, TYP_I_IMPL);
+            }
+            else
+            {
+                ret->AsOp()->gtOp1 = gtNewLclFldNode(lvaSwiftErrorLocal, TYP_I_IMPL, 0);
+            }
+        }
+    }
+
+    return PhaseStatus::MODIFIED_EVERYTHING;
+}
+#endif // SWIFT_SUPPORT
+
 /*****************************************************************************/
 /*****************************************************************************/
 
diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp
index d9b48c7e8b55ad..12a612fde1a66a 100644
--- a/src/coreclr/jit/importer.cpp
+++ b/src/coreclr/jit/importer.cpp
@@ -10883,21 +10883,6 @@ bool Compiler::impReturnInstruction(int prefixFlags, OPCODE& opcode)
         }
     }
 
-#ifdef SWIFT_SUPPORT
-    // If this method has a SwiftError* out parameter, we need to set the error register upon returning.
-    // GT_SWIFT_ERROR_RET handles returning both the normal and error value,
-    // and will be the terminator node for this block.
-    if (lvaSwiftErrorArg != BAD_VAR_NUM)
-    {
-        assert(info.compCallConv == CorInfoCallConvExtension::Swift);
-        assert(lvaSwiftErrorLocal != BAD_VAR_NUM);
-        GenTree* const swiftErrorNode = gtNewLclFldNode(lvaSwiftErrorLocal, TYP_I_IMPL, 0);
-        op1->SetOperRaw(GT_SWIFT_ERROR_RET);
-        op1->AsOp()->gtOp2 = op1->AsOp()->gtOp1;
-        op1->AsOp()->gtOp1 = swiftErrorNode;
-    }
-#endif // SWIFT_SUPPORT
-
     impAppendTree(op1, CHECK_SPILL_NONE, impCurStmtDI);
 #ifdef DEBUG
     // Remember at which BC offset the tree was finished

From de4457b46e55771c3b3b04edf972143b0c0a9732 Mon Sep 17 00:00:00 2001
From: Aman Khalid <amankhalid@microsoft.com>
Date: Wed, 10 Apr 2024 18:25:33 +0000
Subject: [PATCH 16/28] Handle remaining GT_RETURN-specific code

---
 src/coreclr/jit/assertionprop.cpp  |  9 +--
 src/coreclr/jit/codegenlinear.cpp  |  4 +-
 src/coreclr/jit/compiler.h         |  2 +-
 src/coreclr/jit/decomposelongs.cpp |  3 +-
 src/coreclr/jit/flowgraph.cpp      |  2 +-
 src/coreclr/jit/forwardsub.cpp     |  2 +-
 src/coreclr/jit/ifconversion.cpp   | 30 +++++-----
 src/coreclr/jit/lir.cpp            |  5 +-
 src/coreclr/jit/morph.cpp          | 10 ++--
 src/coreclr/jit/optimizebools.cpp  | 91 ++++++++++++++++++++----------
 src/coreclr/jit/promotion.cpp      |  6 +-
 11 files changed, 98 insertions(+), 66 deletions(-)

diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp
index ebcb101663a308..0fa846632a63cd 100644
--- a/src/coreclr/jit/assertionprop.cpp
+++ b/src/coreclr/jit/assertionprop.cpp
@@ -4109,7 +4109,7 @@ GenTree* Compiler::optAssertionProp_ModDiv(ASSERT_VALARG_TP assertions, GenTreeO
 }
 
 //------------------------------------------------------------------------
-// optAssertionProp_Return: Try and optimize a GT_RETURN via assertions.
+// optAssertionProp_Return: Try and optimize a GT_RETURN/GT_SWIFT_ERROR_RET via assertions.
 //
 // Propagates ZEROOBJ for the return value.
 //
@@ -4124,9 +4124,9 @@ GenTree* Compiler::optAssertionProp_ModDiv(ASSERT_VALARG_TP assertions, GenTreeO
 // Notes:
 //   stmt may be nullptr during local assertion prop
 //
-GenTree* Compiler::optAssertionProp_Return(ASSERT_VALARG_TP assertions, GenTreeUnOp* ret, Statement* stmt)
+GenTree* Compiler::optAssertionProp_Return(ASSERT_VALARG_TP assertions, GenTreeOp* ret, Statement* stmt)
 {
-    GenTree* retValue = ret->gtGetOp1();
+    GenTree* retValue = ret->GetReturnValue();
 
     // Only propagate zeroes that lowering can deal with.
     if (!ret->TypeIs(TYP_VOID) && varTypeIsStruct(retValue) && !varTypeIsStruct(info.compRetNativeType))
@@ -5512,7 +5512,8 @@ GenTree* Compiler::optAssertionProp(ASSERT_VALARG_TP assertions, GenTree* tree,
             return optAssertionProp_BlockStore(assertions, tree->AsBlk(), stmt);
 
         case GT_RETURN:
-            return optAssertionProp_Return(assertions, tree->AsUnOp(), stmt);
+        case GT_SWIFT_ERROR_RET:
+            return optAssertionProp_Return(assertions, tree->AsOp(), stmt);
 
         case GT_MOD:
         case GT_DIV:
diff --git a/src/coreclr/jit/codegenlinear.cpp b/src/coreclr/jit/codegenlinear.cpp
index 351ca14942838b..c00f83580e82af 100644
--- a/src/coreclr/jit/codegenlinear.cpp
+++ b/src/coreclr/jit/codegenlinear.cpp
@@ -498,9 +498,9 @@ void CodeGen::genCodeForBBlist()
         // as the determiner because something we are tracking as a byref
         // might be used as a return value of a int function (which is legal)
         GenTree* blockLastNode = block->lastNode();
-        if ((blockLastNode != nullptr) && (blockLastNode->gtOper == GT_RETURN) &&
+        if ((blockLastNode != nullptr) && (blockLastNode->OperIs(GT_RETURN, GT_SWIFT_ERROR_RET)) &&
             (varTypeIsGC(compiler->info.compRetType) ||
-             (blockLastNode->AsOp()->gtOp1 != nullptr && varTypeIsGC(blockLastNode->AsOp()->gtOp1->TypeGet()))))
+             (blockLastNode->AsOp()->GetReturnValue() != nullptr && varTypeIsGC(blockLastNode->AsOp()->GetReturnValue()->TypeGet()))))
         {
             nonVarPtrRegs &= ~RBM_INTRET;
         }
diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h
index 80eeaf18f5e77a..00e8edc20861ae 100644
--- a/src/coreclr/jit/compiler.h
+++ b/src/coreclr/jit/compiler.h
@@ -7882,7 +7882,7 @@ class Compiler
     GenTree* optAssertionProp_LocalStore(ASSERT_VALARG_TP assertions, GenTreeLclVarCommon* store, Statement* stmt);
     GenTree* optAssertionProp_BlockStore(ASSERT_VALARG_TP assertions, GenTreeBlk* store, Statement* stmt);
     GenTree* optAssertionProp_ModDiv(ASSERT_VALARG_TP assertions, GenTreeOp* tree, Statement* stmt);
-    GenTree* optAssertionProp_Return(ASSERT_VALARG_TP assertions, GenTreeUnOp* ret, Statement* stmt);
+    GenTree* optAssertionProp_Return(ASSERT_VALARG_TP assertions, GenTreeOp* ret, Statement* stmt);
     GenTree* optAssertionProp_Ind(ASSERT_VALARG_TP assertions, GenTree* tree, Statement* stmt);
     GenTree* optAssertionProp_Cast(ASSERT_VALARG_TP assertions, GenTreeCast* cast, Statement* stmt);
     GenTree* optAssertionProp_Call(ASSERT_VALARG_TP assertions, GenTreeCall* call, Statement* stmt);
diff --git a/src/coreclr/jit/decomposelongs.cpp b/src/coreclr/jit/decomposelongs.cpp
index ea87a996dbb1aa..84802400feeb0a 100644
--- a/src/coreclr/jit/decomposelongs.cpp
+++ b/src/coreclr/jit/decomposelongs.cpp
@@ -188,7 +188,8 @@ GenTree* DecomposeLongs::DecomposeNode(GenTree* tree)
             break;
 
         case GT_RETURN:
-            assert(tree->AsOp()->gtOp1->OperGet() == GT_LONG);
+        case GT_SWIFT_ERROR_RET:
+            assert(tree->AsOp()->GetReturnValue()->OperIs(GT_LONG));
             break;
 
         case GT_STOREIND:
diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp
index 18471ce62c6c27..464ad5131bc5dc 100644
--- a/src/coreclr/jit/flowgraph.cpp
+++ b/src/coreclr/jit/flowgraph.cpp
@@ -1586,7 +1586,7 @@ GenTree* Compiler::fgCreateMonitorTree(unsigned lvaMonAcquired, unsigned lvaThis
     }
 #endif
 
-    if (block->KindIs(BBJ_RETURN) && block->lastStmt()->GetRootNode()->gtOper == GT_RETURN)
+    if (block->KindIs(BBJ_RETURN) && block->lastStmt()->GetRootNode()->OperIs(GT_RETURN))
     {
         GenTreeUnOp* retNode = block->lastStmt()->GetRootNode()->AsUnOp();
         GenTree*     retExpr = retNode->gtOp1;
diff --git a/src/coreclr/jit/forwardsub.cpp b/src/coreclr/jit/forwardsub.cpp
index de4ac5fe8a4758..9d57fa3a4a6dff 100644
--- a/src/coreclr/jit/forwardsub.cpp
+++ b/src/coreclr/jit/forwardsub.cpp
@@ -798,7 +798,7 @@ bool Compiler::fgForwardSubStatement(Statement* stmt)
     // interaction between decomposition and RA.
     //
     if (compMethodReturnsMultiRegRetType() && (fsv.GetParentNode() != nullptr) &&
-        fsv.GetParentNode()->OperIs(GT_RETURN))
+        fsv.GetParentNode()->OperIs(GT_RETURN, GT_SWIFT_ERROR_RET))
     {
 #if defined(TARGET_X86)
         if (fwdSubNode->TypeGet() == TYP_LONG)
diff --git a/src/coreclr/jit/ifconversion.cpp b/src/coreclr/jit/ifconversion.cpp
index 6d7ead881a91c5..290ab35ba57070 100644
--- a/src/coreclr/jit/ifconversion.cpp
+++ b/src/coreclr/jit/ifconversion.cpp
@@ -140,7 +140,8 @@ bool OptIfConversionDsc::IfConvertCheckThenFlow()
             if (thenBlock->KindIs(BBJ_RETURN))
             {
                 assert(m_finalBlock == nullptr);
-                m_mainOper = GT_RETURN;
+                m_mainOper = thenBlock->lastNode()->OperGet();
+                assert((m_mainOper == GT_RETURN) || (m_mainOper == GT_SWIFT_ERROR_RET));
             }
             else
             {
@@ -231,7 +232,7 @@ bool OptIfConversionDsc::IfConvertCheckStmts(BasicBlock* fromBlock, IfConvertOpe
         for (Statement* const stmt : block->Statements())
         {
             GenTree* tree = stmt->GetRootNode();
-            switch (tree->gtOper)
+            switch (tree->OperGet())
             {
                 case GT_STORE_LCL_VAR:
                 {
@@ -286,8 +287,9 @@ bool OptIfConversionDsc::IfConvertCheckStmts(BasicBlock* fromBlock, IfConvertOpe
                 }
 
                 case GT_RETURN:
+                case GT_SWIFT_ERROR_RET:
                 {
-                    GenTree* op1 = tree->gtGetOp1();
+                    GenTree* const retVal = tree->AsOp()->GetReturnValue();
 
                     // Only allow RETURNs if else conversion is being used.
                     if (!m_doElseConversion)
@@ -296,7 +298,7 @@ bool OptIfConversionDsc::IfConvertCheckStmts(BasicBlock* fromBlock, IfConvertOpe
                     }
 
                     // Only one per operation per block can be conditionally executed.
-                    if (found || op1 == nullptr)
+                    if (found || retVal == nullptr)
                     {
                         return false;
                     }
@@ -317,7 +319,7 @@ bool OptIfConversionDsc::IfConvertCheckStmts(BasicBlock* fromBlock, IfConvertOpe
 #endif
 
                     // Ensure it won't cause any additional side effects.
-                    if ((op1->gtFlags & (GTF_SIDE_EFFECT | GTF_ORDER_SIDEEFF)) != 0)
+                    if ((retVal->gtFlags & (GTF_SIDE_EFFECT | GTF_ORDER_SIDEEFF)) != 0)
                     {
                         return false;
                     }
@@ -326,7 +328,7 @@ bool OptIfConversionDsc::IfConvertCheckStmts(BasicBlock* fromBlock, IfConvertOpe
                     // with the condition (for example, the condition could be an explicit bounds
                     // check and the operand could read an array element). Disallow this except
                     // for some common cases that we know are always side effect free.
-                    if (((m_cond->gtFlags & GTF_ORDER_SIDEEFF) != 0) && !op1->IsInvariant() && !op1->OperIsLocal())
+                    if (((m_cond->gtFlags & GTF_ORDER_SIDEEFF) != 0) && !retVal->IsInvariant() && !retVal->OperIsLocal())
                     {
                         return false;
                     }
@@ -575,7 +577,7 @@ bool OptIfConversionDsc::optIfConvert()
     {
         return false;
     }
-    assert(m_thenOperation.node->OperIs(GT_STORE_LCL_VAR, GT_RETURN));
+    assert(m_thenOperation.node->OperIs(GT_STORE_LCL_VAR, GT_RETURN, GT_SWIFT_ERROR_RET));
     if (m_doElseConversion)
     {
         if (!IfConvertCheckStmts(m_startBlock->GetTrueTarget(), &m_elseOperation))
@@ -633,11 +635,11 @@ bool OptIfConversionDsc::optIfConvert()
         }
         else
         {
-            assert(m_mainOper == GT_RETURN);
-            thenCost = m_thenOperation.node->gtGetOp1()->GetCostEx();
+            assert((m_mainOper == GT_RETURN) || (m_mainOper == GT_SWIFT_ERROR_RET));
+            thenCost = m_thenOperation.node->AsOp()->GetReturnValue()->GetCostEx();
             if (m_doElseConversion)
             {
-                elseCost = m_elseOperation.node->gtGetOp1()->GetCostEx();
+                elseCost = m_elseOperation.node->AsOp()->GetReturnValue()->GetCostEx();
             }
         }
 
@@ -693,12 +695,12 @@ bool OptIfConversionDsc::optIfConvert()
     }
     else
     {
-        assert(m_mainOper == GT_RETURN);
+        assert((m_mainOper == GT_RETURN) || (m_mainOper == GT_SWIFT_ERROR_RET));
         assert(m_doElseConversion);
         assert(m_thenOperation.node->TypeGet() == m_elseOperation.node->TypeGet());
 
-        selectTrueInput  = m_elseOperation.node->gtGetOp1();
-        selectFalseInput = m_thenOperation.node->gtGetOp1();
+        selectTrueInput  = m_elseOperation.node->AsOp()->GetReturnValue();
+        selectFalseInput = m_thenOperation.node->AsOp()->GetReturnValue();
         selectType       = genActualType(m_thenOperation.node);
     }
 
@@ -714,7 +716,7 @@ bool OptIfConversionDsc::optIfConvert()
     }
     else
     {
-        m_thenOperation.node->AsOp()->gtOp1 = select;
+        m_thenOperation.node->AsOp()->SetReturnValue(select);
     }
     m_comp->gtSetEvalOrder(m_thenOperation.node);
     m_comp->fgSetStmtSeq(m_thenOperation.stmt);
diff --git a/src/coreclr/jit/lir.cpp b/src/coreclr/jit/lir.cpp
index d172cea22d369a..feabec03d02cad 100644
--- a/src/coreclr/jit/lir.cpp
+++ b/src/coreclr/jit/lir.cpp
@@ -1792,12 +1792,11 @@ void LIR::InsertBeforeTerminator(BasicBlock* block, LIR::Range&& range)
                 break;
 
             case BBJ_SWITCH:
-                assert((insertionPoint->OperGet() == GT_SWITCH) || (insertionPoint->OperGet() == GT_SWITCH_TABLE));
+                assert(insertionPoint->OperIs(GT_SWITCH, GT_SWITCH_TABLE));
                 break;
 
             case BBJ_RETURN:
-                assert((insertionPoint->OperGet() == GT_RETURN) || (insertionPoint->OperGet() == GT_JMP) ||
-                       (insertionPoint->OperGet() == GT_CALL));
+                assert(insertionPoint->OperIs(GT_RETURN, GT_SWIFT_ERROR_RET, GT_JMP, GT_CALL));
                 break;
 
             default:
diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp
index 28f5d3816cf89e..03ee2cd4355996 100644
--- a/src/coreclr/jit/morph.cpp
+++ b/src/coreclr/jit/morph.cpp
@@ -9712,7 +9712,7 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA
 //    tree - tree to examine and possibly modify
 //
 // Notes:
-//    Currently only called when the tree parent is a GT_RETURN.
+//    Currently only called when the tree parent is a GT_RETURN/GT_SWIFT_ERROR_RET.
 //
 void Compiler::fgTryReplaceStructLocalWithField(GenTree* tree)
 {
@@ -14122,12 +14122,12 @@ void Compiler::fgMergeBlockReturn(BasicBlock* block)
             // Method must be returning a value other than TYP_VOID.
             noway_assert(compMethodHasRetVal());
 
-            // This block must be ending with a GT_RETURN
+            // This block must be ending with a GT_RETURN/GT_SWIFT_ERROR_RET
             noway_assert(lastStmt != nullptr);
             noway_assert(lastStmt->GetNextStmt() == nullptr);
             noway_assert(ret != nullptr);
 
-            // GT_RETURN must have non-null operand as the method is returning the value assigned to
+            // Return node must have non-null operand as the method is returning the value assigned to
             // genReturnLocal
             GenTree* const retVal = ret->AsOp()->GetReturnValue();
             noway_assert(retVal != nullptr);
@@ -14159,11 +14159,11 @@ void Compiler::fgMergeBlockReturn(BasicBlock* block)
         }
         else if ((ret != nullptr) && ret->OperIs(GT_RETURN, GT_SWIFT_ERROR_RET))
         {
-            // This block ends with a GT_RETURN
+            // This block ends with a GT_RETURN/GT_SWIFT_ERROR_RET
             noway_assert(lastStmt != nullptr);
             noway_assert(lastStmt->GetNextStmt() == nullptr);
 
-            // Must be a void GT_RETURN/GT_SWIFT_ERROR_RET with null operand; delete it as this block branches to
+            // Must be a void return node with null operand; delete it as this block branches to
             // oneReturn block
             GenTree* const retVal = ret->AsOp()->GetReturnValue();
             noway_assert(ret->TypeGet() == TYP_VOID);
diff --git a/src/coreclr/jit/optimizebools.cpp b/src/coreclr/jit/optimizebools.cpp
index 1e5d5a00b107c0..79b0f506d4f63a 100644
--- a/src/coreclr/jit/optimizebools.cpp
+++ b/src/coreclr/jit/optimizebools.cpp
@@ -18,15 +18,43 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
 /*****************************************************************************/
 
 //-----------------------------------------------------------------------------
-// OptTestInfo:     Member of OptBoolsDsc struct used to test if a GT_JTRUE or GT_RETURN node
+// OptTestInfo:     Member of OptBoolsDsc struct used to test if a GT_JTRUE or return node
 //                  is a boolean comparison
 //
 struct OptTestInfo
 {
     Statement* testStmt; // Last statement of the basic block
-    GenTree*   testTree; // The root node of the testStmt (GT_JTRUE or GT_RETURN).
+    GenTree*   testTree; // The root node of the testStmt (GT_JTRUE or GT_RETURN/GT_SWIFT_ERROR_RET).
     GenTree*   compTree; // The compare node (i.e. GT_EQ or GT_NE node) of the testTree
     bool       isBool;   // If the compTree is boolean expression
+
+    GenTree* GetTestOp() const
+    {
+        assert(testTree != nullptr);
+
+        if (testTree->OperIs(GT_JTRUE))
+        {
+            return testTree->gtGetOp1();
+        }
+
+        assert(testTree->OperIs(GT_RETURN, GT_SWIFT_ERROR_RET));
+        return testTree->AsOp()->GetReturnValue();
+    }
+
+    void SetTestOp(GenTree* const op)
+    {
+        assert(testTree != nullptr);
+
+        if (testTree->OperIs(GT_JTRUE))
+        {
+            testTree->AsOp()->gtOp1 = op;
+        }
+        else
+        {
+            assert(testTree->OperIs(GT_RETURN, GT_SWIFT_ERROR_RET));
+            testTree->AsOp()->SetReturnValue(op);
+        }
+    }
 };
 
 //-----------------------------------------------------------------------------
@@ -100,12 +128,12 @@ class OptBoolsDsc
 //          For example, (x == 0 && y == 0 && z == 0) generates
 //              B1: GT_JTRUE (BBJ_COND), jump to B4
 //              B2: GT_JTRUE (BBJ_COND), jump to B4
-//              B3: GT_RETURN (BBJ_RETURN)
-//              B4: GT_RETURN (BBJ_RETURN)
+//              B3: GT_RETURN/GT_SWIFT_ERROR_RET (BBJ_RETURN)
+//              B4: GT_RETURN/GT_SWIFT_ERROR_RET (BBJ_RETURN)
 //          and B1 and B2 are folded into B1:
 //              B1: GT_JTRUE (BBJ_COND), jump to B4
-//              B3: GT_RETURN (BBJ_RETURN)
-//              B4: GT_RETURN (BBJ_RETURN)
+//              B3: GT_RETURN/GT_SWIFT_ERROR_RET (BBJ_RETURN)
+//              B4: GT_RETURN/GT_SWIFT_ERROR_RET (BBJ_RETURN)
 //
 //      Case 2: if B2->FalseTargetIs(B1->GetTarget()), it transforms
 //          B1 : brtrue(t1, B3)
@@ -234,8 +262,8 @@ bool OptBoolsDsc::optOptimizeBoolsCondBlock()
             cmpOp  = GT_EQ;
         }
         else if (m_testInfo1.compTree->gtOper == GT_LT && m_testInfo2.compTree->gtOper == GT_LT &&
-                 (!m_testInfo1.testTree->AsOp()->gtOp1->IsUnsigned() &&
-                  !m_testInfo2.testTree->AsOp()->gtOp1->IsUnsigned()))
+                 (!m_testInfo1.GetTestOp()->IsUnsigned() &&
+                  !m_testInfo2.GetTestOp()->IsUnsigned()))
         {
             // t1:c1<0 t2:c2<0 ==> Branch to BX if either value < 0
             // So we will branch to BX if (c1|c2)<0
@@ -297,8 +325,8 @@ bool OptBoolsDsc::optOptimizeBoolsCondBlock()
             cmpOp  = GT_NE;
         }
         else if (m_testInfo1.compTree->gtOper == GT_LT && m_testInfo2.compTree->gtOper == GT_GE &&
-                 (!m_testInfo1.testTree->AsOp()->gtOp1->IsUnsigned() &&
-                  !m_testInfo2.testTree->AsOp()->gtOp1->IsUnsigned()))
+                 (!m_testInfo1.GetTestOp()->IsUnsigned() &&
+                  !m_testInfo2.GetTestOp()->IsUnsigned()))
         {
             // t1:c1<0 t2:c2>=0 ==> Branch to BX if both values >= 0
             // So we will branch to BX if (c1|c2)>=0
@@ -1023,7 +1051,7 @@ bool OptBoolsDsc::optOptimizeCompareChainCondBlock()
         m_comp->gtNewOperNode(GT_NE, TYP_INT, chainedConditions, m_comp->gtNewZeroConNode(TYP_INT));
 
     // Wire the chain into the second block
-    m_testInfo2.testTree->AsOp()->gtOp1 = testcondition;
+    m_testInfo2.SetTestOp(testcondition);
     m_testInfo2.testTree->AsOp()->gtFlags |= (testcondition->gtFlags & GTF_ALL_EFFECT);
     m_comp->gtSetEvalOrder(m_testInfo2.testTree);
     m_comp->fgSetStmtSeq(s2);
@@ -1062,7 +1090,7 @@ bool OptBoolsDsc::optOptimizeCompareChainCondBlock()
 //
 // Notes:
 //      This method checks if the second (and third block for cond/return/return case) contains only one statement,
-//      and checks if tree operators are of the right type, e.g, GT_JTRUE, GT_RETURN.
+//      and checks if tree operators are of the right type, e.g, GT_JTRUE, GT_RETURN, GT_SWIFT_ERROR_RET.
 //
 //      On entry, m_b1, m_b2 are set and m_b3 is set for cond/return/return case.
 //      If it passes all the conditions, m_testInfo1.testTree, m_testInfo2.testTree and m_t3 are set
@@ -1091,7 +1119,7 @@ Statement* OptBoolsDsc::optOptimizeBoolsChkBlkCond()
     Statement* s1 = m_b1->lastStmt();
 
     GenTree* testTree1 = s1->GetRootNode();
-    assert(testTree1->gtOper == GT_JTRUE);
+    assert(testTree1->OperIs(GT_JTRUE));
 
     // The second and the third block must contain a single statement
 
@@ -1105,11 +1133,11 @@ Statement* OptBoolsDsc::optOptimizeBoolsChkBlkCond()
 
     if (!optReturnBlock)
     {
-        assert(testTree2->gtOper == GT_JTRUE);
+        assert(testTree2->OperIs(GT_JTRUE));
     }
     else
     {
-        if (testTree2->gtOper != GT_RETURN)
+        if (!testTree2->OperIs(GT_RETURN, GT_SWIFT_ERROR_RET))
         {
             return nullptr;
         }
@@ -1121,7 +1149,7 @@ Statement* OptBoolsDsc::optOptimizeBoolsChkBlkCond()
         }
 
         GenTree* testTree3 = s3->GetRootNode();
-        if (testTree3->gtOper != GT_RETURN)
+        if (!testTree3->OperIs(GT_RETURN, GT_SWIFT_ERROR_RET))
         {
             return nullptr;
         }
@@ -1132,12 +1160,13 @@ Statement* OptBoolsDsc::optOptimizeBoolsChkBlkCond()
         }
 
         // The third block is Return with "CNS_INT int 0/1"
-        if (testTree3->AsOp()->gtOp1->gtOper != GT_CNS_INT)
+        GenTree* const retVal = testTree3->AsOp()->GetReturnValue();
+        if (!retVal->OperIs(GT_CNS_INT))
         {
             return nullptr;
         }
 
-        if (testTree3->AsOp()->gtOp1->gtType != TYP_INT)
+        if (!retVal->TypeIs(TYP_INT))
         {
             return nullptr;
         }
@@ -1236,10 +1265,10 @@ void OptBoolsDsc::optOptimizeBoolsUpdateTrees()
     t1Comp->AsOp()->gtOp2->gtType = m_foldType; // Could have been varTypeIsGC()
     if (optReturnBlock)
     {
-        // Update tree when m_b1 is BBJ_COND and m_b2 and m_b3 are GT_RETURN (BBJ_RETURN)
+        // Update tree when m_b1 is BBJ_COND and m_b2 and m_b3 are GT_RETURN/GT_SWIFT_ERROR_RET (BBJ_RETURN)
         t1Comp->AsOp()->gtOp2->AsIntCon()->gtIconVal = 0;
-        m_testInfo1.testTree->gtOper                 = GT_RETURN;
-        m_testInfo1.testTree->gtType                 = m_testInfo2.testTree->gtType;
+        m_testInfo1.testTree->gtOper                 = m_testInfo2.testTree->OperGet();
+        m_testInfo1.testTree->gtType                 = m_testInfo2.testTree->TypeGet();
 
         // Update the return count of flow graph
         assert(m_comp->fgReturnCount >= 2);
@@ -1388,10 +1417,10 @@ void OptBoolsDsc::optOptimizeBoolsUpdateTrees()
 //
 //          For example, (x==0 && y==0) generates:
 //              B1: GT_JTRUE (BBJ_COND), jumps to B3
-//              B2: GT_RETURN (BBJ_RETURN)
-//              B3: GT_RETURN (BBJ_RETURN),
+//              B2: GT_RETURN/GT_SWIFT_ERROR (BBJ_RETURN)
+//              B3: GT_RETURN/GT_SWIFT_ERROR (BBJ_RETURN),
 //          and it is folded into
-//              B1: GT_RETURN (BBJ_RETURN)
+//              B1: GT_RETURN/GT_SWIFT_ERROR (BBJ_RETURN)
 //
 bool OptBoolsDsc::optOptimizeBoolsReturnBlock(BasicBlock* b3)
 {
@@ -1530,7 +1559,7 @@ bool OptBoolsDsc::optOptimizeBoolsReturnBlock(BasicBlock* b3)
     }
     else if ((m_testInfo1.compTree->gtOper == GT_LT && m_testInfo2.compTree->gtOper == GT_GE) &&
              (it1val == 0 && it2val == 0 && it3val == 0) &&
-             (!m_testInfo1.testTree->AsOp()->gtOp1->IsUnsigned() && !m_testInfo2.testTree->AsOp()->gtOp1->IsUnsigned()))
+             (!m_testInfo1.GetTestOp()->IsUnsigned() && !m_testInfo2.GetTestOp()->IsUnsigned()))
     {
         // Case: x >= 0 && y >= 0
         //      t1:c1<0 t2:c2>=0 t3:c3==0
@@ -1559,7 +1588,7 @@ bool OptBoolsDsc::optOptimizeBoolsReturnBlock(BasicBlock* b3)
     }
     else if ((m_testInfo1.compTree->gtOper == GT_LT && m_testInfo2.compTree->gtOper == GT_LT) &&
              (it1val == 0 && it2val == 0 && it3val == 1) &&
-             (!m_testInfo1.testTree->AsOp()->gtOp1->IsUnsigned() && !m_testInfo2.testTree->AsOp()->gtOp1->IsUnsigned()))
+             (!m_testInfo1.GetTestOp()->IsUnsigned() && !m_testInfo2.GetTestOp()->IsUnsigned()))
     {
         // Case: x < 0 || y < 0
         //      t1:c1<0 t2:c2<0 t3:c3==1
@@ -1676,7 +1705,7 @@ void OptBoolsDsc::optOptimizeBoolsGcStress()
 //      On success, compTree is set to the compare node (i.e. GT_EQ or GT_NE or GT_LT or GT_GE) of the testTree.
 //      isBool is set to true if the comparand (i.e., operand 1 of compTree is boolean. Otherwise, false.
 //
-//      Given a GT_JTRUE or GT_RETURN node, this method checks if it is a boolean comparison
+//      Given a GT_JTRUE or GT_RETURN/GT_SWIFT_ERROR_RET node, this method checks if it is a boolean comparison
 //      of the form "if (boolVal ==/!=/>=/<  0/1)".This is translated into
 //      a GT_EQ/GT_NE/GT_GE/GT_LT node with "opr1" being a boolean lclVar and "opr2" the const 0/1.
 //
@@ -1687,8 +1716,8 @@ GenTree* OptBoolsDsc::optIsBoolComp(OptTestInfo* pOptTest)
 {
     pOptTest->isBool = false;
 
-    assert(pOptTest->testTree->gtOper == GT_JTRUE || pOptTest->testTree->gtOper == GT_RETURN);
-    GenTree* cond = pOptTest->testTree->AsOp()->gtOp1;
+    assert(pOptTest->testTree->OperIs(GT_JTRUE, GT_RETURN, GT_SWIFT_ERROR_RET));
+    GenTree* cond = pOptTest->GetTestOp();
 
     // The condition must be "!= 0" or "== 0" or >=0 or <= 0 or > 0 or < 0
     if (!cond->OperIs(GT_EQ, GT_NE, GT_LT, GT_GT, GT_GE, GT_LE))
@@ -1749,13 +1778,13 @@ GenTree* OptBoolsDsc::optIsBoolComp(OptTestInfo* pOptTest)
 }
 
 //-----------------------------------------------------------------------------
-// optOptimizeBools:    Folds boolean conditionals for GT_JTRUE/GT_RETURN nodes
+// optOptimizeBools:    Folds boolean conditionals for GT_JTRUE/GT_RETURN/GT_SWIFT_ERROR_RET nodes
 //
 // Returns:
 //    suitable phase status
 //
 // Notes:
-//      If the operand of GT_JTRUE/GT_RETURN node is GT_EQ/GT_NE/GT_GE/GT_LE/GT_GT/GT_LT of the form
+//      If the operand of GT_JTRUE/GT_RETURN/GT_SWIFT_ERROR_RET node is GT_EQ/GT_NE/GT_GE/GT_LE/GT_GT/GT_LT of the form
 //      "if (boolVal ==/!=/>=/<  0/1)", the GT_EQ/GT_NE/GT_GE/GT_LE/GT_GT/GT_LT nodes are translated into a
 //      GT_EQ/GT_NE/GT_GE/GT_LE/GT_GT/GT_LT node with
 //          "op1" being a boolean GT_OR/GT_AND lclVar and
diff --git a/src/coreclr/jit/promotion.cpp b/src/coreclr/jit/promotion.cpp
index e02a5f0e06bab3..3f195d8d8a5bc4 100644
--- a/src/coreclr/jit/promotion.cpp
+++ b/src/coreclr/jit/promotion.cpp
@@ -1446,9 +1446,9 @@ class LocalsUseVisitor : public GenTreeVisitor<LocalsUseVisitor>
             flags |= AccessKindFlags::IsStoreSource;
         }
 
-        if (user->OperIs(GT_RETURN))
+        if (user->OperIs(GT_RETURN, GT_SWIFT_ERROR_RET))
         {
-            assert(user->gtGetOp1()->gtEffectiveVal() == lcl);
+            assert(user->AsOp()->GetReturnValue()->gtEffectiveVal() == lcl);
             flags |= AccessKindFlags::IsReturned;
         }
 #endif
@@ -2546,7 +2546,7 @@ void ReplaceVisitor::ReplaceLocal(GenTree** use, GenTree* user)
         JITDUMP("Processing struct use [%06u] of V%02u.[%03u..%03u)\n", Compiler::dspTreeID(lcl), lclNum, offs,
                 offs + lcl->GetLayout(m_compiler)->GetSize());
 
-        assert(effectiveUser->OperIs(GT_CALL, GT_RETURN));
+        assert(effectiveUser->OperIs(GT_CALL, GT_RETURN, GT_SWIFT_ERROR_RET));
         unsigned size = lcl->GetLayout(m_compiler)->GetSize();
         WriteBackBeforeUse(use, lclNum, lcl->GetLclOffs(), size);
 

From 7096e23c5ba19522f0fb2f1baa688275b0663c0d Mon Sep 17 00:00:00 2001
From: Aman Khalid <amankhalid@microsoft.com>
Date: Wed, 10 Apr 2024 18:29:19 +0000
Subject: [PATCH 17/28] Style

---
 src/coreclr/jit/codegenlinear.cpp | 3 ++-
 src/coreclr/jit/compiler.h        | 2 +-
 src/coreclr/jit/flowgraph.cpp     | 2 +-
 src/coreclr/jit/ifconversion.cpp  | 3 ++-
 src/coreclr/jit/morph.cpp         | 3 ++-
 src/coreclr/jit/optimizebools.cpp | 6 ++----
 6 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/src/coreclr/jit/codegenlinear.cpp b/src/coreclr/jit/codegenlinear.cpp
index c00f83580e82af..97b6ce2d582aad 100644
--- a/src/coreclr/jit/codegenlinear.cpp
+++ b/src/coreclr/jit/codegenlinear.cpp
@@ -500,7 +500,8 @@ void CodeGen::genCodeForBBlist()
         GenTree* blockLastNode = block->lastNode();
         if ((blockLastNode != nullptr) && (blockLastNode->OperIs(GT_RETURN, GT_SWIFT_ERROR_RET)) &&
             (varTypeIsGC(compiler->info.compRetType) ||
-             (blockLastNode->AsOp()->GetReturnValue() != nullptr && varTypeIsGC(blockLastNode->AsOp()->GetReturnValue()->TypeGet()))))
+             (blockLastNode->AsOp()->GetReturnValue() != nullptr &&
+              varTypeIsGC(blockLastNode->AsOp()->GetReturnValue()->TypeGet()))))
         {
             nonVarPtrRegs &= ~RBM_INTRET;
         }
diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h
index 00e8edc20861ae..175912a8760312 100644
--- a/src/coreclr/jit/compiler.h
+++ b/src/coreclr/jit/compiler.h
@@ -8412,7 +8412,7 @@ class Compiler
 
 #ifdef SWIFT_SUPPORT
     unsigned genReturnErrorLocal; // Local number for the Swift error value when applicable.
-#endif // SWIFT_SUPPORT
+#endif                            // SWIFT_SUPPORT
 
     // The following properties are part of CodeGenContext.  Getters are provided here for
     // convenience and backward compatibility, but the properties can only be set by invoking
diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp
index 464ad5131bc5dc..b06b81fdfd9a3a 100644
--- a/src/coreclr/jit/flowgraph.cpp
+++ b/src/coreclr/jit/flowgraph.cpp
@@ -2587,7 +2587,7 @@ PhaseStatus Compiler::fgAddSwiftErrorReturns()
                 assert(genReturnErrorLocal == BAD_VAR_NUM);
                 genReturnErrorLocal = lvaGrabTemp(true DEBUGARG("Single return block SwiftError value"));
                 lvaGetDesc(genReturnErrorLocal)->lvType = genActualType(TYP_I_IMPL);
-                ret->AsOp()->gtOp1 = gtNewLclvNode(genReturnErrorLocal, TYP_I_IMPL);
+                ret->AsOp()->gtOp1                      = gtNewLclvNode(genReturnErrorLocal, TYP_I_IMPL);
             }
             else
             {
diff --git a/src/coreclr/jit/ifconversion.cpp b/src/coreclr/jit/ifconversion.cpp
index 290ab35ba57070..7c088804f8dfdd 100644
--- a/src/coreclr/jit/ifconversion.cpp
+++ b/src/coreclr/jit/ifconversion.cpp
@@ -328,7 +328,8 @@ bool OptIfConversionDsc::IfConvertCheckStmts(BasicBlock* fromBlock, IfConvertOpe
                     // with the condition (for example, the condition could be an explicit bounds
                     // check and the operand could read an array element). Disallow this except
                     // for some common cases that we know are always side effect free.
-                    if (((m_cond->gtFlags & GTF_ORDER_SIDEEFF) != 0) && !retVal->IsInvariant() && !retVal->OperIsLocal())
+                    if (((m_cond->gtFlags & GTF_ORDER_SIDEEFF) != 0) && !retVal->IsInvariant() &&
+                        !retVal->OperIsLocal())
                     {
                         return false;
                     }
diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp
index 03ee2cd4355996..f0fb4905dd98d6 100644
--- a/src/coreclr/jit/morph.cpp
+++ b/src/coreclr/jit/morph.cpp
@@ -14187,7 +14187,8 @@ void Compiler::fgMergeBlockReturn(BasicBlock* block)
         {
             assert(genReturnErrorLocal != BAD_VAR_NUM);
             Statement* insertAfterStmt = block->lastStmt();
-            GenTree* swiftErrorStore = gtNewTempStore(genReturnErrorLocal, errorVal, CHECK_SPILL_NONE, &insertAfterStmt, insertAfterStmt->GetDebugInfo(), block);
+            GenTree* swiftErrorStore = gtNewTempStore(genReturnErrorLocal, errorVal, CHECK_SPILL_NONE, &insertAfterStmt,
+                                                      insertAfterStmt->GetDebugInfo(), block);
             fgNewStmtAtEnd(block, swiftErrorStore);
         }
 #endif // SWIFT_SUPPORT
diff --git a/src/coreclr/jit/optimizebools.cpp b/src/coreclr/jit/optimizebools.cpp
index 79b0f506d4f63a..07da4731252ff6 100644
--- a/src/coreclr/jit/optimizebools.cpp
+++ b/src/coreclr/jit/optimizebools.cpp
@@ -262,8 +262,7 @@ bool OptBoolsDsc::optOptimizeBoolsCondBlock()
             cmpOp  = GT_EQ;
         }
         else if (m_testInfo1.compTree->gtOper == GT_LT && m_testInfo2.compTree->gtOper == GT_LT &&
-                 (!m_testInfo1.GetTestOp()->IsUnsigned() &&
-                  !m_testInfo2.GetTestOp()->IsUnsigned()))
+                 (!m_testInfo1.GetTestOp()->IsUnsigned() && !m_testInfo2.GetTestOp()->IsUnsigned()))
         {
             // t1:c1<0 t2:c2<0 ==> Branch to BX if either value < 0
             // So we will branch to BX if (c1|c2)<0
@@ -325,8 +324,7 @@ bool OptBoolsDsc::optOptimizeBoolsCondBlock()
             cmpOp  = GT_NE;
         }
         else if (m_testInfo1.compTree->gtOper == GT_LT && m_testInfo2.compTree->gtOper == GT_GE &&
-                 (!m_testInfo1.GetTestOp()->IsUnsigned() &&
-                  !m_testInfo2.GetTestOp()->IsUnsigned()))
+                 (!m_testInfo1.GetTestOp()->IsUnsigned() && !m_testInfo2.GetTestOp()->IsUnsigned()))
         {
             // t1:c1<0 t2:c2>=0 ==> Branch to BX if both values >= 0
             // So we will branch to BX if (c1|c2)>=0

From 927a55e9ff4ab1c18f8b4869652f04967382b518 Mon Sep 17 00:00:00 2001
From: Aman Khalid <amankhalid@microsoft.com>
Date: Wed, 10 Apr 2024 18:33:29 +0000
Subject: [PATCH 18/28] Add comment

---
 src/coreclr/jit/flowgraph.cpp | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp
index b06b81fdfd9a3a..cb948a45651de9 100644
--- a/src/coreclr/jit/flowgraph.cpp
+++ b/src/coreclr/jit/flowgraph.cpp
@@ -2560,6 +2560,14 @@ PhaseStatus Compiler::fgAddInternal()
 }
 
 #ifdef SWIFT_SUPPORT
+//------------------------------------------------------------------------
+// fgAddSwiftErrorReturns: If this method uses Swift error handling,
+// transform all GT_RETURN nodes into GT_SWIFT_ERROR_RET nodes
+// to handle returning the error value alongside the normal return value.
+//
+// Returns:
+//   Suitable phase status.
+//
 PhaseStatus Compiler::fgAddSwiftErrorReturns()
 {
     if (lvaSwiftErrorArg == BAD_VAR_NUM)

From 848704be21f55027e1ea2c931380dee876c926fe Mon Sep 17 00:00:00 2001
From: Aman Khalid <amankhalid@microsoft.com>
Date: Wed, 10 Apr 2024 18:49:56 +0000
Subject: [PATCH 19/28] Remove ifdef

---
 src/coreclr/jit/compphases.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/src/coreclr/jit/compphases.h b/src/coreclr/jit/compphases.h
index 6ecf8a70a64852..950313286d26fe 100644
--- a/src/coreclr/jit/compphases.h
+++ b/src/coreclr/jit/compphases.h
@@ -34,9 +34,7 @@ CompPhaseNameMacro(PHASE_INCPROFILE,                 "Profile incorporation",
 CompPhaseNameMacro(PHASE_MORPH_INIT,                 "Morph - Init",                   false, -1, false)
 CompPhaseNameMacro(PHASE_MORPH_INLINE,               "Morph - Inlining",               false, -1, true)
 CompPhaseNameMacro(PHASE_MORPH_ADD_INTERNAL,         "Morph - Add internal blocks",    false, -1, true)
-#ifdef SWIFT_SUPPORT
 CompPhaseNameMacro(PHASE_SWIFT_ERROR_RET,            "Add Swift error returns",        false, -1, true)
-#endif // SWIFT_SUPPORT
 CompPhaseNameMacro(PHASE_ALLOCATE_OBJECTS,           "Allocate Objects",               false, -1, false)
 CompPhaseNameMacro(PHASE_EMPTY_TRY,                  "Remove empty try",               false, -1, false)
 CompPhaseNameMacro(PHASE_EMPTY_FINALLY,              "Remove empty finally",           false, -1, false)

From 502f4715d9440d7730d01ecb2f6c0af9b467f274 Mon Sep 17 00:00:00 2001
From: Aman Khalid <amankhalid@microsoft.com>
Date: Wed, 10 Apr 2024 19:07:07 +0000
Subject: [PATCH 20/28] Fix store ordering

---
 src/coreclr/jit/morph.cpp | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp
index f0fb4905dd98d6..ef1ad0541dadd1 100644
--- a/src/coreclr/jit/morph.cpp
+++ b/src/coreclr/jit/morph.cpp
@@ -14111,7 +14111,17 @@ void Compiler::fgMergeBlockReturn(BasicBlock* block)
         }
 
 #ifdef SWIFT_SUPPORT
-        GenTree* const errorVal = ((ret != nullptr) && ret->OperIs(GT_SWIFT_ERROR_RET)) ? ret->gtGetOp1() : nullptr;
+        // If merging GT_SWIFT_ERROR_RET nodes, ensure the error operand is stored to the merged return error local,
+        // so the correct error value is retrieved in the merged return 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);
+            fgInsertStmtBefore(block, lastStmt, newStmt);
+        }
 #endif // SWIFT_SUPPORT
 
         if (genReturnLocal != BAD_VAR_NUM)
@@ -14180,19 +14190,6 @@ void Compiler::fgMergeBlockReturn(BasicBlock* block)
             }
         }
 
-#ifdef SWIFT_SUPPORT
-        // If merging GT_SWIFT_ERROR_RET nodes, ensure the error operand is stored to the merged return error local,
-        // so the correct error value is retrieved in the merged return block.
-        if (errorVal != nullptr)
-        {
-            assert(genReturnErrorLocal != BAD_VAR_NUM);
-            Statement* insertAfterStmt = block->lastStmt();
-            GenTree* swiftErrorStore = gtNewTempStore(genReturnErrorLocal, errorVal, CHECK_SPILL_NONE, &insertAfterStmt,
-                                                      insertAfterStmt->GetDebugInfo(), block);
-            fgNewStmtAtEnd(block, swiftErrorStore);
-        }
-#endif // SWIFT_SUPPORT
-
         JITDUMP("\nUpdate " FMT_BB " to jump to common return block.\n", block->bbNum);
         DISPBLOCK(block);
 

From e8b1dbc859bbd55a2964ef44c6b116b5f0e5cb08 Mon Sep 17 00:00:00 2001
From: Aman Khalid <amankhalid@microsoft.com>
Date: Wed, 10 Apr 2024 19:09:11 +0000
Subject: [PATCH 21/28] Style

---
 src/coreclr/jit/morph.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp
index ef1ad0541dadd1..a7bcd803c615a1 100644
--- a/src/coreclr/jit/morph.cpp
+++ b/src/coreclr/jit/morph.cpp
@@ -14117,8 +14117,8 @@ void Compiler::fgMergeBlockReturn(BasicBlock* block)
         {
             assert(genReturnErrorLocal != BAD_VAR_NUM);
             const DebugInfo& di = lastStmt->GetDebugInfo();
-            GenTree* swiftErrorStore = gtNewTempStore(genReturnErrorLocal, ret->gtGetOp1(), CHECK_SPILL_NONE, nullptr,
-                                                      di, block);
+            GenTree*         swiftErrorStore =
+                gtNewTempStore(genReturnErrorLocal, ret->gtGetOp1(), CHECK_SPILL_NONE, nullptr, di, block);
             Statement* const newStmt = gtNewStmt(swiftErrorStore, di);
             fgInsertStmtBefore(block, lastStmt, newStmt);
         }

From 2dce428ab3835788ba7c27bfbfe3f47bdecbac31 Mon Sep 17 00:00:00 2001
From: Aman Khalid <amankhalid@microsoft.com>
Date: Wed, 10 Apr 2024 19:58:22 +0000
Subject: [PATCH 22/28] Do SwiftError local conversion in phase

---
 src/coreclr/jit/flowgraph.cpp | 33 +++++++++++++++++++++++++++++++++
 src/coreclr/jit/importer.cpp  | 15 ---------------
 2 files changed, 33 insertions(+), 15 deletions(-)

diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp
index cb948a45651de9..562b174f874d63 100644
--- a/src/coreclr/jit/flowgraph.cpp
+++ b/src/coreclr/jit/flowgraph.cpp
@@ -2564,6 +2564,8 @@ PhaseStatus Compiler::fgAddInternal()
 // fgAddSwiftErrorReturns: If this method uses Swift error handling,
 // transform all GT_RETURN nodes into GT_SWIFT_ERROR_RET nodes
 // to handle returning the error value alongside the normal return value.
+// Also transform any GT_LCL_VAR uses of lvaSwiftErrorArg (the SwiftError* parameter)
+// into GT_LCL_ADDR uses of lvaSwiftErrorLocal (the SwiftError pseudolocal).
 //
 // Returns:
 //   Suitable phase status.
@@ -2579,8 +2581,39 @@ PhaseStatus Compiler::fgAddSwiftErrorReturns()
     assert(lvaSwiftErrorLocal != BAD_VAR_NUM);
     assert(info.compCallConv == CorInfoCallConvExtension::Swift);
 
+    struct ReplaceSwiftErrorVisitor final : public GenTreeVisitor<ReplaceSwiftErrorVisitor>
+    {
+        enum
+        {
+            DoPreOrder    = true,
+            DoLclVarsOnly = true,
+        };
+
+        ReplaceSwiftErrorVisitor(Compiler* comp)
+            : GenTreeVisitor(comp)
+        {
+        }
+
+        fgWalkResult PreOrderVisit(GenTree** use, GenTree* user)
+        {
+            if ((*use)->OperIs(GT_LCL_VAR) && ((*use)->AsLclVarCommon()->GetLclNum() == m_compiler->lvaSwiftErrorArg))
+            {
+                *use = m_compiler->gtNewLclVarAddrNode(m_compiler->lvaSwiftErrorLocal, genActualType(*use));
+            }
+
+            return fgWalkResult::WALK_CONTINUE;
+        }
+    };
+
+    ReplaceSwiftErrorVisitor visitor(this);
+
     for (BasicBlock* block : Blocks())
     {
+        for (Statement* const stmt : block->Statements())
+        {
+            visitor.WalkTree(stmt->GetRootNodePointer(), nullptr);
+        }
+
         if (block->KindIs(BBJ_RETURN))
         {
             GenTree* const ret = block->lastNode();
diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp
index 12a612fde1a66a..242162e77e50cb 100644
--- a/src/coreclr/jit/importer.cpp
+++ b/src/coreclr/jit/importer.cpp
@@ -10464,21 +10464,6 @@ void Compiler::impLoadArg(unsigned ilArgNum, IL_OFFSET offset)
         {
             lclNum = lvaArg0Var;
         }
-#ifdef SWIFT_SUPPORT
-        else if (lclNum == lvaSwiftErrorArg)
-        {
-            // Convert any usages of the SwiftError pointer/ref parameter to pointers/refs to the SwiftError pseudolocal
-            // (set side effect flags so usages of references to pseudolocal aren't removed)
-            assert(info.compCallConv == CorInfoCallConvExtension::Swift);
-            assert(lvaSwiftErrorArg != BAD_VAR_NUM);
-            assert(lvaSwiftErrorLocal != BAD_VAR_NUM);
-            const var_types type               = lvaGetDesc(lvaSwiftErrorArg)->TypeGet();
-            GenTree* const  swiftErrorLocalRef = gtNewLclVarAddrNode(lvaSwiftErrorLocal, type);
-            impPushOnStack(swiftErrorLocalRef, typeInfo(type));
-            JITDUMP("\nCreated GT_LCL_ADDR of SwiftError pseudolocal\n");
-            return;
-        }
-#endif // SWIFT_SUPPORT
 
         impLoadVar(lclNum, offset);
     }

From 38ad8a70bcd79cbca021635470ca177d8b64443d Mon Sep 17 00:00:00 2001
From: Aman Khalid <amankhalid@microsoft.com>
Date: Wed, 10 Apr 2024 19:59:35 +0000
Subject: [PATCH 23/28] Delete assert

---
 src/coreclr/jit/promotion.cpp | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/coreclr/jit/promotion.cpp b/src/coreclr/jit/promotion.cpp
index 3f195d8d8a5bc4..c22bf74e8309f6 100644
--- a/src/coreclr/jit/promotion.cpp
+++ b/src/coreclr/jit/promotion.cpp
@@ -1448,7 +1448,6 @@ class LocalsUseVisitor : public GenTreeVisitor<LocalsUseVisitor>
 
         if (user->OperIs(GT_RETURN, GT_SWIFT_ERROR_RET))
         {
-            assert(user->AsOp()->GetReturnValue()->gtEffectiveVal() == lcl);
             flags |= AccessKindFlags::IsReturned;
         }
 #endif

From 18df9c7e9bae345a0a9ae9b55f81399dabde4176 Mon Sep 17 00:00:00 2001
From: "Aman Khalid (from Dev Box)" <amankhalid@microsoft.com>
Date: Wed, 10 Apr 2024 17:59:55 -0400
Subject: [PATCH 24/28] Fix build

---
 src/coreclr/jit/ifconversion.cpp | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/src/coreclr/jit/ifconversion.cpp b/src/coreclr/jit/ifconversion.cpp
index 7c088804f8dfdd..185eb4d90644b8 100644
--- a/src/coreclr/jit/ifconversion.cpp
+++ b/src/coreclr/jit/ifconversion.cpp
@@ -140,8 +140,16 @@ bool OptIfConversionDsc::IfConvertCheckThenFlow()
             if (thenBlock->KindIs(BBJ_RETURN))
             {
                 assert(m_finalBlock == nullptr);
-                m_mainOper = thenBlock->lastNode()->OperGet();
-                assert((m_mainOper == GT_RETURN) || (m_mainOper == GT_SWIFT_ERROR_RET));
+#ifdef SWIFT_SUPPORT
+                if (m_comp->lvaSwiftErrorArg != BAD_VAR_NUM)
+                {
+                    m_mainOper = GT_SWIFT_ERROR_RET;
+                }
+                else
+#endif // SWIFT_SUPPORT
+                {
+                    m_mainOper = GT_RETURN;
+                }
             }
             else
             {

From 86a0825770d6d1726c812ce5b3db697375f6f298 Mon Sep 17 00:00:00 2001
From: Aman Khalid <amankhalid@microsoft.com>
Date: Thu, 11 Apr 2024 01:55:07 +0000
Subject: [PATCH 25/28] Fix codegen on arm64

---
 src/coreclr/jit/codegenarm64.cpp  |  2 +-
 src/coreclr/jit/codegencommon.cpp | 10 +++++-----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/coreclr/jit/codegenarm64.cpp b/src/coreclr/jit/codegenarm64.cpp
index 32c944c69870ae..d4e8d58db741b7 100644
--- a/src/coreclr/jit/codegenarm64.cpp
+++ b/src/coreclr/jit/codegenarm64.cpp
@@ -2998,7 +2998,7 @@ void CodeGen::genCodeForStoreLclVar(GenTreeLclVar* lclNode)
 void CodeGen::genSimpleReturn(GenTree* treeNode)
 {
     assert(treeNode->OperIs(GT_RETURN, GT_RETFILT, GT_SWIFT_ERROR_RET));
-    GenTree*  op1        = treeNode->gtGetOp1();
+    GenTree*  op1        = treeNode->AsOp()->GetReturnValue();
     var_types targetType = treeNode->TypeGet();
 
     assert(targetType != TYP_STRUCT);
diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp
index f46ccdc4409808..d2709fc4c66144 100644
--- a/src/coreclr/jit/codegencommon.cpp
+++ b/src/coreclr/jit/codegencommon.cpp
@@ -7733,8 +7733,8 @@ GenTreeIntCon CodeGen::intForm(var_types type, ssize_t value)
 //
 void CodeGen::genLongReturn(GenTree* treeNode)
 {
-    assert(treeNode->OperGet() == GT_RETURN || treeNode->OperGet() == GT_RETFILT);
-    assert(treeNode->TypeGet() == TYP_LONG);
+    assert(treeNode->OperIs(GT_RETURN, GT_RETFILT));
+    assert(treeNode->TypeIs(TYP_LONG));
     GenTree*  op1        = treeNode->gtGetOp1();
     var_types targetType = treeNode->TypeGet();
 
@@ -8006,11 +8006,11 @@ void CodeGen::genStructReturn(GenTree* treeNode)
 {
     assert(treeNode->OperIs(GT_RETURN, GT_SWIFT_ERROR_RET));
 
-    genConsumeRegs(treeNode->gtGetOp1());
-
-    GenTree* op1       = treeNode->gtGetOp1();
+    GenTree* op1       = treeNode->AsOp()->GetReturnValue();
     GenTree* actualOp1 = op1->gtSkipReloadOrCopy();
 
+    genConsumeRegs(op1);
+
     ReturnTypeDesc retTypeDesc = compiler->compRetTypeDesc;
     const unsigned regCount    = retTypeDesc.GetReturnRegCount();
     assert(regCount <= MAX_RET_REG_COUNT);

From 41ec8f7ab640c8cd36ae66059935b6d2cc5927f5 Mon Sep 17 00:00:00 2001
From: Aman Khalid <amankhalid@microsoft.com>
Date: Thu, 11 Apr 2024 14:26:23 +0000
Subject: [PATCH 26/28] Feedback

---
 src/coreclr/jit/flowgraph.cpp    |  7 ++++++-
 src/coreclr/jit/ifconversion.cpp |  4 ++--
 src/coreclr/jit/morph.cpp        |  7 +++----
 src/coreclr/jit/valuenum.cpp     | 16 +++++++++++++---
 4 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp
index 562b174f874d63..c09ee90a41bf33 100644
--- a/src/coreclr/jit/flowgraph.cpp
+++ b/src/coreclr/jit/flowgraph.cpp
@@ -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");
+                }
+
                 *use = m_compiler->gtNewLclVarAddrNode(m_compiler->lvaSwiftErrorLocal, genActualType(*use));
             }
 
diff --git a/src/coreclr/jit/ifconversion.cpp b/src/coreclr/jit/ifconversion.cpp
index 185eb4d90644b8..bd3f96466e9b82 100644
--- a/src/coreclr/jit/ifconversion.cpp
+++ b/src/coreclr/jit/ifconversion.cpp
@@ -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)
diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp
index a7bcd803c615a1..22f2faa7f1ea51 100644
--- a/src/coreclr/jit/morph.cpp
+++ b/src/coreclr/jit/morph.cpp
@@ -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
diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp
index 03a14fec9968c9..d80a747e974d91 100644
--- a/src/coreclr/jit/valuenum.cpp
+++ b/src/coreclr/jit/valuenum.cpp
@@ -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;
 

From 6d9d360e24abb3073fb13f38d58a94a81e397653 Mon Sep 17 00:00:00 2001
From: Aman Khalid <amankhalid@microsoft.com>
Date: Thu, 11 Apr 2024 14:55:45 +0000
Subject: [PATCH 27/28] Update BADCODE message

---
 src/coreclr/jit/flowgraph.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp
index c09ee90a41bf33..209be64422bcd3 100644
--- a/src/coreclr/jit/flowgraph.cpp
+++ b/src/coreclr/jit/flowgraph.cpp
@@ -2600,7 +2600,7 @@ PhaseStatus Compiler::fgAddSwiftErrorReturns()
             {
                 if (!(*use)->OperIs(GT_LCL_VAR))
                 {
-                    BADCODE("Expected GT_LCL_VAR of SwiftError* parameter, got other GenTreeLclVarCommon node");
+                    BADCODE("Found invalid use of SwiftError* parameter");
                 }
 
                 *use = m_compiler->gtNewLclVarAddrNode(m_compiler->lvaSwiftErrorLocal, genActualType(*use));

From 0cd339d8729930483059737b5d1c39d362aa0b70 Mon Sep 17 00:00:00 2001
From: Aman Khalid <amankhalid@microsoft.com>
Date: Fri, 12 Apr 2024 14:41:24 +0000
Subject: [PATCH 28/28] Feedback

---
 src/coreclr/jit/flowgraph.cpp    |  2 +-
 src/coreclr/jit/ifconversion.cpp | 17 ++++-------------
 src/coreclr/jit/lower.cpp        | 13 +++++--------
 src/coreclr/jit/lower.h          |  2 +-
 src/coreclr/jit/morph.cpp        | 12 ++++++++++++
 5 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp
index 209be64422bcd3..c9beffdb6575a6 100644
--- a/src/coreclr/jit/flowgraph.cpp
+++ b/src/coreclr/jit/flowgraph.cpp
@@ -2632,7 +2632,7 @@ PhaseStatus Compiler::fgAddSwiftErrorReturns()
             {
                 assert(genReturnErrorLocal == BAD_VAR_NUM);
                 genReturnErrorLocal = lvaGrabTemp(true DEBUGARG("Single return block SwiftError value"));
-                lvaGetDesc(genReturnErrorLocal)->lvType = genActualType(TYP_I_IMPL);
+                lvaGetDesc(genReturnErrorLocal)->lvType = TYP_I_IMPL;
                 ret->AsOp()->gtOp1                      = gtNewLclvNode(genReturnErrorLocal, TYP_I_IMPL);
             }
             else
diff --git a/src/coreclr/jit/ifconversion.cpp b/src/coreclr/jit/ifconversion.cpp
index bd3f96466e9b82..6008bb432550b6 100644
--- a/src/coreclr/jit/ifconversion.cpp
+++ b/src/coreclr/jit/ifconversion.cpp
@@ -140,16 +140,7 @@ bool OptIfConversionDsc::IfConvertCheckThenFlow()
             if (thenBlock->KindIs(BBJ_RETURN))
             {
                 assert(m_finalBlock == nullptr);
-#ifdef SWIFT_SUPPORT
-                if (m_comp->lvaSwiftErrorArg != BAD_VAR_NUM)
-                {
-                    m_mainOper = GT_SWIFT_ERROR_RET;
-                }
-                else
-#endif // SWIFT_SUPPORT
-                {
-                    m_mainOper = GT_RETURN;
-                }
+                m_mainOper = GT_RETURN;
             }
             else
             {
@@ -586,7 +577,7 @@ bool OptIfConversionDsc::optIfConvert()
     {
         return false;
     }
-    assert(m_thenOperation.node->OperIs(GT_STORE_LCL_VAR, GT_RETURN, GT_SWIFT_ERROR_RET));
+    assert(m_thenOperation.node->OperIs(GT_STORE_LCL_VAR, GT_RETURN));
     if (m_doElseConversion)
     {
         if (!IfConvertCheckStmts(m_startBlock->GetTrueTarget(), &m_elseOperation))
@@ -644,7 +635,7 @@ bool OptIfConversionDsc::optIfConvert()
         }
         else
         {
-            assert((m_mainOper == GT_RETURN) || (m_mainOper == GT_SWIFT_ERROR_RET));
+            assert(m_mainOper == GT_RETURN);
             thenCost = m_thenOperation.node->AsOp()->GetReturnValue()->GetCostEx();
             if (m_doElseConversion)
             {
@@ -704,7 +695,7 @@ bool OptIfConversionDsc::optIfConvert()
     }
     else
     {
-        assert((m_mainOper == GT_RETURN) || (m_mainOper == GT_SWIFT_ERROR_RET));
+        assert(m_mainOper == GT_RETURN);
         assert(m_doElseConversion);
         assert(m_thenOperation.node->TypeGet() == m_elseOperation.node->TypeGet());
 
diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp
index 2e74e1069b6777..2257fca33d4e4a 100644
--- a/src/coreclr/jit/lower.cpp
+++ b/src/coreclr/jit/lower.cpp
@@ -521,11 +521,8 @@ GenTree* Lowering::LowerNode(GenTree* node)
             break;
 
         case GT_SWIFT_ERROR_RET:
-            LowerNode(node->gtGetOp1());
-
-            FALLTHROUGH;
         case GT_RETURN:
-            LowerRet(node->AsUnOp());
+            LowerRet(node->AsOp());
             break;
 
         case GT_RETURNTRAP:
@@ -4545,8 +4542,8 @@ void Lowering::LowerJmpMethod(GenTree* jmp)
     }
 }
 
-// Lower GT_RETURN node to insert PInvoke method epilog if required.
-void Lowering::LowerRet(GenTreeUnOp* ret)
+// Lower GT_RETURN/GT_SWIFT_ERROR_RET node to insert PInvoke method epilog if required.
+void Lowering::LowerRet(GenTreeOp* ret)
 {
     assert(ret->OperIs(GT_RETURN, GT_SWIFT_ERROR_RET));
 
@@ -4554,7 +4551,7 @@ void Lowering::LowerRet(GenTreeUnOp* ret)
     DISPNODE(ret);
     JITDUMP("============\n");
 
-    GenTree* retVal = ret->AsOp()->GetReturnValue();
+    GenTree* retVal = ret->GetReturnValue();
     // There are two kinds of retyping:
     // - A simple bitcast can be inserted when:
     //   - We're returning a floating type as an integral type or vice-versa, or
@@ -4576,7 +4573,7 @@ void Lowering::LowerRet(GenTreeUnOp* ret)
 #endif
 
         GenTree* bitcast = comp->gtNewBitCastNode(ret->TypeGet(), retVal);
-        ret->AsOp()->SetReturnValue(bitcast);
+        ret->SetReturnValue(bitcast);
         BlockRange().InsertBefore(ret, bitcast);
         ContainCheckBitCast(bitcast);
     }
diff --git a/src/coreclr/jit/lower.h b/src/coreclr/jit/lower.h
index 0538c661e16260..055f425b664566 100644
--- a/src/coreclr/jit/lower.h
+++ b/src/coreclr/jit/lower.h
@@ -154,7 +154,7 @@ class Lowering final : public Phase
     bool       TryLowerConditionToFlagsNode(GenTree* parent, GenTree* condition, GenCondition* code);
     GenTreeCC* LowerNodeCC(GenTree* node, GenCondition condition);
     void       LowerJmpMethod(GenTree* jmp);
-    void       LowerRet(GenTreeUnOp* ret);
+    void       LowerRet(GenTreeOp* ret);
     void       LowerStoreLocCommon(GenTreeLclVarCommon* lclVar);
     void       LowerRetStruct(GenTreeUnOp* ret);
     void       LowerRetSingleRegStructLclVar(GenTreeUnOp* ret);
diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp
index 22f2faa7f1ea51..56f5d89372050a 100644
--- a/src/coreclr/jit/morph.cpp
+++ b/src/coreclr/jit/morph.cpp
@@ -8755,6 +8755,18 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA
             if (fgGlobalMorph && varTypeIsSmall(info.compRetType) && (retVal != nullptr) && !retVal->TypeIs(TYP_VOID) &&
                 fgCastNeeded(retVal, info.compRetType))
             {
+#ifdef SWIFT_SUPPORT
+                // Morph error operand if tree is a GT_SWIFT_ERROR_RET node
+                if (tree->OperIs(GT_SWIFT_ERROR_RET))
+                {
+                    GenTree* const errorVal = fgMorphTree(tree->gtGetOp1());
+                    tree->AsOp()->gtOp1     = errorVal;
+
+                    // Propagate side effect flags
+                    tree->SetAllEffectsFlags(errorVal);
+                }
+#endif // SWIFT_SUPPORT
+
                 // Small-typed return values are normalized by the callee
                 retVal = gtNewCastNode(TYP_INT, retVal, false, info.compRetType);