Fix R2R fixups for delegate ctors
authorJan Kotas <jkotas@microsoft.com>
Thu, 31 Mar 2016 03:49:26 +0000 (20:49 -0700)
committerJan Kotas <jkotas@microsoft.com>
Thu, 31 Mar 2016 03:49:26 +0000 (20:49 -0700)
- Add getReadyToRunDelegateHelper to JIT-EE interface. This method has an extra argument describing the type of the delegate. This argument is required to emit correct fixups.
- Call the new method in both JIT and JIT32
- Add the new method to superpmi
- Bump minor version of R2R file format
- Add regression test for #3975 to R2R unit test

Fixes https://github.com/dotnet/coreclr/issues/3975

[tfs-changeset: 1591698]

13 files changed:
src/inc/corinfo.h
src/jit/flowgraph.cpp
src/jit/gentree.cpp [changed mode: 0755->0644]
src/jit/gentree.h
src/jit/importer.cpp
src/vm/jitinterface.cpp
src/vm/jitinterface.h
src/vm/win32threadpool.cpp
src/zap/zapimport.cpp
src/zap/zapimport.h
src/zap/zapinfo.cpp
src/zap/zapinfo.h
tests/src/readytorun/main.cs

index bb1a1c3..fb83999 100644 (file)
@@ -231,11 +231,11 @@ TODO: Talk about initializing strutures before use
 #if COR_JIT_EE_VERSION > 460
 
 // Update this one
-SELECTANY const GUID JITEEVersionIdentifier = { /* 35ef98ab-fd22-4ccc-8ddb-b1156a7d94f3 */
-    0x35ef98ab,
-    0xfd22,
-    0x4ccc,
-    { 0x8d, 0xdb, 0xb1, 0x15, 0x6a, 0x7d, 0x94, 0xf3 }
+SELECTANY const GUID JITEEVersionIdentifier = { /* 27626524-7315-4ed0-b74e-a0e4579883bb */
+    0x27626524, 
+    0x7315, 
+    0x4ed0, 
+    { 0xb7, 0x4e, 0xa0, 0xe4, 0x57, 0x98, 0x83, 0xbb }
 };
 
 #else
@@ -2390,6 +2390,14 @@ public:
             CORINFO_CONST_LOOKUP *   pLookup
             ) = 0;
 
+#if COR_JIT_EE_VERSION > 460
+    virtual void getReadyToRunDelegateCtorHelper(
+            CORINFO_RESOLVED_TOKEN * pTargetMethod,
+            CORINFO_CLASS_HANDLE     delegateType,
+            CORINFO_CONST_LOOKUP *   pLookup
+            ) = 0;
+#endif
+
     virtual const char* getHelperName(
             CorInfoHelpFunc
             ) = 0;
index d075401..4e3ba86 100644 (file)
@@ -6924,7 +6924,12 @@ GenTreePtr    Compiler::fgOptimizeDelegateConstructor(GenTreePtr call, CORINFO_C
                 helperArgs->gtOp.gtOp2 = gtNewArgList(call->gtCall.gtCallArgs->gtOp.gtOp1);
 
                 call = gtNewHelperCallNode(CORINFO_HELP_READYTORUN_DELEGATE_CTOR, TYP_VOID, GTF_EXCEPT, helperArgs);
-                call->gtCall.gtEntryPoint = targetMethod->gtFptrVal.gtDelegateCtor;
+#if COR_JIT_EE_VERSION > 460
+                info.compCompHnd->getReadyToRunDelegateCtorHelper(targetMethod->gtFptrVal.gtLdftnResolvedToken, clsHnd, &call->gtCall.gtEntryPoint);
+#else
+                info.compCompHnd->getReadyToRunHelper(targetMethod->gtFptrVal.gtLdftnResolvedToken,
+                    CORINFO_HELP_READYTORUN_DELEGATE_CTOR, &call->gtCall.gtEntryPoint);
+#endif
             }
         }
         else
old mode 100755 (executable)
new mode 100644 (file)
index cad8417..a0ce79b
@@ -5954,7 +5954,7 @@ GenTreePtr          Compiler::gtCloneExpr(GenTree * tree,
 
 #ifdef FEATURE_READYTORUN_COMPILER
             copy->gtFptrVal.gtEntryPoint = tree->gtFptrVal.gtEntryPoint;
-            copy->gtFptrVal.gtDelegateCtor = tree->gtFptrVal.gtDelegateCtor;
+            copy->gtFptrVal.gtLdftnResolvedToken = tree->gtFptrVal.gtLdftnResolvedToken;
 #endif
             goto DONE;
 
index 631f67b..d95c473 100644 (file)
@@ -2547,7 +2547,7 @@ struct GenTreeFptrVal: public GenTree
 
 #ifdef FEATURE_READYTORUN_COMPILER
     CORINFO_CONST_LOOKUP gtEntryPoint;
-    CORINFO_CONST_LOOKUP gtDelegateCtor;
+    CORINFO_RESOLVED_TOKEN* gtLdftnResolvedToken;
 #endif
 
     GenTreeFptrVal(var_types type, CORINFO_METHOD_HANDLE meth) : 
index 7993918..558fc1f 100644 (file)
@@ -1663,10 +1663,8 @@ GenTreePtr Compiler::impMethodPointer(CORINFO_RESOLVED_TOKEN * pResolvedToken, C
         if (opts.IsReadyToRun())
         {
             op1->gtFptrVal.gtEntryPoint = pCallInfo->codePointerLookup.constLookup;
-
-            // In almost all cases, we are going to create the delegate out of the function pointer. While we are here,
-            // get the pointer to the optimized delegate helper. Only one of the two is going to be embedded into the code.
-            info.compCompHnd->getReadyToRunHelper(pResolvedToken, CORINFO_HELP_READYTORUN_DELEGATE_CTOR, &op1->gtFptrVal.gtDelegateCtor);
+            op1->gtFptrVal.gtLdftnResolvedToken = new(this, CMK_Unknown) CORINFO_RESOLVED_TOKEN;
+            *op1->gtFptrVal.gtLdftnResolvedToken = *pResolvedToken;
         }
         else
             op1->gtFptrVal.gtEntryPoint.addr = nullptr;
index aabc658..4654248 100644 (file)
@@ -6092,6 +6092,17 @@ void CEEInfo::getReadyToRunHelper(
 }
 
 /***********************************************************************/
+void CEEInfo::getReadyToRunDelegateCtorHelper(
+        CORINFO_RESOLVED_TOKEN * pTargetMethod,
+        CORINFO_CLASS_HANDLE     delegateType,
+        CORINFO_CONST_LOOKUP *   pLookup
+        )
+{
+    LIMITED_METHOD_CONTRACT;
+    UNREACHABLE();      // only called during NGen
+}
+
+/***********************************************************************/
 // see code:Nullable#NullableVerification
 
 CORINFO_CLASS_HANDLE  CEEInfo::getTypeForBox(CORINFO_CLASS_HANDLE  cls)
index 215ceda..00edc6e 100644 (file)
@@ -538,6 +538,12 @@ public:
             CORINFO_CONST_LOOKUP *   pLookup
             );
 
+    void getReadyToRunDelegateCtorHelper(
+            CORINFO_RESOLVED_TOKEN * pTargetMethod,
+            CORINFO_CLASS_HANDLE     delegateType,
+            CORINFO_CONST_LOOKUP *   pLookup
+            );
+
     CorInfoInitClassResult initClass(
             CORINFO_FIELD_HANDLE    field,
             CORINFO_METHOD_HANDLE   method,
index af7b26c..fc25f4e 100644 (file)
@@ -305,11 +305,13 @@ DWORD GetDefaultMaxLimitWorkerThreads(DWORD minLimit)
 
 DWORD GetForceMinWorkerThreadsValue()
 {
+    WRAPPER_NO_CONTRACT;
     return Configuration::GetKnobDWORDValue(W("System.Threading.ThreadPool.MinThreads"), CLRConfig::INTERNAL_ThreadPool_ForceMinWorkerThreads);
 }
 
 DWORD GetForceMaxWorkerThreadsValue()
 {
+    WRAPPER_NO_CONTRACT;
     return Configuration::GetKnobDWORDValue(W("System.Threading.ThreadPool.MaxThreads"), CLRConfig::INTERNAL_ThreadPool_ForceMaxWorkerThreads);
 }
 
index c1947a9..4e7b151 100644 (file)
@@ -1791,13 +1791,20 @@ ZapImport * ZapImportTable::GetDynamicHelperCell(CORCOMPILE_FIXUP_BLOB_KIND kind
     return pImport;
 }
 
-ZapImport * ZapImportTable::GetDynamicHelperCell(CORCOMPILE_FIXUP_BLOB_KIND kind, CORINFO_METHOD_HANDLE handle, CORINFO_RESOLVED_TOKEN * pResolvedToken)
+ZapImport * ZapImportTable::GetDynamicHelperCell(CORCOMPILE_FIXUP_BLOB_KIND kind, CORINFO_METHOD_HANDLE handle, CORINFO_RESOLVED_TOKEN * pResolvedToken, 
+    CORINFO_CLASS_HANDLE delegateType /*=NULL*/)
 {
     SigBuilder sigBuilder;
 
     EncodeMethod((CORCOMPILE_FIXUP_BLOB_KIND)(kind & ~CORINFO_HELP_READYTORUN_ATYPICAL_CALLSITE),
         handle, &sigBuilder, pResolvedToken);
 
+    if (delegateType != NULL)
+    {
+        _ASSERTE((CORCOMPILE_FIXUP_BLOB_KIND)(kind & ~CORINFO_HELP_READYTORUN_ATYPICAL_CALLSITE) == ENCODE_DELEGATE_CTOR);
+        GetCompileInfo()->EncodeClass(m_pImage->GetModuleHandle(), delegateType, &sigBuilder, NULL, NULL);
+    }
+
     return GetImportForSignature<ZapDynamicHelperCell, ZapNodeType_DynamicHelperCell>((void *)kind, &sigBuilder);
 }
 
index 9d55e03..9aa5e95 100644 (file)
@@ -432,7 +432,8 @@ public:
     ZapImport * GetExternalMethodCell(CORINFO_METHOD_HANDLE handle, CORINFO_RESOLVED_TOKEN * pResolvedToken, CORINFO_RESOLVED_TOKEN * pConstrainedResolvedToken);
 
     ZapImport * GetDynamicHelperCell(CORCOMPILE_FIXUP_BLOB_KIND kind, CORINFO_CLASS_HANDLE handle);
-    ZapImport * GetDynamicHelperCell(CORCOMPILE_FIXUP_BLOB_KIND kind, CORINFO_METHOD_HANDLE handle, CORINFO_RESOLVED_TOKEN * pResolvedToken);
+    ZapImport * GetDynamicHelperCell(CORCOMPILE_FIXUP_BLOB_KIND kind, CORINFO_METHOD_HANDLE handle, CORINFO_RESOLVED_TOKEN * pResolvedToken, 
+                    CORINFO_CLASS_HANDLE delegateType = NULL);
     ZapImport * GetDynamicHelperCell(CORCOMPILE_FIXUP_BLOB_KIND kind, CORINFO_FIELD_HANDLE handle, CORINFO_RESOLVED_TOKEN * pResolvedToken);
 
 #ifdef FEATURE_READYTORUN_COMPILER
index c8a08a3..cf8e9b7 100644 (file)
@@ -2667,7 +2667,7 @@ CORINFO_METHOD_HANDLE ZapInfo::GetDelegateCtor(CORINFO_METHOD_HANDLE   methHnd,
                                                CORINFO_METHOD_HANDLE   targetMethodHnd,
                                                DelegateCtorArgs *      pCtorData)
 {
-    // For ReadyToRun, this optimization is done via ZapInfo::getReadyToRunHelper
+    // For ReadyToRun, this optimization is done via ZapInfo::getReadyToRunDelegateCtorHelper
     if (IsReadyToRunCompilation())
         return methHnd;
 
@@ -3410,11 +3410,6 @@ void ZapInfo::getReadyToRunHelper(
         }
         break;
 
-    case CORINFO_HELP_READYTORUN_DELEGATE_CTOR:
-        pImport = m_pImage->GetImportTable()->GetDynamicHelperCell(
-            (CORCOMPILE_FIXUP_BLOB_KIND)(ENCODE_DELEGATE_CTOR | fAtypicalCallsite), pResolvedToken->hMethod, pResolvedToken);
-        break;
-
     default:
         _ASSERTE(false);
         ThrowHR(E_NOTIMPL);
@@ -3425,6 +3420,21 @@ void ZapInfo::getReadyToRunHelper(
 #endif
 }
 
+void ZapInfo::getReadyToRunDelegateCtorHelper(
+        CORINFO_RESOLVED_TOKEN * pTargetMethod,
+        CORINFO_CLASS_HANDLE     delegateType,
+        CORINFO_CONST_LOOKUP *   pLookup
+        )
+{
+#ifdef FEATURE_READYTORUN_COMPILER
+    _ASSERTE(IsReadyToRunCompilation());
+
+    pLookup->accessType = IAT_PVALUE;
+    pLookup->addr = m_pImage->GetImportTable()->GetDynamicHelperCell(
+            (CORCOMPILE_FIXUP_BLOB_KIND)(ENCODE_DELEGATE_CTOR), pTargetMethod->hMethod, pTargetMethod, delegateType);
+#endif
+}
+
 
 //
 // ICorModuleInfo
index 83528e9..5fe3dcb 100644 (file)
@@ -550,6 +550,12 @@ public:
             CORINFO_CONST_LOOKUP *   pLookup
             );
 
+    void getReadyToRunDelegateCtorHelper(
+            CORINFO_RESOLVED_TOKEN * pTargetMethod,
+            CORINFO_CLASS_HANDLE     delegateType,
+            CORINFO_CONST_LOOKUP *   pLookup
+            );
+
     CorInfoInitClassResult initClass(
             CORINFO_FIELD_HANDLE    field,
             CORINFO_METHOD_HANDLE   method,
index 427ca42..70157bc 100644 (file)
@@ -40,6 +40,15 @@ class InheritingFromGrowingBase : GrowingBase
     public int x;
 }
 
+
+static class OpenClosedDelegateExtension
+{
+    public static string OpenClosedDelegateTarget(this string x, string foo)
+    {
+        return x + ", " + foo;
+    }
+}
+
 class Program
 {
     static void TestVirtualMethodCalls()
@@ -335,6 +344,18 @@ class Program
         Assert.AreEqual(o.ChildByte, (byte)67);
     }
 
+    static void TestOpenClosedDelegate()
+    {
+        // This test is verifying the the fixups for open vs. closed delegate created against the same target
+        // method are encoded correctly.
+
+        Func<string, string, object> idOpen = OpenClosedDelegateExtension.OpenClosedDelegateTarget;
+        Assert.AreEqual(idOpen("World", "foo"), "World, foo");
+
+        Func<string, object> idClosed = "World".OpenClosedDelegateTarget;
+        Assert.AreEqual(idClosed("hey"), "World, hey");
+    }
+
     static void RunAllTests()
     {
         TestVirtualMethodCalls();
@@ -380,6 +401,8 @@ class Program
         TestCastClassCSE();
 
         TestRangeCheckElimination();
+
+        TestOpenClosedDelegate();
     }
 
     static int Main()