JIT: Enable inlining for late devirtualization

src/coreclr/jit/compiler.h

@@ -3727,6 +3727,8 @@ class Compiler
 
     void gtUpdateNodeOperSideEffects(GenTree* tree);
 
+    bool gtSubTreeAndChildrenAreFirstExecutedEffects(GenTree* tree, GenTree* subTree, bool early = false);
+
     // Returns "true" iff the complexity (not formally defined, but first interpretation
     // is #of nodes in subtree) of "tree" is greater than "limit".
     // (This is somewhat redundant with the "GetCostEx()/GetCostSz()" fields, but can be used
@@ -5114,6 +5116,8 @@ class Compiler
         SpillCliqueSucc
     };
 
+    friend class SubstitutePlaceholdersAndDevirtualizeWalker;
+
     // Abstract class for receiving a callback while walking a spill clique
     class SpillCliqueWalker
     {

src/coreclr/jit/fginline.cpp

@@ -204,7 +204,9 @@ bool Compiler::TypeInstantiationComplexityExceeds(CORINFO_CLASS_HANDLE handle, i
 
 class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitor<SubstitutePlaceholdersAndDevirtualizeWalker>
 {
-    bool m_madeChanges = false;
+    bool       m_madeChanges  = false;
+    Statement* m_curStmt      = nullptr;
+    Statement* m_firstNewStmt = nullptr;
 
 public:
     enum
@@ -219,11 +221,29 @@ class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitor<Substi
     {
     }
 
-    bool MadeChanges()
+    bool MadeChanges() const
     {
         return m_madeChanges;
     }
 
+    // ------------------------------------------------------------------------
+    // WalkStatement: Walk the tree of a statement, and return the first newly
+    // added statement if any, otherwise return the original statement.
+    //
+    // Arguments:
+    //    stmt - the statement to walk.
+    //
+    // Return Value:
+    //    The first newly added statement if any, or the original statement.
+    //
+    Statement* WalkStatement(Statement* stmt)
+    {
+        m_curStmt      = stmt;
+        m_firstNewStmt = nullptr;
+        WalkTree(m_curStmt->GetRootNodePointer(), nullptr);
+        return m_firstNewStmt == nullptr ? m_curStmt : m_firstNewStmt;
+    }
+
     fgWalkResult PreOrderVisit(GenTree** use, GenTree* user)
     {
         GenTree* tree = *use;
@@ -586,8 +606,59 @@ class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitor<Substi
                 }
 
                 CORINFO_CONTEXT_HANDLE contextInput = context;
+                context                             = nullptr;
                 m_compiler->impDevirtualizeCall(call, nullptr, &method, &methodFlags, &contextInput, &context,
                                                 isLateDevirtualization, explicitTailCall);
+
+                if (context != nullptr)
+                {
+                    bool isFirstExecutedEffect =
+                        m_compiler->gtSubTreeAndChildrenAreFirstExecutedEffects(m_curStmt->GetRootNode(), call, true);
+
+                    if (isFirstExecutedEffect)
+                    {
+                        CORINFO_CALL_INFO callInfo = {};
+                        callInfo.hMethod           = method;
+                        callInfo.methodFlags       = methodFlags;
+                        m_compiler->impMarkInlineCandidate(call, context, false, &callInfo);
+
+                        if (call->IsInlineCandidate())
+                        {
+                            // If the call is the root expression in a statement, and it returns void,
+                            // we can inline it directly without creating a RET_EXPR.
+                            if (parent != nullptr || call->gtReturnType != TYP_VOID)
+                            {
+                                Statement* stmt = m_compiler->gtNewStmt(call);
+                                m_compiler->fgInsertStmtBefore(m_compiler->compCurBB, m_curStmt, stmt);
+                                if (m_firstNewStmt == nullptr)
+                                {
+                                    m_firstNewStmt = stmt;
+                                }
+
+                                GenTreeRetExpr* retExpr =
+                                    m_compiler->gtNewInlineCandidateReturnExpr(call->AsCall(),
+                                                                               genActualType(call->TypeGet()));
+                                call->GetSingleInlineCandidateInfo()->retExpr = retExpr;
+
+                                JITDUMP("Creating new RET_EXPR for [%06u]:\n", call->gtTreeID);
+                                DISPTREE(retExpr);
+
+                                *pTree = retExpr;
+                            }
+
+                            call->GetSingleInlineCandidateInfo()->exactContextHandle = context;
+                            INDEBUG(call->GetSingleInlineCandidateInfo()->inlinersContext = call->gtInlineContext);
+
+                            JITDUMP("New inline candidate due to late devirtualization:\n");
+                            DISPTREE(call);
+                        }
+                    }
+                    else
+                    {
+                        // TODO-CQ: Split the effects and enable inlining.
+                        JITDUMP("Give up inlining call [%06u] due to side effects\n", call->gtTreeID);
+                    }
+                }
                 m_madeChanges = true;
             }
         }
@@ -730,17 +801,10 @@ PhaseStatus Compiler::fgInline()
     do
     {
         // Make the current basic block address available globally
-        compCurBB = block;
-
-        for (Statement* const stmt : block->Statements())
+        compCurBB       = block;
+        Statement* stmt = block->firstStmt();
+        while (stmt != nullptr)
         {
-
-#if defined(DEBUG)
-            // In debug builds we want the inline tree to show all failed
-            // inlines. Some inlines may fail very early and never make it to
-            // candidate stage. So scan the tree looking for those early failures.
-            fgWalkTreePre(stmt->GetRootNodePointer(), fgFindNonInlineCandidate, stmt);
-#endif
             // See if we need to replace some return value place holders.
             // Also, see if this replacement enables further devirtualization.
             //
@@ -755,7 +819,7 @@ PhaseStatus Compiler::fgInline()
             // possible further optimization, as the (now complete) GT_RET_EXPR
             // replacement may have enabled optimizations by providing more
             // specific types for trees or variables.
-            walker.WalkTree(stmt->GetRootNodePointer(), nullptr);
+            stmt = walker.WalkStatement(stmt);
 
             GenTree* expr = stmt->GetRootNode();
 
@@ -805,6 +869,13 @@ PhaseStatus Compiler::fgInline()
                 madeChanges = true;
                 stmt->SetRootNode(expr->AsOp()->gtOp1);
             }
+
+#if defined(DEBUG)
+            // In debug builds we want the inline tree to show all failed
+            // inlines.
+            fgWalkTreePre(stmt->GetRootNodePointer(), fgFindNonInlineCandidate, stmt);
+#endif
+            stmt = stmt->GetNextStmt();
         }
 
         block = block->Next();

src/coreclr/jit/gentree.cpp

@@ -10213,6 +10213,77 @@ void Compiler::gtUpdateNodeSideEffects(GenTree* tree)
     });
 }
 
+//------------------------------------------------------------------------
+// gtSubTreeAndChildrenAreFirstExecutedEffects: Check whether the subtree is the first
+//                                              executed effect in a tree.
+//
+// Arguments:
+//    tree - Tree to check the side effects on
+//    subTree - The subtree to be the first executed effect
+//    early - Whether the run is early, in which case we need to check lvHasLdAddrOp as
+//            we don't have valid GTF_GLOB_REF yet
+//
+// Returns:
+//    A boolean that indicates whether the subtree is the first executed effect in a tree.
+//
+bool Compiler::gtSubTreeAndChildrenAreFirstExecutedEffects(GenTree* tree, GenTree* subTree, bool early)
+{
+    struct Visitor : GenTreeVisitor<Visitor>
+    {
+        enum
+        {
+            DoPreOrder        = true,
+            DoPostOrder       = true,
+            UseExecutionOrder = true,
+        };
+
+        Visitor(Compiler* comp, GenTree* subTree, bool checkLocal)
+            : GenTreeVisitor(comp)
+            , m_subTree(subTree)
+            , m_checkLocal(checkLocal)
+        {
+        }
+
+        fgWalkResult PreOrderVisit(GenTree** use, GenTree* user)
+        {
+            if (*use == m_subTree)
+            {
+                Result = true;
+                return WALK_ABORT;
+            }
+
+            return WALK_CONTINUE;
+        }
+
+        fgWalkResult PostOrderVisit(GenTree** use, GenTree* user)
+        {
+            if (((*use)->gtFlags & GTF_ALL_EFFECT) != GTF_EMPTY)
+            {
+                Result = false;
+                return WALK_ABORT;
+            }
+
+            if (m_checkLocal && (*use)->OperIsAnyLocal() &&
+                m_compiler->lvaGetDesc((*use)->AsLclVarCommon())->lvHasLdAddrOp)
+            {
+                Result = false;
+                return WALK_ABORT;
+            }
+
+            return WALK_CONTINUE;
+        }
+
+        bool Result = false;
+    private:
+        GenTree* m_subTree;
+        bool     m_checkLocal;
+    };
+
+    Visitor visitor(this, subTree, early);
+    visitor.WalkTree(&tree, nullptr);
+    return visitor.Result;
+}
+
 bool GenTree::gtSetFlags() const
 {
     return (gtFlags & GTF_SET_FLAGS) != 0;

src/coreclr/jit/importercalls.cpp

@@ -329,7 +329,7 @@ var_types Compiler::impImportCall(OPCODE                  opcode,
                     assert((sig->callConv & CORINFO_CALLCONV_MASK) != CORINFO_CALLCONV_VARARG &&
                            (sig->callConv & CORINFO_CALLCONV_MASK) != CORINFO_CALLCONV_NATIVEVARARG);
 
-                    call = gtNewIndCallNode(stubAddr, callRetTyp);
+                    call = gtNewIndCallNode(stubAddr, callRetTyp, di);
 
                     call->gtFlags |= GTF_EXCEPT | (stubAddr->gtFlags & GTF_GLOB_EFFECT);
                     call->gtFlags |= GTF_CALL_VIRT_STUB;