JIT: Improve inliner - static readonly fields, pinvokes, Span::Length

src/coreclr/jit/compiler.cpp

@@ -6329,6 +6329,7 @@ int Compiler::compCompileHelper(CORINFO_MODULE_HANDLE classPtr,
 
     compHasBackwardJump          = false;
     compHasBackwardJumpInHandler = false;
+    compHasPinvokes              = false;
 
 #ifdef DEBUG
     compCurBB = nullptr;

src/coreclr/jit/compiler.h

@@ -9487,6 +9487,7 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
     bool compSwitchedToOptimized;      // Codegen initially was Tier0 but jit switched to FullOpts
     bool compSwitchedToMinOpts;        // Codegen initially was Tier1/FullOpts but jit switched to MinOpts
     bool compSuppressedZeroInit;       // There are vars with lvSuppressedZeroInit set
+    bool compHasPinvokes;              // Does the method have a pinvoke?
 
 // NOTE: These values are only reliable after
 //       the importing is completely finished.

src/coreclr/jit/compiler.hpp

@@ -1149,7 +1149,6 @@ inline GenTree* Compiler::gtNewIndexRef(var_types typ, GenTree* arrayOp, GenTree
 inline GenTreeArrLen* Compiler::gtNewArrLen(var_types typ, GenTree* arrayOp, int lenOffset, BasicBlock* block)
 {
     GenTreeArrLen* arrLen = new (this, GT_ARR_LENGTH) GenTreeArrLen(typ, arrayOp, lenOffset);
-    static_assert_no_msg(GTF_ARRLEN_NONFAULTING == GTF_IND_NONFAULTING);
     arrLen->SetIndirExceptionFlags(this);
     if (block != nullptr)
     {

src/coreclr/jit/fgbasic.cpp

@@ -1072,6 +1072,12 @@ void Compiler::fgFindJumpTargets(const BYTE* codeAddr, IL_OFFSET codeSize, Fixed
                     if (FgStack::IsConstantOrConstArg(slot, impInlineInfo) ||
                         FgStack::IsExactArgument(slot, impInlineInfo))
                     {
+                        if ((opcode == CEE_ISINST) && FgStack::IsExactArgument(slot, impInlineInfo))
+                        {
+                            // isinst on top of non-constant "exact" argument (at callsite)
+                            // most likely will fold this isinst
+                            pushedStack.PushConstant();
+                        }
                         compInlineResult->Note(InlineObservation::CALLSITE_FOLDABLE_EXPR_UN);
                         handled = true; // and keep argument in the pushedStack
                     }
@@ -1100,13 +1106,15 @@ void Compiler::fgFindJumpTargets(const BYTE* codeAddr, IL_OFFSET codeSize, Fixed
 
                 CORINFO_METHOD_HANDLE methodHnd   = nullptr;
                 bool                  isIntrinsic = false;
+                bool                  isPinvoke   = false;
                 NamedIntrinsic        ni          = NI_Illegal;
 
                 if (resolveTokens)
                 {
                     impResolveToken(codeAddr, &resolvedToken, CORINFO_TOKENKIND_Method);
                     methodHnd   = resolvedToken.hMethod;
                     isIntrinsic = eeIsIntrinsic(methodHnd);
+                    isPinvoke   = info.compCompHnd->getMethodAttribs(methodHnd) & CORINFO_FLG_PINVOKE;
                 }
 
                 if (isIntrinsic)
@@ -1127,14 +1135,20 @@ void Compiler::fgFindJumpTargets(const BYTE* codeAddr, IL_OFFSET codeSize, Fixed
                             // These are most likely foldable without arguments
                             case NI_System_Collections_Generic_Comparer_get_Default:
                             case NI_System_Collections_Generic_EqualityComparer_get_Default:
-                            case NI_System_Enum_HasFlag:
                             case NI_System_GC_KeepAlive:
                             {
                                 pushedStack.PushUnknown();
                                 foldableIntrinsc = true;
                                 break;
                             }
 
+                            case NI_System_Enum_HasFlag:
+                            {
+                                // keep its argument in the stack as a return value
+                                foldableIntrinsc = true;
+                                break;
+                            }
+
                             case NI_System_Span_get_Item:
                             case NI_System_ReadOnlySpan_get_Item:
                             {
@@ -1145,6 +1159,26 @@ void Compiler::fgFindJumpTargets(const BYTE* codeAddr, IL_OFFSET codeSize, Fixed
                                 break;
                             }
 
+                            case NI_System_Span_get_Length:
+                            case NI_System_ReadOnlySpan_get_Length:
+                            {
+                                // Keep argument on top of the stack
+                                handled = FgStack::IsArgument(pushedStack.Top(0));
+                                break;
+                            }
+
+                            case NI_System_Object_GetType:
+                            {
+                                // obj.GetType() is folded into typeof() if obj is exact
+                                if (FgStack::IsExactArgument(pushedStack.Top(0), impInlineInfo) ||
+                                    FgStack::IsConstantOrConstArg(pushedStack.Top(), impInlineInfo))
+                                {
+                                    pushedStack.PushConstant();
+                                    foldableIntrinsc = true;
+                                }
+                                break;
+                            }
+
                             // These are foldable if the first argument is a constant
                             case NI_System_Type_get_IsValueType:
                             case NI_System_Type_GetTypeFromHandle:
@@ -1237,6 +1271,10 @@ void Compiler::fgFindJumpTargets(const BYTE* codeAddr, IL_OFFSET codeSize, Fixed
                         }
                     }
                 }
+                else if (isPinvoke)
+                {
+                    compInlineResult->NoteBool(InlineObservation::CALLEE_HAS_PINVOKE, true);
+                }
 
                 if ((codeAddr < codeEndp - sz) && (OPCODE)getU1LittleEndian(codeAddr + sz) == CEE_RET)
                 {
@@ -1249,6 +1287,7 @@ void Compiler::fgFindJumpTargets(const BYTE* codeAddr, IL_OFFSET codeSize, Fixed
                 {
                     // Optimistically assume that "call(arg)" returns something arg-dependent.
                     // However, we don't know how many args it expects and its return type.
+                    // TODO: Make IL scan 100% precise so we can guess it looking at the stack.
                     handled = true;
                 }
             }
@@ -1538,7 +1577,7 @@ void Compiler::fgFindJumpTargets(const BYTE* codeAddr, IL_OFFSET codeSize, Fixed
                             if (FgStack::IsConstArgument(op1, impInlineInfo) ||
                                 FgStack::IsConstArgument(op2, impInlineInfo))
                             {
-                                compInlineResult->Note(InlineObservation::CALLSITE_CONSTANT_ARG_FEEDS_TEST);
+                                compInlineResult->Note(InlineObservation::CALLSITE_CONST_ARG_FEEDS_TEST);
                             }
 
                             if ((FgStack::IsArgument(op1) && FgStack::IsArrayLen(op2)) ||
@@ -1590,14 +1629,40 @@ void Compiler::fgFindJumpTargets(const BYTE* codeAddr, IL_OFFSET codeSize, Fixed
             case CEE_LDFLDA:
             case CEE_LDFLD:
             case CEE_STFLD:
-            {
-                if (FgStack::IsArgument(pushedStack.Top()))
+            case CEE_STSFLD:
+                if (makeInlineObservations)
                 {
-                    compInlineResult->Note(InlineObservation::CALLEE_ARG_STRUCT_FIELD_ACCESS);
-                    handled = true; // keep argument on top of the stack
+                    if (FgStack::IsConstArgument(pushedStack.Top(), impInlineInfo) &&
+                        ((opcode == CEE_STFLD) || (opcode == CEE_STSFLD)))
+                    {
+                        // If we inline we'll set a constant value (e.g. null) to this field
+                        // If it's of reference type we'll avoid an expensive write barrier here
+                        compInlineResult->Note(InlineObservation::CALLSITE_CONST_ARG_SETS_FLD);
+                    }
+                    else if (FgStack::IsArgument(pushedStack.Top()))
+                    {
+                        compInlineResult->Note(InlineObservation::CALLEE_ARG_STRUCT_FIELD_ACCESS);
+                        handled = true; // keep argument on top of the stack
+                    }
+                    break;
+                }
+                break;
+
+            case CEE_LDSFLD:
+                if (resolveTokens && !opts.jitFlags->IsSet(JitFlags::JIT_FLAG_PREJIT))
+                {
+                    // Push a constant to the stack for static readonly fields (initialized)
+                    impResolveToken(codeAddr, &resolvedToken, CORINFO_TOKENKIND_Field);
+                    CORINFO_FIELD_HANDLE fldHnd = resolvedToken.hField;
+                    bool                 inited = false;
+                    if ((info.compCompHnd->getStaticFieldCurrentClass(fldHnd, &inited) == NO_CLASS_HANDLE) && inited)
+                    {
+                        compInlineResult->Note(InlineObservation::CALLSITE_FOLDABLE_LDSFLD);
+                        pushedStack.PushConstant();
+                        handled = true;
+                    }
                 }
                 break;
-            }
 
             case CEE_LDELEM_I1:
             case CEE_LDELEM_U1:
@@ -2287,7 +2352,7 @@ void Compiler::fgObserveInlineConstants(OPCODE opcode, const FgStack& stack, boo
                     unsigned varNum = FgStack::SlotTypeToArgNum(slot0);
                     if (impInlineInfo->inlArgInfo[varNum].argIsInvariant)
                     {
-                        compInlineResult->Note(InlineObservation::CALLSITE_CONSTANT_ARG_FEEDS_TEST);
+                        compInlineResult->Note(InlineObservation::CALLSITE_CONST_ARG_FEEDS_TEST);
                     }
                 }
             }
@@ -2329,7 +2394,7 @@ void Compiler::fgObserveInlineConstants(OPCODE opcode, const FgStack& stack, boo
             unsigned varNum = FgStack::SlotTypeToArgNum(slot0);
             if (impInlineInfo->inlArgInfo[varNum].argIsInvariant)
             {
-                compInlineResult->Note(InlineObservation::CALLSITE_CONSTANT_ARG_FEEDS_TEST);
+                compInlineResult->Note(InlineObservation::CALLSITE_CONST_ARG_FEEDS_TEST);
             }
         }
 
@@ -2340,7 +2405,7 @@ void Compiler::fgObserveInlineConstants(OPCODE opcode, const FgStack& stack, boo
             unsigned varNum = FgStack::SlotTypeToArgNum(slot1);
             if (impInlineInfo->inlArgInfo[varNum].argIsInvariant)
             {
-                compInlineResult->Note(InlineObservation::CALLSITE_CONSTANT_ARG_FEEDS_TEST);
+                compInlineResult->Note(InlineObservation::CALLSITE_CONST_ARG_FEEDS_TEST);
             }
         }
     }

src/coreclr/jit/gentree.h

@@ -495,7 +495,6 @@ enum GenTreeFlags : unsigned int
     GTF_IND_TGT_NOT_HEAP        = 0x80000000, // GT_IND   -- the target is not on the heap
     GTF_IND_VOLATILE            = 0x40000000, // GT_IND   -- the load or store must use volatile sematics (this is a nop on X86)
     GTF_IND_NONFAULTING         = 0x20000000, // Operations for which OperIsIndir() is true  -- An indir that cannot fault.
-                                              // Same as GTF_ARRLEN_NONFAULTING.
     GTF_IND_TGTANYWHERE         = 0x10000000, // GT_IND   -- the target could be anywhere
     GTF_IND_TLS_REF             = 0x08000000, // GT_IND   -- the target is accessed via TLS
     GTF_IND_ASG_LHS             = 0x04000000, // GT_IND   -- this GT_IND node is (the effective val) of the LHS of an
@@ -582,7 +581,6 @@ enum GenTreeFlags : unsigned int
     GTF_CHK_INDEX_INBND         = 0x80000000, // GT_BOUNDS_CHECK -- have proved this check is always in-bounds
 
     GTF_ARRLEN_ARR_IDX          = 0x80000000, // GT_ARR_LENGTH -- Length which feeds into an array index expression
-    GTF_ARRLEN_NONFAULTING      = 0x20000000, // GT_ARR_LENGTH  -- An array length operation that cannot fault. Same as GT_IND_NONFAULTING.
 
     GTF_SIMDASHW_OP             = 0x80000000, // GT_HWINTRINSIC -- Indicates that the structHandle should be gotten from gtGetStructHandleForSIMD
                                               //                   rather than from gtGetStructHandleForHWSIMD.

src/coreclr/jit/importer.cpp

@@ -5239,13 +5239,21 @@ NamedIntrinsic Compiler::lookupNamedIntrinsic(CORINFO_METHOD_HANDLE method)
             {
                 result = NI_System_Span_get_Item;
             }
+            else if (strcmp(methodName, "get_Length") == 0)
+            {
+                result = NI_System_Span_get_Length;
+            }
         }
         else if (strcmp(className, "ReadOnlySpan`1") == 0)
         {
             if (strcmp(methodName, "get_Item") == 0)
             {
                 result = NI_System_ReadOnlySpan_get_Item;
             }
+            else if (strcmp(methodName, "get_Length") == 0)
+            {
+                result = NI_System_ReadOnlySpan_get_Length;
+            }
         }
         else if (strcmp(className, "EETypePtr") == 0)
         {
@@ -7696,6 +7704,7 @@ void Compiler::impCheckForPInvokeCall(
     if ((mflags & CORINFO_FLG_PINVOKE) != 0)
     {
         call->gtCallMoreFlags |= GTF_CALL_M_PINVOKE;
+        compHasPinvokes = true;
     }
 
     bool suppressGCTransition = false;

src/coreclr/jit/inline.def

@@ -89,6 +89,7 @@ INLINE_OBSERVATION(HAS_LOCALLOC,              bool,   "has localloc",
 INLINE_OBSERVATION(HAS_PINNED_LOCALS,         bool,   "has pinned locals",                    INFORMATION, CALLEE)
 INLINE_OBSERVATION(HAS_SIMD,                  bool,   "has SIMD arg, local, or ret",          INFORMATION, CALLEE)
 INLINE_OBSERVATION(HAS_SWITCH,                bool,   "has switch",                           INFORMATION, CALLEE)
+INLINE_OBSERVATION(HAS_PINVOKE,               bool,   "has pinvoke",                           INFORMATION, CALLEE)
 INLINE_OBSERVATION(IL_CODE_SIZE,              int,    "number of bytes of IL",                INFORMATION, CALLEE)
 INLINE_OBSERVATION(IS_CLASS_CTOR,             bool,   "class constructor",                    INFORMATION, CALLEE)
 INLINE_OBSERVATION(IS_DISCRETIONARY_INLINE,   bool,   "can inline, check heuristics",         INFORMATION, CALLEE)
@@ -178,12 +179,14 @@ INLINE_OBSERVATION(ARG_EXACT_CLS_SIG_IS_NOT,  int,    "arg is more concrete than
 INLINE_OBSERVATION(ARG_CONST,                 int,    "arg is a constant",                    INFORMATION, CALLSITE)
 INLINE_OBSERVATION(ARG_BOXED,                 int,    "arg is boxed at call site",            INFORMATION, CALLSITE)
 INLINE_OBSERVATION(FOLDABLE_INTRINSIC,        int,    "foldable intrinsic",                   INFORMATION, CALLSITE)
+INLINE_OBSERVATION(FOLDABLE_LDSFLD,           int,    "foldable ldsfld",                      INFORMATION, CALLSITE)
 INLINE_OBSERVATION(FOLDABLE_EXPR,             int,    "foldable binary expression",           INFORMATION, CALLSITE)
 INLINE_OBSERVATION(FOLDABLE_EXPR_UN,          int,    "foldable unary expression",            INFORMATION, CALLSITE)
 INLINE_OBSERVATION(FOLDABLE_BRANCH,           int,    "foldable branch",                      INFORMATION, CALLSITE)
 INLINE_OBSERVATION(FOLDABLE_SWITCH,           int,    "foldable switch",                      INFORMATION, CALLSITE)
 INLINE_OBSERVATION(DIV_BY_CNS,                int,    "dividy by const",                      INFORMATION, CALLSITE)
-INLINE_OBSERVATION(CONSTANT_ARG_FEEDS_TEST,   bool,   "constant argument feeds test",         INFORMATION, CALLSITE)
+INLINE_OBSERVATION(CONST_ARG_FEEDS_TEST,      bool,   "constant argument feeds test",         INFORMATION, CALLSITE)
+INLINE_OBSERVATION(CONST_ARG_SETS_FLD,        bool,   "constant argument sets field",         INFORMATION, CALLSITE)
 INLINE_OBSERVATION(DEPTH,                     int,    "depth",                                INFORMATION, CALLSITE)
 INLINE_OBSERVATION(FREQUENCY,                 int,    "rough call site frequency",            INFORMATION, CALLSITE)
 INLINE_OBSERVATION(HAS_PROFILE,               bool,   "profile data is available",            INFORMATION, CALLSITE)