Improvements for ReadyToRun helpers in JIT value numbering.

The following ReadyToRun helpers are now treated similarly to their normal
counterparts in value numbering:
CORINFO_HELP_READYTORUN_NEW
CORINFO_HELP_READYTORUN_NEWARR_1
CORINFO_HELP_READYTORUN_ISINSTANCEOF
CORINFO_HELP_READYTORUN_CHKCAST
CORINFO_HELP_READYTORUN_STATIC_BASE

In particular, this allows CSE-ing calls to the last 3 of the above helpers when
possible. #3281 is an issue for CORINFO_HELP_READYTORUN_STATIC_BASE.

Compiler::fgValueNumberHelperCallFunc is refactored to reduce code
duplication.

Closes #3281.

diff --git a/src/jit/utils.cpp b/src/jit/utils.cpp
index 85ccdb7..f2539ca 100644 (file)
--- a/src/jit/utils.cpp
+++ b/src/jit/utils.cpp
@@ -1194,6 +1194,7 @@ void HelperCallProperties::init()
 
         case CORINFO_HELP_NEW_CROSSCONTEXT:
         case CORINFO_HELP_NEWFAST:
+        case CORINFO_HELP_READYTORUN_NEW:
 
             mayFinalize   = true;  // These may run a finalizer
             isAllocator   = true;
@@ -1215,7 +1216,7 @@ void HelperCallProperties::init()
         case CORINFO_HELP_NEW_MDARR:
         case CORINFO_HELP_NEWARR_1_DIRECT:
         case CORINFO_HELP_NEWARR_1_OBJ:
-
+        case CORINFO_HELP_READYTORUN_NEWARR_1:
 
             mayFinalize   = true;  // These may run a finalizer
             isAllocator   = true;
@@ -1263,7 +1264,8 @@ void HelperCallProperties::init()
         case CORINFO_HELP_ISINSTANCEOFARRAY:
         case CORINFO_HELP_ISINSTANCEOFCLASS:
         case CORINFO_HELP_ISINSTANCEOFANY:
-            
+        case CORINFO_HELP_READYTORUN_ISINSTANCEOF:
+
             isPure   = true;
             noThrow  = true;   // These return null for a failing cast
             break;
@@ -1274,7 +1276,8 @@ void HelperCallProperties::init()
         case CORINFO_HELP_CHKCASTCLASS:
         case CORINFO_HELP_CHKCASTANY:
         case CORINFO_HELP_CHKCASTCLASS_SPECIAL:
-            
+        case CORINFO_HELP_READYTORUN_CHKCAST:
+
             // These throw for a failing cast
             // But if given a null input arg will return null
             isPure = true;
@@ -1314,6 +1317,7 @@ void HelperCallProperties::init()
         case CORINFO_HELP_GETSTATICFIELDADDR_TLS:
         case CORINFO_HELP_GETGENERICS_GCSTATIC_BASE:
         case CORINFO_HELP_GETGENERICS_NONGCSTATIC_BASE:
+        case CORINFO_HELP_READYTORUN_STATIC_BASE:
 
             // These may invoke static class constructors
             // These can throw InvalidProgram exception if the class can not be constructed

diff --git a/src/jit/valuenum.cpp b/src/jit/valuenum.cpp
index 54ef578..5817d40 100644 (file)
--- a/src/jit/valuenum.cpp
+++ b/src/jit/valuenum.cpp
@@ -3024,20 +3024,24 @@ ValueNum ValueNumStore::GetArrForLenVn(ValueNum vn)
     return NoVN;
 }
 
-bool ValueNumStore::IsVNNewArr(ValueNum vn)
+bool ValueNumStore::IsVNNewArr(ValueNum vn, VNFuncApp* funcApp)
 {
     if (vn == NoVN) return false;
-    VNFuncApp funcAttr;
-    return (GetVNFunc(vn, &funcAttr) && funcAttr.m_func == VNF_JitNewArr);
+    bool result = false;
+    if (GetVNFunc(vn, funcApp))
+    {
+        result = (funcApp->m_func == VNF_JitNewArr) ||
+                 (funcApp->m_func == VNF_JitReadyToRunNewArr);
+    }
+    return result;
 }
 
 int ValueNumStore::GetNewArrSize(ValueNum vn)
 {
-    if (vn == NoVN) return 0;
-    VNFuncApp funcAttr;
-    if (GetVNFunc(vn, &funcAttr) && funcAttr.m_func == VNF_JitNewArr)
+    VNFuncApp funcApp;
+    if (IsVNNewArr(vn, &funcApp))
     {
-        ValueNum arg1VN = funcAttr.m_args[1];
+        ValueNum arg1VN = funcApp.m_args[1];
         if (IsVNConstant(arg1VN) && TypeOfVN(arg1VN) == TYP_INT)
         {
             return ConstantValue<int>(arg1VN);
@@ -6387,123 +6391,173 @@ void Compiler::fgValueNumberHelperCallFunc(GenTreeCall* call, VNFunc vnf, ValueN
 {
     unsigned nArgs = ValueNumStore::VNFuncArity(vnf);
     assert(vnf != VNF_Boundary);
+    GenTreeArgList* args = call->gtCallArgs;
+    bool generateUniqueVN = false;
+    bool useEntryPointAddrAsArg0 = false;
+
     switch (vnf)
     {
     case VNF_JitNew:
         {
-            GenTreeArgList* args = call->gtCallArgs;
-            ValueNumPair vnp0; ValueNumPair vnp0x = ValueNumStore::VNPForEmptyExcSet();
-            vnStore->VNPUnpackExc(args->Current()->gtVNPair, &vnp0, &vnp0x);
-            // Generate unique VN so, VNForFunc generates a uniq value number for new.
-            ValueNumPair vnpUniq;
-            vnpUniq.SetBoth(vnStore->VNForExpr(call->TypeGet()));
-            call->gtVNPair = vnStore->VNPWithExc(vnStore->VNPairForFunc(call->TypeGet(), vnf, vnp0, vnpUniq), vnp0x);
+            generateUniqueVN = true;
+            vnpExc = ValueNumStore::VNPForEmptyExcSet();
         }
         break;
 
     case VNF_JitNewArr:
         {
-            GenTreeArgList* args = call->gtCallArgs;
-            ValueNumPair vnp0; ValueNumPair vnp0x = ValueNumStore::VNPForEmptyExcSet();
-            vnStore->VNPUnpackExc(args->Current()->gtVNPair, &vnp0, &vnp0x);
-            args = args->Rest();
-            ValueNumPair vnp1; ValueNumPair vnp1x = ValueNumStore::VNPForEmptyExcSet();
-            vnStore->VNPUnpackExc(args->Current()->gtVNPair, &vnp1, &vnp1x);
-
-            // Generate unique VN so, VNForFunc generates a uniq value number for new.
-            ValueNumPair vnpUniq;  vnpUniq.SetBoth(vnStore->VNForExpr(call->TypeGet()));
+            generateUniqueVN = true;
+            ValueNumPair vnp1 = vnStore->VNPNormVal(args->Rest()->Current()->gtVNPair);
 
             // The New Array helper may throw an overflow exception
             vnpExc = vnStore->VNPExcSetSingleton(vnStore->VNPairForFunc(TYP_REF, VNF_NewArrOverflowExc, vnp1));
-
-            // Also include in the argument exception sets
-            vnpExc = vnStore->VNPExcSetUnion(vnpExc, vnp0x);
-            vnpExc = vnStore->VNPExcSetUnion(vnpExc, vnp1x);
-
-            call->gtVNPair = vnStore->VNPWithExc(vnStore->VNPairForFunc(call->TypeGet(), vnf, vnp0, vnp1, vnpUniq), vnpExc);
         }
         break;
 
     case VNF_BoxNullable:
         {
-            GenTreeArgList* args = call->gtCallArgs;
-            ValueNumPair vnp0; ValueNumPair vnp0x = ValueNumStore::VNPForEmptyExcSet();
-            vnStore->VNPUnpackExc(args->Current()->gtVNPair, &vnp0, &vnp0x);
-            args = args->Rest();
-            ValueNumPair vnp1; ValueNumPair vnp1x = ValueNumStore::VNPForEmptyExcSet();
-            vnStore->VNPUnpackExc(args->Current()->gtVNPair, &vnp1, &vnp1x);
-
             // Generate unique VN so, VNForFunc generates a uniq value number for box nullable.
-            ValueNumPair vnpUniq;  
-            vnpUniq.SetBoth(vnStore->VNForExpr(call->TypeGet()));
-
             // Alternatively instead of using vnpUniq below in VNPairForFunc(...), 
             // we could use the value number of what the byref arg0 points to.
             // 
             // But retrieving the value number of what the byref arg0 points to is quite a bit more work
             // and doing so only very rarely allows for an additional optimization.
+            generateUniqueVN = true;
+        }
+        break;
 
-            // Also include in the argument exception sets
-            vnpExc = vnStore->VNPExcSetUnion(vnpExc, vnp0x);
-            vnpExc = vnStore->VNPExcSetUnion(vnpExc, vnp1x);
+    case VNF_JitReadyToRunNew:
+        {
+            generateUniqueVN = true;
+            vnpExc = ValueNumStore::VNPForEmptyExcSet();
+            useEntryPointAddrAsArg0 = true;
+        }
+        break;
 
-            call->gtVNPair = vnStore->VNPWithExc(vnStore->VNPairForFunc(call->TypeGet(), vnf, vnp0, vnp1, vnpUniq), vnpExc);
+    case VNF_JitReadyToRunNewArr:
+        {
+            generateUniqueVN = true;
+            ValueNumPair vnp1 = vnStore->VNPNormVal(args->Current()->gtVNPair);
+
+            // The New Array helper may throw an overflow exception
+            vnpExc = vnStore->VNPExcSetSingleton(vnStore->VNPairForFunc(TYP_REF, VNF_NewArrOverflowExc, vnp1));
+            useEntryPointAddrAsArg0 = true;
+        }
+        break;
+
+    case VNF_ReadyToRunStaticBase:
+    case VNF_ReadyToRunIsInstanceOf:
+    case VNF_ReadyToRunCastClass:
+        {
+            useEntryPointAddrAsArg0 = true;
         }
         break;
 
     default:
-        GenTreeArgList* args = call->gtCallArgs;
-        assert(s_helperCallProperties.IsPure(eeGetHelperNum(call->gtCallMethHnd)));
+        {
+            assert(s_helperCallProperties.IsPure(eeGetHelperNum(call->gtCallMethHnd)));
+        }
+        break;
+    }
+
+    if (generateUniqueVN)
+    {
+        nArgs--;
+    }
 
-        if (nArgs == 0)
+    ValueNumPair vnpUniq;
+    if (generateUniqueVN)
+    {
+        // Generate unique VN so, VNForFunc generates a unique value number.
+        vnpUniq.SetBoth(vnStore->VNForExpr(call->TypeGet()));
+    }
+
+    if (nArgs == 0)
+    {
+        if (generateUniqueVN)
+        {
+            call->gtVNPair = vnStore->VNPairForFunc(call->TypeGet(), vnf, vnpUniq);
+        }
+        else
         {
             call->gtVNPair.SetBoth(vnStore->VNForFunc(call->TypeGet(), vnf));
         }
+    }
+    else
+    {
+        // Has at least one argument.
+        ValueNumPair vnp0; ValueNumPair vnp0x = ValueNumStore::VNPForEmptyExcSet();
+#ifdef FEATURE_READYTORUN_COMPILER
+        if (useEntryPointAddrAsArg0)
+        {
+            ValueNum callAddrVN = vnStore->VNForPtrSizeIntCon((ssize_t)call->gtCall.gtEntryPoint.addr);
+            vnp0 = ValueNumPair(callAddrVN, callAddrVN);
+        }
         else
+#endif
         {
-            // Has at least one argument.
+            assert(!useEntryPointAddrAsArg0);
             ValueNumPair vnp0wx = args->Current()->gtVNPair;
-            ValueNumPair vnp0; ValueNumPair vnp0x = ValueNumStore::VNPForEmptyExcSet();
             vnStore->VNPUnpackExc(vnp0wx, &vnp0, &vnp0x);
 
             // Also include in the argument exception sets
             vnpExc = vnStore->VNPExcSetUnion(vnpExc, vnp0x);
 
             args = args->Rest();
-            if (nArgs == 1)
+        }
+        if (nArgs == 1)
+        {
+            if (generateUniqueVN)
+            {
+                call->gtVNPair = vnStore->VNPairForFunc(call->TypeGet(), vnf, vnp0, vnpUniq);
+            }
+            else
             {
                 call->gtVNPair = vnStore->VNPairForFunc(call->TypeGet(), vnf, vnp0);
             }
+        }
+        else
+        {
+            // Has at least two arguments.
+            ValueNumPair vnp1wx = args->Current()->gtVNPair;
+            ValueNumPair vnp1; ValueNumPair vnp1x = ValueNumStore::VNPForEmptyExcSet();
+            vnStore->VNPUnpackExc(vnp1wx, &vnp1, &vnp1x);
+            vnpExc = vnStore->VNPExcSetUnion(vnpExc, vnp1x);
+
+            args = args->Rest();
+            if (nArgs == 2)
+            {
+                if (generateUniqueVN)
+                {
+                    call->gtVNPair = vnStore->VNPairForFunc(call->TypeGet(), vnf, vnp0, vnp1, vnpUniq);
+                }
+                else
+                {
+                    call->gtVNPair = vnStore->VNPairForFunc(call->TypeGet(), vnf, vnp0, vnp1);
+                }
+            }
             else
             {
-                // Has at least two arguments.
-                ValueNumPair vnp1wx = args->Current()->gtVNPair;
-                ValueNumPair vnp1; ValueNumPair vnp1x = ValueNumStore::VNPForEmptyExcSet();
-                vnStore->VNPUnpackExc(vnp1wx, &vnp1, &vnp1x);
-                vnpExc = vnStore->VNPExcSetUnion(vnpExc, vnp1x);
+                ValueNumPair vnp2wx = args->Current()->gtVNPair;
+                ValueNumPair vnp2; ValueNumPair vnp2x = ValueNumStore::VNPForEmptyExcSet();
+                vnStore->VNPUnpackExc(vnp2wx, &vnp2, &vnp2x);
+                vnpExc = vnStore->VNPExcSetUnion(vnpExc, vnp2x);
 
                 args = args->Rest();
-                if (nArgs == 2)
+                assert(nArgs == 3);  // Our current maximum.
+                assert(args == NULL);
+                if (generateUniqueVN)
                 {
-                    call->gtVNPair = vnStore->VNPairForFunc(call->TypeGet(), vnf, vnp0, vnp1);
+                    call->gtVNPair = vnStore->VNPairForFunc(call->TypeGet(), vnf, vnp0, vnp1, vnp2, vnpUniq);
                 }
                 else
                 {
-                    ValueNumPair vnp2wx = args->Current()->gtVNPair;
-                    ValueNumPair vnp2; ValueNumPair vnp2x = ValueNumStore::VNPForEmptyExcSet();
-                    vnStore->VNPUnpackExc(vnp2wx, &vnp2, &vnp2x);
-                    vnpExc = vnStore->VNPExcSetUnion(vnpExc, vnp2x);
-
-                    args = args->Rest();
-                    assert(nArgs == 3);  // Our current maximum.
-                    assert(args == NULL);
                     call->gtVNPair = vnStore->VNPairForFunc(call->TypeGet(), vnf, vnp0, vnp1, vnp2);
                 }
             }
-            // Add the accumulated exceptions.
-            call->gtVNPair = vnStore->VNPWithExc(call->gtVNPair, vnpExc);
         }
-        break;
+        // Add the accumulated exceptions.
+        call->gtVNPair = vnStore->VNPWithExc(call->gtVNPair, vnpExc);
     }
 }
 
@@ -6659,6 +6713,10 @@ VNFunc Compiler::fgValueNumberHelperMethVNFunc(CorInfoHelpFunc helpFunc)
         vnf = VNF_JitNew;
         break;
 
+    case CORINFO_HELP_READYTORUN_NEW:
+        vnf = VNF_JitReadyToRunNew;
+        break;
+
     case CORINFO_HELP_NEWARR_1_DIRECT:
     case CORINFO_HELP_NEWARR_1_OBJ:
     case CORINFO_HELP_NEWARR_1_VC:
@@ -6666,6 +6724,10 @@ VNFunc Compiler::fgValueNumberHelperMethVNFunc(CorInfoHelpFunc helpFunc)
         vnf = VNF_JitNewArr;
         break;
 
+    case CORINFO_HELP_READYTORUN_NEWARR_1:
+        vnf = VNF_JitReadyToRunNewArr;
+        break;
+
     case CORINFO_HELP_GETGENERICS_GCSTATIC_BASE:
         vnf = VNF_GetgenericsGcstaticBase; break;
     case CORINFO_HELP_GETGENERICS_NONGCSTATIC_BASE:
@@ -6678,6 +6740,8 @@ VNFunc Compiler::fgValueNumberHelperMethVNFunc(CorInfoHelpFunc helpFunc)
         vnf = VNF_GetsharedGcstaticBaseNoctor; break;
     case CORINFO_HELP_GETSHARED_NONGCSTATIC_BASE_NOCTOR:
         vnf = VNF_GetsharedNongcstaticBaseNoctor; break;
+    case CORINFO_HELP_READYTORUN_STATIC_BASE:
+        vnf = VNF_ReadyToRunStaticBase; break;
     case CORINFO_HELP_GETSHARED_GCSTATIC_BASE_DYNAMICCLASS:
         vnf = VNF_GetsharedGcstaticBaseDynamicclass; break;
     case CORINFO_HELP_GETSHARED_NONGCSTATIC_BASE_DYNAMICCLASS:
@@ -6722,12 +6786,19 @@ VNFunc Compiler::fgValueNumberHelperMethVNFunc(CorInfoHelpFunc helpFunc)
     case CORINFO_HELP_CHKCASTINTERFACE:
     case CORINFO_HELP_CHKCASTANY:
         vnf = VNF_CastClass; break;
+
+    case CORINFO_HELP_READYTORUN_CHKCAST:
+        vnf = VNF_ReadyToRunCastClass; break;
+
     case CORINFO_HELP_ISINSTANCEOFCLASS:
     case CORINFO_HELP_ISINSTANCEOFINTERFACE:
     case CORINFO_HELP_ISINSTANCEOFARRAY:
     case CORINFO_HELP_ISINSTANCEOFANY:
         vnf = VNF_IsInstanceOf; break;
 
+    case CORINFO_HELP_READYTORUN_ISINSTANCEOF:
+        vnf = VNF_ReadyToRunIsInstanceOf; break;
+
     case CORINFO_HELP_LDELEMA_REF:
         vnf = VNF_LdElemA; break;
 

diff --git a/src/jit/valuenum.h b/src/jit/valuenum.h
index 220af79..ab1bef4 100644 (file)
--- a/src/jit/valuenum.h
+++ b/src/jit/valuenum.h
@@ -584,7 +584,7 @@ public:
     };
 
     // Check if "vn" is "new [] (type handle, size)"
-    bool IsVNNewArr(ValueNum vn);
+    bool IsVNNewArr(ValueNum vn, VNFuncApp* funcApp);
 
     // Check if "vn" IsVNNewArr and return <= 0 if arr size cannot be determined, else array size.
     int GetNewArrSize(ValueNum vn);

diff --git a/src/jit/valuenumfuncs.h b/src/jit/valuenumfuncs.h
index 165ee4c..8d71fb1 100644 (file)
--- a/src/jit/valuenumfuncs.h
+++ b/src/jit/valuenumfuncs.h
@@ -30,7 +30,8 @@ ValueNumFuncDef(Cast, 2, false, false, false)               // VNF_Cast: Cast Op
 
 ValueNumFuncDef(CastClass, 2, false, false, false)          // Args: 0: Handle of class being cast to, 1: object being cast.
 ValueNumFuncDef(IsInstanceOf, 2, false, false, false)       // Args: 0: Handle of class being queried, 1: object being queried.
-
+ValueNumFuncDef(ReadyToRunCastClass, 2, false, false, false)          // Args: 0: Helper stub address, 1: object being cast.
+ValueNumFuncDef(ReadyToRunIsInstanceOf, 2, false, false, false)       // Args: 0: Helper stub address, 1: object being queried.
 
 ValueNumFuncDef(LdElemA, 3, false, false, false)            // Args: 0: array value; 1: index value; 2: type handle of element.
 
@@ -96,6 +97,7 @@ ValueNumFuncDef(GetsharedGcstaticBase, 2, false, true, true)
 ValueNumFuncDef(GetsharedNongcstaticBase, 2, false, true, true)
 ValueNumFuncDef(GetsharedGcstaticBaseNoctor, 1, false, true, true)
 ValueNumFuncDef(GetsharedNongcstaticBaseNoctor, 1, false, true, true)
+ValueNumFuncDef(ReadyToRunStaticBase, 1, false, true, true)
 ValueNumFuncDef(GetsharedGcstaticBaseDynamicclass, 2, false, true, true)
 ValueNumFuncDef(GetsharedNongcstaticBaseDynamicclass, 2, false, true, true)
 ValueNumFuncDef(GetgenericsGcthreadstaticBase, 1, false, true, true)
@@ -116,6 +118,8 @@ ValueNumFuncDef(GetStaticAddrTLS, 1, false, true, false)
 
 ValueNumFuncDef(JitNew, 2, false, true, false)
 ValueNumFuncDef(JitNewArr, 3, false, true, false)
+ValueNumFuncDef(JitReadyToRunNew, 2, false, true, false)
+ValueNumFuncDef(JitReadyToRunNewArr, 3, false, true, false)
 ValueNumFuncDef(BoxNullable, 3, false, false, false)
 
 ValueNumFuncDef(LT_UN, 2, false, false, false)

diff --git a/tests/src/readytorun/main.cs b/tests/src/readytorun/main.cs
index c68baa0..427ca42 100644 (file)
--- a/tests/src/readytorun/main.cs
+++ b/tests/src/readytorun/main.cs
@@ -249,6 +249,47 @@ class Program
         new MyClass().GetType().ToString();
     }
 
+    [MethodImplAttribute(MethodImplOptions.NoInlining)]
+    static void TestStaticBaseCSE()
+    {
+        // There should be just one call to CORINFO_HELP_READYTORUN_STATIC_BASE
+        // in the generated code.
+        s++;
+        s++;
+        Assert.AreEqual(s, 2);
+        s = 0;
+    }
+
+    [MethodImplAttribute(MethodImplOptions.NoInlining)]
+    static void TestIsInstCSE()
+    {
+        // There should be just one call to CORINFO_HELP_READYTORUN_ISINSTANCEOF
+        // in the generated code.
+        object o1 = (s < 1) ? (object)"foo" : (object)1;
+        Assert.AreEqual(o1 is string, true);
+        Assert.AreEqual(o1 is string, true);
+    }
+
+    [MethodImplAttribute(MethodImplOptions.NoInlining)]
+    static void TestCastClassCSE()
+    {
+        // There should be just one call to CORINFO_HELP_READYTORUN_CHKCAST
+        // in the generated code.
+        object o1 = (s < 1) ? (object)"foo" : (object)1;
+        string str1 = (string)o1;
+        string str2 = (string)o1;
+        Assert.AreEqual(str1, str2);
+    }
+
+    [MethodImplAttribute(MethodImplOptions.NoInlining)]
+    static void TestRangeCheckElimination()
+    {
+        // Range checks for array accesses should be eliminated by the compiler.
+        int[] array = new int[5];
+        array[2] = 2;
+        Assert.AreEqual(array[2], 2);
+    }
+
 #if CORECLR
     class MyLoadContext : AssemblyLoadContext
     {
@@ -331,6 +372,14 @@ class Program
 #endif
 
         TestFieldLayoutNGenMixAndMatch();
+
+        TestStaticBaseCSE();
+
+        TestIsInstCSE();
+
+        TestCastClassCSE();
+
+        TestRangeCheckElimination();
     }
 
     static int Main()
@@ -353,4 +402,6 @@ class Program
     }
 
     static bool LLILCJitEnabled;
+
+    static int s;
 }