Changes for basic block flags. (#37335)

Fix the code that propagates flags from the basic block of the return
expression to the caller's basic block during inlining. We are now
properly tracking the basic block of the return expression.
Fixes #36588.

Don't mark BBJ_RETURN blocks with BBF_BACKWARD_JUMP since they are
executed at most once.

The first change had a few diffs because we propagated BBF_BACKWARD_JUMP
flag. After analyzing them I realized that we shouldn't mark BBJ_RETURN
blocks with BBF_BACKWARD_JUMP in the first place. That resulted in some
diffs because we are less aggressive with inlining of calls outside of
loops.

src/coreclr/src/jit/flowgraph.cpp

@@ -5275,7 +5275,7 @@ void Compiler::fgMarkBackwardJump(BasicBlock* targetBlock, BasicBlock* sourceBlo
 
     for (BasicBlock* block = targetBlock; block != sourceBlock->bbNext; block = block->bbNext)
     {
-        if ((block->bbFlags & BBF_BACKWARD_JUMP) == 0)
+        if (((block->bbFlags & BBF_BACKWARD_JUMP) == 0) && (block->bbJumpKind != BBJ_RETURN))
         {
             block->bbFlags |= BBF_BACKWARD_JUMP;
             compHasBackwardJump = true;
@@ -22625,9 +22625,10 @@ Compiler::fgWalkResult Compiler::fgUpdateInlineReturnExpressionPlaceHolder(GenTr
         // This folding may uncover more GT_RET_EXPRs, so we loop around
         // until we've got something distinct.
         //
-        GenTree* inlineCandidate = tree->gtRetExprVal();
-        inlineCandidate          = comp->gtFoldExpr(inlineCandidate);
-        var_types retType        = tree->TypeGet();
+        unsigned __int64 bbFlags         = 0;
+        GenTree*         inlineCandidate = tree->gtRetExprVal(&bbFlags);
+        inlineCandidate                  = comp->gtFoldExpr(inlineCandidate);
+        var_types retType                = tree->TypeGet();
 
 #ifdef DEBUG
         if (comp->verbose)
@@ -22643,6 +22644,7 @@ Compiler::fgWalkResult Compiler::fgUpdateInlineReturnExpressionPlaceHolder(GenTr
 #endif // DEBUG
 
         tree->ReplaceWith(inlineCandidate, comp);
+        comp->compCurBB->bbFlags |= (bbFlags & BBF_SPLIT_GAINED);
 
 #ifdef DEBUG
         if (comp->verbose)
@@ -22992,6 +22994,7 @@ void Compiler::fgInvokeInlineeCompiler(GenTreeCall* call, InlineResult* inlineRe
     inlineInfo.iciBlock               = compCurBB;
     inlineInfo.thisDereferencedFirst  = false;
     inlineInfo.retExpr                = nullptr;
+    inlineInfo.retBB                  = nullptr;
     inlineInfo.retExprClassHnd        = nullptr;
     inlineInfo.retExprClassHndIsExact = false;
     inlineInfo.inlineResult           = inlineResult;
@@ -23355,7 +23358,7 @@ void Compiler::fgInsertInlineeBlocks(InlineInfo* pInlineInfo)
 
         // topBlock contains at least one statement before the split.
         // And the split is before the first statement.
-        // In this case, topBlock should be empty, and everything else should be moved to the bottonBlock.
+        // In this case, topBlock should be empty, and everything else should be moved to the bottomBlock.
         bottomBlock->bbStmtList = topBlock->bbStmtList;
         topBlock->bbStmtList    = nullptr;
     }
@@ -23535,13 +23538,17 @@ void Compiler::fgInsertInlineeBlocks(InlineInfo* pInlineInfo)
             gtDispTree(pInlineInfo->retExpr);
         }
 #endif // DEBUG
-        // Replace the call with the return expression
-        iciCall->ReplaceWith(pInlineInfo->retExpr, this);
 
-        if (bottomBlock != nullptr)
+        // Replace the call with the return expression. Note that iciCall won't be part of the IR
+        // but may still be referenced from a GT_RET_EXPR node. We will replace GT_RET_EXPR node
+        // in fgUpdateInlineReturnExpressionPlaceHolder. At that time we will also update the flags
+        // on the basic block of GT_RET_EXPR node.
+        if (iciCall->gtInlineCandidateInfo->retExpr->OperGet() == GT_RET_EXPR)
         {
-            bottomBlock->bbFlags |= InlineeCompiler->fgLastBB->bbFlags & BBF_SPLIT_GAINED;
+            // Save the basic block flags from the retExpr basic block.
+            iciCall->gtInlineCandidateInfo->retExpr->AsRetExpr()->bbFlags = pInlineInfo->retBB->bbFlags;
         }
+        iciCall->ReplaceWith(pInlineInfo->retExpr, this);
     }
 
     //

src/coreclr/src/jit/gentree.cpp

@@ -6263,9 +6263,11 @@ GenTree* Compiler::gtNewInlineCandidateReturnExpr(GenTree* inlineCandidate, var_
 {
     assert(GenTree::s_gtNodeSizes[GT_RET_EXPR] == TREE_NODE_SZ_LARGE);
 
-    GenTree* node = new (this, GT_RET_EXPR) GenTreeRetExpr(type);
+    GenTreeRetExpr* node = new (this, GT_RET_EXPR) GenTreeRetExpr(type);
 
-    node->AsRetExpr()->gtInlineCandidate = inlineCandidate;
+    node->gtInlineCandidate = inlineCandidate;
+
+    node->bbFlags = 0;
 
     if (varTypeIsStruct(inlineCandidate) && !inlineCandidate->OperIsBlkOp())
     {

src/coreclr/src/jit/gentree.h

@@ -1708,7 +1708,7 @@ struct GenTree
     inline GenTree* gtEffectiveVal(bool commaOnly = false);
 
     // Tunnel through any GT_RET_EXPRs
-    inline GenTree* gtRetExprVal();
+    inline GenTree* gtRetExprVal(unsigned __int64* pbbFlags = nullptr);
 
     // Return the child of this node if it is a GT_RELOAD or GT_COPY; otherwise simply return the node itself
     inline GenTree* gtSkipReloadOrCopy();
@@ -5628,6 +5628,8 @@ struct GenTreeRetExpr : public GenTree
 {
     GenTree* gtInlineCandidate;
 
+    unsigned __int64 bbFlags;
+
     CORINFO_CLASS_HANDLE gtRetClsHnd;
 
     GenTreeRetExpr(var_types type) : GenTree(GT_RET_EXPR, type)
@@ -7040,6 +7042,11 @@ inline GenTree* GenTree::gtEffectiveVal(bool commaOnly)
 //-------------------------------------------------------------------------
 // gtRetExprVal - walk back through GT_RET_EXPRs
 //
+// Arguments:
+//    pbbFlags - out-parameter that is set to the flags of the basic block
+//               containing the inlinee return value. The value is 0
+//               for unsuccessful inlines.
+//
 // Returns:
 //    tree representing return value from a successful inline,
 //    or original call for failed or yet to be determined inline.
@@ -7048,17 +7055,25 @@ inline GenTree* GenTree::gtEffectiveVal(bool commaOnly)
 //    Multi-level inlines can form chains of GT_RET_EXPRs.
 //    This method walks back to the root of the chain.
 
-inline GenTree* GenTree::gtRetExprVal()
+inline GenTree* GenTree::gtRetExprVal(unsigned __int64* pbbFlags)
 {
-    GenTree* retExprVal = this;
+    GenTree*         retExprVal = this;
+    unsigned __int64 bbFlags    = 0;
+
     for (;;)
     {
         if (retExprVal->gtOper == GT_RET_EXPR)
         {
-            retExprVal = retExprVal->AsRetExpr()->gtInlineCandidate;
+            GenTreeRetExpr* retExp = retExprVal->AsRetExpr();
+            retExprVal             = retExp->gtInlineCandidate;
+            bbFlags                = retExp->bbFlags;
         }
         else
         {
+            if (pbbFlags != nullptr)
+            {
+                *pbbFlags = bbFlags;
+            }
             return retExprVal;
         }
     }

src/coreclr/src/jit/importer.cpp

@@ -16836,6 +16836,11 @@ bool Compiler::impReturnInstruction(int prefixFlags, OPCODE& opcode)
                     }
                 }
             }
+
+            if (impInlineInfo->retExpr != nullptr)
+            {
+                impInlineInfo->retBB = compCurBB;
+            }
         }
     }
 

src/coreclr/src/jit/inline.h

@@ -584,6 +584,7 @@ struct InlineInfo
     InlineResult* inlineResult;
 
     GenTree*             retExpr; // The return expression of the inlined candidate.
+    BasicBlock*          retBB;   // The basic block of the return expression of the inlined candidate.
     CORINFO_CLASS_HANDLE retExprClassHnd;
     bool                 retExprClassHndIsExact;