Skip to content
This repository has been archived by the owner on Nov 1, 2020. It is now read-only.

Implement ldvirtftn in shared generic contexts #2339

Merged
merged 3 commits into from
Dec 15, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,8 @@ public override IEnumerable<CombinedDependencyListEntry> GetConditionalStaticDep
// that use the same dictionary layout.
foreach (var method in _owningType.GetAllMethods())
{
// Static and generic methods have their own generic dictionaries
if (method.Signature.IsStatic || method.HasInstantiation)
// Generic methods have their own generic dictionaries
if (method.HasInstantiation)
continue;

// Abstract methods don't have a body
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,11 @@ internal sealed class VirtualDispatchGenericLookupResult : GenericLookupResult
public VirtualDispatchGenericLookupResult(MethodDesc method)
{
Debug.Assert(method.IsRuntimeDeterminedExactMethod);
Debug.Assert(method.IsVirtual);

// Normal virtual methods don't need a generic lookup.
Debug.Assert(method.OwningType.IsInterface || method.HasInstantiation);

_method = method;
}

Expand All @@ -146,6 +151,42 @@ public override void AppendMangledName(NameMangler nameMangler, Utf8StringBuilde
public override string ToString() => $"VirtualCall: {_method}";
}

/// <summary>
/// Generic lookup result that points to a virtual function address load stub.
/// </summary>
internal sealed class VirtualResolveGenericLookupResult : GenericLookupResult
{
private MethodDesc _method;

public VirtualResolveGenericLookupResult(MethodDesc method)
{
Debug.Assert(method.IsRuntimeDeterminedExactMethod);
Debug.Assert(method.IsVirtual);

// Normal virtual methods don't need a generic lookup.
Debug.Assert(method.OwningType.IsInterface || method.HasInstantiation);

_method = method;
}

public override ISymbolNode GetTarget(NodeFactory factory, Instantiation typeInstantiation, Instantiation methodInstantiation)
{
MethodDesc instantiatedMethod = _method.InstantiateSignature(typeInstantiation, methodInstantiation);

// https://github.com/dotnet/corert/issues/2342 - we put a pointer to the virtual call helper into the dictionary
// but this should be something that will let us compute the target of the dipatch (e.g. interface dispatch cell).
return factory.ReadyToRunHelper(ReadyToRunHelperId.VirtualCall, instantiatedMethod);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put in a comment describing why this shouldn't be here... Something like, the function pointer returned should be the result of the virtual dispatch, not a pointer to the virtual dispatcher.

}

public override void AppendMangledName(NameMangler nameMangler, Utf8StringBuilder sb)
{
sb.Append("VirtualResolve_");
sb.Append(nameMangler.GetMangledMethodName(_method));
}

public override string ToString() => $"VirtualResolve: {_method}";
}

/// <summary>
/// Generic lookup result that points to the non-GC static base of a type.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@ private void CreateNodeCaches()
return new VirtualDispatchGenericLookupResult(method);
});

_virtualResolveHelpers = new NodeCache<MethodDesc, GenericLookupResult>(method =>
{
return new VirtualResolveGenericLookupResult(method);
});

_typeGCStaticBaseSymbols = new NodeCache<TypeDesc, GenericLookupResult>(type =>
{
return new TypeGCStaticBaseGenericLookupResult(type);
Expand Down Expand Up @@ -88,6 +93,13 @@ public GenericLookupResult VirtualCall(MethodDesc method)
return _virtualCallHelpers.GetOrAdd(method);
}

private NodeCache<MethodDesc, GenericLookupResult> _virtualResolveHelpers;

public GenericLookupResult VirtualMethodAddress(MethodDesc method)
{
return _virtualResolveHelpers.GetOrAdd(method);
}

private NodeCache<MethodDesc, GenericLookupResult> _methodEntrypoints;

public GenericLookupResult MethodEntry(MethodDesc method)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ private static GenericLookupResult GetLookupSignature(NodeFactory factory, Ready
return factory.GenericLookup.MethodDictionary((MethodDesc)target);
case ReadyToRunHelperId.VirtualCall:
return factory.GenericLookup.VirtualCall((MethodDesc)target);
case ReadyToRunHelperId.ResolveVirtualFunction:
return factory.GenericLookup.VirtualMethodAddress((MethodDesc)target);
case ReadyToRunHelperId.MethodEntry:
return factory.GenericLookup.MethodEntry((MethodDesc)target);
default:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ protected sealed override void EmitCode(NodeFactory factory, ref X64Emitter enco
case ReadyToRunHelperId.TypeHandle:
case ReadyToRunHelperId.MethodDictionary:
case ReadyToRunHelperId.VirtualCall:
case ReadyToRunHelperId.ResolveVirtualFunction:
case ReadyToRunHelperId.MethodEntry:
{
EmitDictionaryLookup(factory, ref encoder, encoder.TargetRegister.Arg0, encoder.TargetRegister.Result, _lookupSignature, relocsOnly);
Expand Down
43 changes: 37 additions & 6 deletions src/JitInterface/src/CorInfoImpl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2683,13 +2683,39 @@ private void getCallInfo(ref CORINFO_RESOLVED_TOKEN pResolvedToken, CORINFO_RESO
(void*)ObjectToHandle(_compilation.NodeFactory.ReadyToRunHelper(ReadyToRunHelperId.ResolveGenericVirtualMethod, targetMethod));
pResult.nullInstanceCheck = false;
}
else if ((flags & CORINFO_CALLINFO_FLAGS.CORINFO_CALLINFO_LDFTN) != 0)
else if((flags & CORINFO_CALLINFO_FLAGS.CORINFO_CALLINFO_LDFTN) != 0)
{
pResult.kind = CORINFO_CALL_KIND.CORINFO_VIRTUALCALL_LDVIRTFTN;
pResult.codePointerOrStubLookup.constLookup.accessType = InfoAccessType.IAT_VALUE;

pResult.codePointerOrStubLookup.constLookup.addr =
(void*)ObjectToHandle(_compilation.NodeFactory.ReadyToRunHelper(ReadyToRunHelperId.ResolveVirtualFunction, targetMethod));
// If this is a non-interface/non-GVM call, we actually don't need a runtime lookup to find the target.
// We don't even need to keep track of the runtime-determined method being called because the system ensures
// that if e.g. Foo<__Canon>.GetHashCode is needed and we're generating a dictionary for Foo<string>,
// Foo<string>.GetHashCode is needed too.
if (pResult.exactContextNeedsRuntimeLookup &&
(targetMethod.HasInstantiation || targetMethod.OwningType.IsInterface))
{
pResult.codePointerOrStubLookup.lookupKind.needsRuntimeLookup = true;
pResult.codePointerOrStubLookup.runtimeLookup.indirections = CORINFO.USEHELPER;

// Do not bother computing the runtime lookup if we are inlining. The JIT is going
// to abort the inlining attempt anyway.
MethodDesc contextMethod = methodFromContext(pResolvedToken.tokenContext);
if (contextMethod != MethodBeingCompiled)
{
return;
}

pResult.codePointerOrStubLookup.lookupKind.runtimeLookupKind = GetGenericRuntimeLookupKind(contextMethod);
pResult.codePointerOrStubLookup.lookupKind.runtimeLookupFlags = (ushort)ReadyToRunHelperId.ResolveVirtualFunction;
}
else
{
pResult.exactContextNeedsRuntimeLookup = false;
pResult.codePointerOrStubLookup.constLookup.accessType = InfoAccessType.IAT_VALUE;

pResult.codePointerOrStubLookup.constLookup.addr =
(void*)ObjectToHandle(_compilation.NodeFactory.ReadyToRunHelper(ReadyToRunHelperId.ResolveVirtualFunction, targetMethod));
}

// The current CoreRT ReadyToRun helpers do not handle null thisptr - ask the JIT to emit explicit null checks
// TODO: Optimize this
Expand All @@ -2701,10 +2727,14 @@ private void getCallInfo(ref CORINFO_RESOLVED_TOKEN pResolvedToken, CORINFO_RESO
// call that should not be inlined.
pResult.kind = CORINFO_CALL_KIND.CORINFO_CALL_CODE_POINTER;

if (pResult.exactContextNeedsRuntimeLookup)
// If this is a non-interface/non-GVM call, we actually don't need a runtime lookup to find the target.
// We don't even need to keep track of the runtime-determined method being called because the system ensures
// that if e.g. Foo<__Canon>.GetHashCode is needed and we're generating a dictionary for Foo<string>,
// Foo<string>.GetHashCode is needed too.
if (pResult.exactContextNeedsRuntimeLookup &&
(targetMethod.HasInstantiation || targetMethod.OwningType.IsInterface))
{
pResult.codePointerOrStubLookup.lookupKind.needsRuntimeLookup = true;
pResult.codePointerOrStubLookup.lookupKind.runtimeLookupFlags = 0;
pResult.codePointerOrStubLookup.runtimeLookup.indirections = CORINFO.USEHELPER;

// Do not bother computing the runtime lookup if we are inlining. The JIT is going
Expand All @@ -2720,6 +2750,7 @@ private void getCallInfo(ref CORINFO_RESOLVED_TOKEN pResolvedToken, CORINFO_RESO
}
else
{
pResult.exactContextNeedsRuntimeLookup = false;
pResult.codePointerOrStubLookup.constLookup.accessType = InfoAccessType.IAT_VALUE;

pResult.codePointerOrStubLookup.constLookup.addr =
Expand Down
59 changes: 59 additions & 0 deletions tests/src/Simple/Generics/Generics.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ static int Main()
TestDelegateFatFunctionPointers.Run();
TestVirtualMethodUseTracking.Run();
TestSlotsInHierarchy.Run();
TestDelegateVirtualMethod.Run();
TestDelegateInterfaceMethod.Run();
TestNameManglingCollisionRegression.Run();
TestUnusedGVMsDoNotCrashCompiler.Run();

Expand Down Expand Up @@ -142,6 +144,63 @@ public static void Run()
}
}

class TestDelegateVirtualMethod
{
static void Generic<T>()
{
Base<T> o = new Derived<T>();
Func<string> f = o.Do;
if (f() != "Derived")
throw new Exception();

o = new Base<T>();
f = o.Do;
if (f() != "Base")
throw new Exception();
}

public static void Run()
{
Generic<string>();
}

class Base<T>
{
public virtual string Do() => "Base";
}

class Derived<T> : Base<T>
{
public override string Do() => "Derived";
}
}

class TestDelegateInterfaceMethod
{
static void Generic<T>()
{
IFoo<T> o = new Foo<T>();
Func<string> f = o.Do;
if (f() != "Foo")
throw new Exception();
}

public static void Run()
{
Generic<string>();
}

interface IFoo<T>
{
string Do();
}

class Foo<T> : IFoo<T>
{
public string Do() => "Foo";
}
}

/// <summary>
/// Tests RyuJIT's initThisClass.
/// </summary>
Expand Down