Skip to content

Commit

Permalink
Do not allow generic inlining of delegate creation (dotnet#102299)
Browse files Browse the repository at this point in the history
Fixes dotnet#102259.

Delegate creation requires form of lookups that are hard to express across inlined generics.

This was introduced when we started doing more generic inlining in dotnet#99265. I [suspected](dotnet#99265 (comment)) things might not be correct here but assumed we'd have test coverage for sure. _Now_ we have test coverage.
  • Loading branch information
MichalStrehovsky authored and Ruihan-Yin committed May 30, 2024
1 parent cebf46f commit fa3a3cf
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 12 deletions.
28 changes: 20 additions & 8 deletions src/coreclr/jit/flowgraph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1061,14 +1061,26 @@ GenTree* Compiler::fgOptimizeDelegateConstructor(GenTreeCall* call,
else
{
assert(oper != GT_FTN_ADDR);
CORINFO_CONST_LOOKUP genericLookup;
info.compCompHnd->getReadyToRunHelper(&ldftnToken->m_token, &pLookup.lookupKind,
CORINFO_HELP_READYTORUN_GENERIC_HANDLE, info.compMethodHnd,
&genericLookup);
GenTree* ctxTree = getRuntimeContextTree(pLookup.lookupKind.runtimeLookupKind);
call = gtNewHelperCallNode(CORINFO_HELP_READYTORUN_DELEGATE_CTOR, TYP_VOID, thisPointer,
targetObjPointers, ctxTree);
call->setEntryPoint(genericLookup);

if (pLookup.lookupKind.runtimeLookupKind != CORINFO_LOOKUP_NOT_SUPPORTED)
{
CORINFO_CONST_LOOKUP genericLookup;
info.compCompHnd->getReadyToRunHelper(&ldftnToken->m_token, &pLookup.lookupKind,
CORINFO_HELP_READYTORUN_GENERIC_HANDLE,
info.compMethodHnd, &genericLookup);
GenTree* ctxTree = getRuntimeContextTree(pLookup.lookupKind.runtimeLookupKind);
call = gtNewHelperCallNode(CORINFO_HELP_READYTORUN_DELEGATE_CTOR, TYP_VOID, thisPointer,
targetObjPointers, ctxTree);
call->setEntryPoint(genericLookup);
}
else
{
// Runtime does not support inlining of all shapes of runtime lookups
// Inlining has to be aborted in such a case
assert(compIsForInlining());
compInlineResult->NoteFatal(InlineObservation::CALLSITE_GENERIC_DICTIONARY_LOOKUP);
JITDUMP("not optimized, generic inlining restriction\n");
}
}
}
else
Expand Down
4 changes: 4 additions & 0 deletions src/coreclr/jit/importercalls.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1005,6 +1005,10 @@ var_types Compiler::impImportCall(OPCODE opcode,
// New inliner morph it in impImportCall.
// This will allow us to inline the call to the delegate constructor.
call = fgOptimizeDelegateConstructor(call->AsCall(), &exactContextHnd, ldftnInfo);
if (compDonotInline())
{
return TYP_UNDEF;
}
}

if (!bIntrinsicImported)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -436,10 +436,17 @@ private void getReadyToRunDelegateCtorHelper(ref CORINFO_RESOLVED_TOKEN pTargetM
{
pLookup.lookupKind.needsRuntimeLookup = true;

MethodDesc contextMethod = HandleToObject(callerHandle);
pLookup.lookupKind.runtimeLookupKind = GetGenericRuntimeLookupKind(contextMethod);
pLookup.lookupKind.runtimeLookupFlags = (ushort)ReadyToRunHelperId.DelegateCtor;
pLookup.lookupKind.runtimeLookupArgs = (void*)ObjectToHandle(delegateInfo);
if (pTargetMethod.tokenContext != contextFromMethodBeingCompiled())
{
pLookup.lookupKind.runtimeLookupKind = CORINFO_RUNTIME_LOOKUP_KIND.CORINFO_LOOKUP_NOT_SUPPORTED;
}
else
{
MethodDesc contextMethod = HandleToObject(callerHandle);
pLookup.lookupKind.runtimeLookupKind = GetGenericRuntimeLookupKind(contextMethod);
pLookup.lookupKind.runtimeLookupFlags = (ushort)ReadyToRunHelperId.DelegateCtor;
pLookup.lookupKind.runtimeLookupArgs = (void*)ObjectToHandle(delegateInfo);
}
}
else
{
Expand Down
34 changes: 34 additions & 0 deletions src/tests/nativeaot/SmokeTests/UnitTests/Generics.cs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ internal static int Run()
TestRecursionInFields.Run();
TestGvmLookupDependency.Run();
Test99198Regression.Run();
Test102259Regression.Run();
TestInvokeMemberCornerCaseInGenerics.Run();
TestRefAny.Run();
TestNullableCasting.Run();
Expand Down Expand Up @@ -3555,6 +3556,26 @@ public static void Run()
}
}

class Test102259Regression
{
class Gen<T>
{
static Func<T, object> _theval;

public static object TheFunc(object obj) => obj;

static Gen()
{
_theval = typeof(Gen<T>).GetMethod(nameof(TheFunc), BindingFlags.Public | BindingFlags.Static).CreateDelegate<Func<T, object>>().WithObjectTResult();
}
}

public static void Run()
{
new Gen<object>();
}
}

class TestInvokeMemberCornerCaseInGenerics
{
class Generic<T>
Expand Down Expand Up @@ -3601,3 +3622,16 @@ public static void Run()
}
}
}

static class Ext
{
public static Func<T, object> WithObjectTResult<T, TResult>(this Func<T, TResult> function)
{
return function.InvokeWithObjectTResult;
}

private static object InvokeWithObjectTResult<T, TResult>(this Func<T, TResult> func, T arg)
{
return func(arg);
}
}

0 comments on commit fa3a3cf

Please sign in to comment.