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 1 commit
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 @@ -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,39 @@ 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);
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
42 changes: 26 additions & 16 deletions src/JitInterface/src/CorInfoImpl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2683,23 +2683,33 @@ 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
Copy link
Member

Choose a reason for hiding this comment

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

This changes the structure to fold together the virtual and non-virtual dispatch paths. While that is a simplification for now(since we only have R2R codegen), it is not in touch with longer term direction where we'll want to have more optimized codegen that handles the various cases in the most optimal form.

{
pResult.kind = CORINFO_CALL_KIND.CORINFO_VIRTUALCALL_LDVIRTFTN;
pResult.codePointerOrStubLookup.constLookup.accessType = InfoAccessType.IAT_VALUE;
ReadyToRunHelperId helper;

pResult.codePointerOrStubLookup.constLookup.addr =
(void*)ObjectToHandle(_compilation.NodeFactory.ReadyToRunHelper(ReadyToRunHelperId.ResolveVirtualFunction, targetMethod));
if ((flags & CORINFO_CALLINFO_FLAGS.CORINFO_CALLINFO_LDFTN) != 0)
{
pResult.kind = CORINFO_CALL_KIND.CORINFO_VIRTUALCALL_LDVIRTFTN;
helper = ReadyToRunHelperId.ResolveVirtualFunction;
}
else
{
// CORINFO_CALL_CODE_POINTER tells the JIT that this is indirect
// call that should not be inlined.
pResult.kind = CORINFO_CALL_KIND.CORINFO_CALL_CODE_POINTER;
helper = ReadyToRunHelperId.VirtualCall;
}

// The current CoreRT ReadyToRun helpers do not handle null thisptr - ask the JIT to emit explicit null checks
// TODO: Optimize this
pResult.nullInstanceCheck = true;
}
else
{
// CORINFO_CALL_CODE_POINTER tells the JIT that this is indirect
// call that should not be inlined.
pResult.kind = CORINFO_CALL_KIND.CORINFO_CALL_CODE_POINTER;
// 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.exactContextNeedsRuntimeLookup = false;
}

if (pResult.exactContextNeedsRuntimeLookup)
{
Expand All @@ -2716,14 +2726,14 @@ private void getCallInfo(ref CORINFO_RESOLVED_TOKEN pResolvedToken, CORINFO_RESO
}

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

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

// The current CoreRT ReadyToRun helpers do not handle null thisptr - ask the JIT to emit explicit null checks
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