From ab57fc7aa1b814a9d85503204d4b3b2fd51de63e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Mon, 23 Jan 2017 18:14:55 -0800 Subject: [PATCH 1/6] Add instantiating unboxing stubs To support calling canonical interface methods on generic valuetypes, the compiler needs to generate unboxing+instantiating thunks that bridge the difference between two calling conventions. As a refresher: * Instance methods on shared generic valuetypes expect two arguments (aside from the arguments declared in the signature): a ByRef to the first byte of the value of the valuetype (`this`), and a generic context argument (EEType) * Interface calls expect `this` to be a reference type (with the generic context to be inferred from `this` by the callee) Instantiating and unboxing stubs bridge this by extracting a managed pointer out of a boxed valuetype, along with the EEType of the boxed valuetype (to provide the generic context) before dispatching to the instance method. We compile them by: * Pretending the unboxing stub is an instance method on a reference type with the same layout as a boxed valuetype * Having the unboxing stub load the `m_pEEType` field (to get generic context) and a byref to the actual value (to get a `this` expected by valuetype methods) * Generating a call to the instance method on the valuetype through a wrapper that has an explicit generic context parameter in it's signature. --- .../TypeSystem/Common/TypeSystemHelpers.cs | 34 +- .../CompilerTypeSystemContext.BoxedTypes.cs | 396 ++++++++++++++++++ .../DependencyAnalysis/RyuJitNodeFactory.cs | 19 +- .../src/ILCompiler.Compiler.csproj | 1 + src/JitInterface/src/CorInfoImpl.cs | 2 +- 5 files changed, 444 insertions(+), 8 deletions(-) create mode 100644 src/ILCompiler.Compiler/src/Compiler/CompilerTypeSystemContext.BoxedTypes.cs diff --git a/src/Common/src/TypeSystem/Common/TypeSystemHelpers.cs b/src/Common/src/TypeSystem/Common/TypeSystemHelpers.cs index a3ccbf9b8d4..9e1d16df5e6 100644 --- a/src/Common/src/TypeSystem/Common/TypeSystemHelpers.cs +++ b/src/Common/src/TypeSystem/Common/TypeSystemHelpers.cs @@ -266,11 +266,16 @@ public static MethodDesc FindVirtualFunctionTargetMethodOnObjectType(this TypeDe } /// - /// Given Foo<T>, returns Foo<!0>. + /// Creates an open instantiation of a type. Given Foo<T>, returns Foo<!0>. + /// If the type is not generic, returns the . /// - private static InstantiatedType InstantiateAsOpen(this MetadataType type) + public static TypeDesc InstantiateAsOpen(this TypeDesc type) { - Debug.Assert(type.IsGenericDefinition); + if (!type.IsGenericDefinition) + { + Debug.Assert(!type.HasInstantiation); + return type; + } TypeSystemContext context = type.Context; @@ -280,7 +285,26 @@ private static InstantiatedType InstantiateAsOpen(this MetadataType type) inst[i] = context.GetSignatureVariable(i, false); } - return context.GetInstantiatedType(type, new Instantiation(inst)); + return context.GetInstantiatedType((MetadataType)type, new Instantiation(inst)); + } + + /// + /// Creates an open instantiation of a field. Given Foo<T>.Field, returns + /// Foo<!0>.Field. If the owning type is not generic, returns the . + /// + public static FieldDesc InstantiateAsOpen(this FieldDesc field) + { + Debug.Assert(field.GetTypicalFieldDefinition() == field); + + TypeDesc owner = field.OwningType; + + if (owner.HasInstantiation) + { + var instantiatedOwner = (InstantiatedType)owner.InstantiateAsOpen(); + return field.Context.GetFieldForInstantiatedType(field, instantiatedOwner); + } + + return field; } /// @@ -295,7 +319,7 @@ public static MethodDesc InstantiateAsOpen(this MethodDesc method) if (owner.HasInstantiation) { - MetadataType instantiatedOwner = ((MetadataType)owner).InstantiateAsOpen(); + MetadataType instantiatedOwner = (MetadataType)owner.InstantiateAsOpen(); return method.Context.GetMethodForInstantiatedType(method, (InstantiatedType)instantiatedOwner); } diff --git a/src/ILCompiler.Compiler/src/Compiler/CompilerTypeSystemContext.BoxedTypes.cs b/src/ILCompiler.Compiler/src/Compiler/CompilerTypeSystemContext.BoxedTypes.cs new file mode 100644 index 00000000000..cb5d743d2ab --- /dev/null +++ b/src/ILCompiler.Compiler/src/Compiler/CompilerTypeSystemContext.BoxedTypes.cs @@ -0,0 +1,396 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Collections.Generic; + +using Internal.TypeSystem; +using Internal.IL; +using Internal.IL.Stubs; + +using Debug = System.Diagnostics.Debug; +using System.Text; + +namespace ILCompiler + +{ + // Contains functionality related to pseudotypes representing boxed instances of value types + partial class CompilerTypeSystemContext + { + public MethodDesc GetSpecialUnboxingThunk(MethodDesc targetMethod, ModuleDesc ownerModuleOfThunk) + { + TypeDesc owningType = targetMethod.OwningType; + var owningTypeDefinition = (MetadataType)owningType.GetTypeDefinition(); + + var typeKey = new BoxedValuetypeHashtableKey(owningTypeDefinition, ownerModuleOfThunk); + BoxedValueType boxedTypeDefinition = _boxedValuetypeHashtable.GetOrCreateValue(typeKey); + + var targetMethodDefinition = targetMethod.GetTypicalMethodDefinition(); + var methodKey = new UnboxingThunkHashtableKey(targetMethodDefinition, boxedTypeDefinition); + GenericUnboxingThunk thunkDefinition = _unboxingThunkHashtable.GetOrCreateValue(methodKey); + + Debug.Assert(owningType != owningTypeDefinition); + InstantiatedType boxedType = boxedTypeDefinition.MakeInstantiatedType(owningType.Instantiation); + + MethodDesc thunk = GetMethodForInstantiatedType(thunkDefinition, boxedType); + Debug.Assert(!thunk.HasInstantiation); + + return thunk; + } + + public bool IsSpecialUnboxingThunkTargetMethod(MethodDesc method) + { + return method.GetTypicalMethodDefinition().GetType() == typeof(ValueTypeInstanceMethodWithHiddenParameter); + } + + public MethodDesc GetRealSpecialUnboxingThunkTargetMethod(MethodDesc method) + { + MethodDesc typicalMethod = method.GetTypicalMethodDefinition(); + MethodDesc methodDefinitionRepresented = ((ValueTypeInstanceMethodWithHiddenParameter)typicalMethod).MethodRepresented; + return GetMethodForInstantiatedType(methodDefinitionRepresented, (InstantiatedType)method.OwningType); + } + + private struct BoxedValuetypeHashtableKey + { + public readonly MetadataType ValueType; + public readonly ModuleDesc OwningModule; + + public BoxedValuetypeHashtableKey(MetadataType valueType, ModuleDesc owningModule) + { + ValueType = valueType; + OwningModule = owningModule; + } + } + + private class BoxedValuetypeHashtable : LockFreeReaderHashtable + { + protected override int GetKeyHashCode(BoxedValuetypeHashtableKey key) + { + return key.ValueType.GetHashCode(); + } + protected override int GetValueHashCode(BoxedValueType value) + { + return value.ValueTypeRepresented.GetHashCode(); + } + protected override bool CompareKeyToValue(BoxedValuetypeHashtableKey key, BoxedValueType value) + { + return Object.ReferenceEquals(key.ValueType, value.ValueTypeRepresented) && + Object.ReferenceEquals(key.OwningModule, value.Module); + } + protected override bool CompareValueToValue(BoxedValueType value1, BoxedValueType value2) + { + return Object.ReferenceEquals(value1.ValueTypeRepresented, value2.ValueTypeRepresented) && + Object.ReferenceEquals(value1.Module, value2.Module); + } + protected override BoxedValueType CreateValueFromKey(BoxedValuetypeHashtableKey key) + { + return new BoxedValueType(key.OwningModule, key.ValueType); + } + } + private BoxedValuetypeHashtable _boxedValuetypeHashtable = new BoxedValuetypeHashtable(); + + private struct UnboxingThunkHashtableKey + { + public readonly MethodDesc TargetMethod; + public readonly BoxedValueType OwningType; + + public UnboxingThunkHashtableKey(MethodDesc targetMethod, BoxedValueType owningType) + { + TargetMethod = targetMethod; + OwningType = owningType; + } + } + + private class UnboxingThunkHashtable : LockFreeReaderHashtable + { + protected override int GetKeyHashCode(UnboxingThunkHashtableKey key) + { + return key.TargetMethod.GetHashCode(); + } + protected override int GetValueHashCode(GenericUnboxingThunk value) + { + return value.TargetMethod.GetHashCode(); + } + protected override bool CompareKeyToValue(UnboxingThunkHashtableKey key, GenericUnboxingThunk value) + { + return Object.ReferenceEquals(key.TargetMethod, value.TargetMethod) && + Object.ReferenceEquals(key.OwningType, value.OwningType); + } + protected override bool CompareValueToValue(GenericUnboxingThunk value1, GenericUnboxingThunk value2) + { + return Object.ReferenceEquals(value1.TargetMethod, value2.TargetMethod) && + Object.ReferenceEquals(value1.OwningType, value2.OwningType); + } + protected override GenericUnboxingThunk CreateValueFromKey(UnboxingThunkHashtableKey key) + { + return new GenericUnboxingThunk(key.OwningType, key.TargetMethod); + } + } + private UnboxingThunkHashtable _unboxingThunkHashtable = new UnboxingThunkHashtable(); + + + /// + /// A type with an identical layout to the layout of a boxed value type. + /// + private class BoxedValueType : MetadataType + { + private const string BoxedValueFieldName = "BoxedValue"; + + public FieldDesc BoxedValue { get; } + + public MetadataType ValueTypeRepresented { get; } + + public override ModuleDesc Module { get; } + + public override string Name => "Boxed_" + ValueTypeRepresented.Name; + + public override string Namespace + { + get + { + StringBuilder sb = new StringBuilder(); + + ArrayBuilder prefixes = new ArrayBuilder(); + + DefType currentType = ValueTypeRepresented; + if (currentType.ContainingType != null) + { + while (currentType.ContainingType != null) + { + prefixes.Add(currentType.Name); + currentType = currentType.ContainingType; + } + + prefixes.Add(currentType.Name); + } + + sb.Append(((IAssemblyDesc)((MetadataType)currentType).Module).GetName().Name); + sb.Append('_'); + sb.Append(currentType.Namespace); + + for (int i = prefixes.Count - 1; i >= 0; i--) + { + sb.Append(prefixes[i]); + + if (i > 0) + sb.Append('+'); + } + + return sb.ToString(); + } + } + + public override Instantiation Instantiation => ValueTypeRepresented.Instantiation; + public override PInvokeStringFormat PInvokeStringFormat => PInvokeStringFormat.AutoClass; + public override bool IsExplicitLayout => false; + public override bool IsSequentialLayout => true; + public override bool IsBeforeFieldInit => false; + public override MetadataType MetadataBaseType => (MetadataType)Context.GetWellKnownType(WellKnownType.Object); + public override bool IsSealed => true; + public override DefType ContainingType => null; + public override DefType[] ExplicitlyImplementedInterfaces => Array.Empty(); + public override TypeSystemContext Context => ValueTypeRepresented.Context; + + public BoxedValueType(ModuleDesc owningModule, MetadataType valuetype) + { + // BoxedValueType has the same genericness as the valuetype it's wrapping. + // Making BoxedValueType wrap the genericness (and be itself nongeneric) would + // require a crazy name mangling scheme to allow generating stable and unique names + // for the wrappers. + Debug.Assert(valuetype.IsTypeDefinition); + + Debug.Assert(valuetype.IsValueType); + + Module = owningModule; + ValueTypeRepresented = valuetype; + BoxedValue = new BoxedValueField(this); + } + + public override ClassLayoutMetadata GetClassLayout() => default(ClassLayoutMetadata); + public override bool HasCustomAttribute(string attributeNamespace, string attributeName) => false; + public override IEnumerable GetNestedTypes() => Array.Empty(); + public override MetadataType GetNestedType(string name) => null; + protected override MethodImplRecord[] ComputeVirtualMethodImplsForType() => Array.Empty(); + public override MethodImplRecord[] FindMethodsImplWithMatchingDeclName(string name) => Array.Empty(); + + public override int GetHashCode() + { + string ns = Namespace; + var hashCodeBuilder = new Internal.NativeFormat.TypeHashingAlgorithms.HashCodeBuilder(ns); + if (ns.Length > 0) + hashCodeBuilder.Append("."); + hashCodeBuilder.Append(Name); + return hashCodeBuilder.ToHashCode(); + } + + protected override TypeFlags ComputeTypeFlags(TypeFlags mask) + { + TypeFlags flags = 0; + + if ((mask & TypeFlags.ContainsGenericVariablesComputed) != 0) + { + flags |= TypeFlags.ContainsGenericVariablesComputed; + } + + if ((mask & TypeFlags.HasGenericVarianceComputed) != 0) + { + flags |= TypeFlags.HasGenericVarianceComputed; + } + + if ((mask & TypeFlags.CategoryMask) != 0) + { + flags |= TypeFlags.Class; + } + + return flags; + } + + public override FieldDesc GetField(string name) + { + if (name == BoxedValueFieldName) + return BoxedValue; + + return null; + } + + public override IEnumerable GetFields() + { + yield return BoxedValue; + } + + private class BoxedValueField : FieldDesc + { + private BoxedValueType _owningType; + + public override TypeSystemContext Context => _owningType.Context; + public override TypeDesc FieldType => _owningType.ValueTypeRepresented.InstantiateAsOpen(); + public override bool HasRva => false; + public override bool IsInitOnly => false; + public override bool IsLiteral => false; + public override bool IsStatic => false; + public override bool IsThreadStatic => false; + public override DefType OwningType => _owningType; + public override bool HasCustomAttribute(string attributeNamespace, string attributeName) => false; + public override string Name => BoxedValueFieldName; + + public BoxedValueField(BoxedValueType owningType) + { + _owningType = owningType; + } + } + } + + private class GenericUnboxingThunk : ILStubMethod + { + private MethodDesc _targetMethod; + private ValueTypeInstanceMethodWithHiddenParameter _nakedTargetMethod; + private BoxedValueType _owningType; + + public GenericUnboxingThunk(BoxedValueType owningType, MethodDesc targetMethod) + { + Debug.Assert(targetMethod.OwningType.IsValueType); + Debug.Assert(!targetMethod.Signature.IsStatic); + + _owningType = owningType; + _targetMethod = targetMethod; + _nakedTargetMethod = new ValueTypeInstanceMethodWithHiddenParameter(targetMethod); + } + + public override TypeSystemContext Context => _targetMethod.Context; + + public override TypeDesc OwningType => _owningType; + + public override MethodSignature Signature => _targetMethod.Signature; + + public MethodDesc TargetMethod => _targetMethod; + + public override string Name + { + get + { + return _targetMethod.Name + "_Unbox"; + } + } + + public override MethodIL EmitIL() + { + ILEmitter emit = new ILEmitter(); + ILCodeStream codeStream = emit.NewCodeStream(); + + FieldDesc eeTypeField = Context.GetWellKnownType(WellKnownType.Object).GetKnownField("m_pEEType"); + FieldDesc boxedValueField = _owningType.BoxedValue.InstantiateAsOpen(); + + codeStream.EmitLdArg(0); + codeStream.Emit(ILOpcode.ldflda, emit.NewToken(boxedValueField)); + + codeStream.EmitLdArg(0); + codeStream.Emit(ILOpcode.ldfld, emit.NewToken(eeTypeField)); + + for (int i = 0; i < _targetMethod.Signature.Length; i++) + { + codeStream.EmitLdArg(i + 1); + } + + TypeDesc[] targetTypeInstantiation = new TypeDesc[OwningType.Instantiation.Length]; + for (int i = 0; i < OwningType.Instantiation.Length; i++) + targetTypeInstantiation[i] = Context.GetSignatureVariable(i, false); + + InstantiatedType targetType = Context.GetInstantiatedType((MetadataType)_nakedTargetMethod.OwningType, new Instantiation(targetTypeInstantiation)); + + MethodDesc targetMethod = Context.GetMethodForInstantiatedType(_nakedTargetMethod, targetType); + + codeStream.Emit(ILOpcode.call, emit.NewToken(targetMethod)); + codeStream.Emit(ILOpcode.ret); + + return emit.Link(this); + } + } + + internal class ValueTypeInstanceMethodWithHiddenParameter : MethodDesc + { + private MethodDesc _methodRepresented; + private MethodSignature _signature; + + public ValueTypeInstanceMethodWithHiddenParameter(MethodDesc methodRepresented) + { + Debug.Assert(methodRepresented.OwningType.IsValueType); + Debug.Assert(!methodRepresented.Signature.IsStatic); + + _methodRepresented = methodRepresented; + } + + public MethodDesc MethodRepresented => _methodRepresented; + + public override bool IsNoInlining => true; + public override TypeSystemContext Context => _methodRepresented.Context; + public override TypeDesc OwningType => _methodRepresented.OwningType; + + public override string Name => _methodRepresented.Name; + + public override MethodSignature Signature + { + get + { + if (_signature == null) + { + TypeDesc[] parameters = new TypeDesc[_methodRepresented.Signature.Length + 1]; + + parameters[0] = Context.GetWellKnownType(WellKnownType.Object).GetKnownField("m_pEEType").FieldType; + for (int i = 0; i < _methodRepresented.Signature.Length; i++) + parameters[i + 1] = _methodRepresented.Signature[0]; + + _signature = new MethodSignature(_methodRepresented.Signature.Flags, + _methodRepresented.Signature.GenericParameterCount, + _methodRepresented.Signature.ReturnType, + parameters); + } + + return _signature; + } + } + + public override bool HasCustomAttribute(string attributeNamespace, string attributeName) => false; + } + } +} diff --git a/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/RyuJitNodeFactory.cs b/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/RyuJitNodeFactory.cs index cb2d749621e..868aeed725b 100644 --- a/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/RyuJitNodeFactory.cs +++ b/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/RyuJitNodeFactory.cs @@ -34,7 +34,14 @@ protected override IMethodNode CreateMethodEntrypointNode(MethodDesc method) if (CompilationModuleGroup.ContainsMethod(method)) { - return new MethodCodeNode(method); + if (TypeSystemContext.IsSpecialUnboxingThunkTargetMethod(method)) + { + return MethodEntrypoint(TypeSystemContext.GetRealSpecialUnboxingThunkTargetMethod(method)); + } + else + { + return new MethodCodeNode(method); + } } else { @@ -44,7 +51,15 @@ protected override IMethodNode CreateMethodEntrypointNode(MethodDesc method) protected override IMethodNode CreateUnboxingStubNode(MethodDesc method) { - return new UnboxingStubNode(method); + if (method.IsCanonicalMethod(CanonicalFormKind.Specific)) + { + // We need an unboxing and instantiating stub. + return new MethodCodeNode(TypeSystemContext.GetSpecialUnboxingThunk(method, CompilationModuleGroup.GeneratedAssembly)); + } + else + { + return new UnboxingStubNode(method); + } } protected override ISymbolNode CreateReadyToRunHelperNode(Tuple helperCall) diff --git a/src/ILCompiler.Compiler/src/ILCompiler.Compiler.csproj b/src/ILCompiler.Compiler/src/ILCompiler.Compiler.csproj index 2330e0a02f3..b6e8e1cccee 100644 --- a/src/ILCompiler.Compiler/src/ILCompiler.Compiler.csproj +++ b/src/ILCompiler.Compiler/src/ILCompiler.Compiler.csproj @@ -90,6 +90,7 @@ JitInterface\JitConfigProvider.cs + diff --git a/src/JitInterface/src/CorInfoImpl.cs b/src/JitInterface/src/CorInfoImpl.cs index 2064ed4de3e..51f0526456b 100644 --- a/src/JitInterface/src/CorInfoImpl.cs +++ b/src/JitInterface/src/CorInfoImpl.cs @@ -499,7 +499,7 @@ private void Get_CORINFO_SIG_INFO(MethodDesc method, out CORINFO_SIG_INFO sig, b Get_CORINFO_SIG_INFO(method.Signature, out sig); // Does the method have a hidden parameter? - if (method.RequiresInstArg() && !isFatFunctionPointer) + if (method.RequiresInstArg() && !isFatFunctionPointer && !_compilation.TypeSystemContext.IsSpecialUnboxingThunkTargetMethod(method)) { sig.callConv |= CorInfoCallConv.CORINFO_CALLCONV_PARAMTYPE; } From f73ca7cd3e3c3e685ef518a4f1540a8684bc44f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Wed, 25 Jan 2017 18:16:07 -0800 Subject: [PATCH 2/6] Add comments --- .../CompilerTypeSystemContext.BoxedTypes.cs | 92 +++++++++++++++++-- 1 file changed, 83 insertions(+), 9 deletions(-) diff --git a/src/ILCompiler.Compiler/src/Compiler/CompilerTypeSystemContext.BoxedTypes.cs b/src/ILCompiler.Compiler/src/Compiler/CompilerTypeSystemContext.BoxedTypes.cs index cb5d743d2ab..c4e888f682d 100644 --- a/src/ILCompiler.Compiler/src/Compiler/CompilerTypeSystemContext.BoxedTypes.cs +++ b/src/ILCompiler.Compiler/src/Compiler/CompilerTypeSystemContext.BoxedTypes.cs @@ -4,32 +4,81 @@ using System; using System.Collections.Generic; +using System.Text; using Internal.TypeSystem; using Internal.IL; using Internal.IL.Stubs; using Debug = System.Diagnostics.Debug; -using System.Text; -namespace ILCompiler +// +// Functionality related to instantiating unboxing thunks +// +// To support calling canonical interface methods on generic valuetypes, +// the compiler needs to generate unboxing+instantiating thunks that bridge +// the difference between the two calling conventions. +// +// As a refresher: +// * Instance methods on shared generic valuetypes expect two arguments +// (aside from the arguments declared in the signature): a ByRef to the +// first byte of the value of the valuetype (this), and a generic context +// argument (EEType) +// * Interface calls expect 'this' to be a reference type (with the generic +// context to be inferred from 'this' by the callee). +// +// Instantiating and unboxing stubs bridge this by extracting a managed +// pointer out of a boxed valuetype, along with the EEType of the boxed +// valuetype (to provide the generic context) before dispatching to the +// instance method with the different calling convention. +// +// We compile them by: +// * Pretending the unboxing stub is an instance method on a reference type +// with the same layout as a boxed valuetype (this matches the calling +// convention expected by the caller). +// * Having the unboxing stub load the m_pEEType field (to get generic +// context) and a byref to the actual value (to get a 'this' expected by +// valuetype methods) +// * Generating a call to a fake instance method on the valuetype that has +// the hidden (generic context) argument explicitly present in the +// signature. We need a fake method to be able to refer to the hidden parameter +// from IL. +// +// At a later stage (once codegen is done), we replace the references to the +// fake instance method with the real instance method. Their signatures after +// compilation is identical. +// +namespace ILCompiler { // Contains functionality related to pseudotypes representing boxed instances of value types partial class CompilerTypeSystemContext { + /// + /// For a shared (canonical) instance method on a generic valuetype, gets a method that can be used to call the + /// method given a boxed version of the generic valuetype as 'this' pointer. + /// public MethodDesc GetSpecialUnboxingThunk(MethodDesc targetMethod, ModuleDesc ownerModuleOfThunk) { + Debug.Assert(targetMethod.IsSharedByGenericInstantiations); + Debug.Assert(!targetMethod.Signature.IsStatic); + TypeDesc owningType = targetMethod.OwningType; + Debug.Assert(owningType.IsValueType); + var owningTypeDefinition = (MetadataType)owningType.GetTypeDefinition(); + // Get a reference type that has the same layout as the boxed valuetype. var typeKey = new BoxedValuetypeHashtableKey(owningTypeDefinition, ownerModuleOfThunk); BoxedValueType boxedTypeDefinition = _boxedValuetypeHashtable.GetOrCreateValue(typeKey); + // Get a method on the reference type with the same signature as the target method (but different + // calling convention, since 'this' will be a reference type). var targetMethodDefinition = targetMethod.GetTypicalMethodDefinition(); var methodKey = new UnboxingThunkHashtableKey(targetMethodDefinition, boxedTypeDefinition); GenericUnboxingThunk thunkDefinition = _unboxingThunkHashtable.GetOrCreateValue(methodKey); + // Find the thunk on the instantiated version of the reference type. Debug.Assert(owningType != owningTypeDefinition); InstantiatedType boxedType = boxedTypeDefinition.MakeInstantiatedType(owningType.Instantiation); @@ -39,11 +88,17 @@ public MethodDesc GetSpecialUnboxingThunk(MethodDesc targetMethod, ModuleDesc ow return thunk; } + /// + /// Returns true of is a standin method for unboxing thunk target. + /// public bool IsSpecialUnboxingThunkTargetMethod(MethodDesc method) { return method.GetTypicalMethodDefinition().GetType() == typeof(ValueTypeInstanceMethodWithHiddenParameter); } + /// + /// Returns the real target method of an unboxing stub. + /// public MethodDesc GetRealSpecialUnboxingThunkTargetMethod(MethodDesc method) { MethodDesc typicalMethod = method.GetTypicalMethodDefinition(); @@ -132,6 +187,7 @@ protected override GenericUnboxingThunk CreateValueFromKey(UnboxingThunkHashtabl /// /// A type with an identical layout to the layout of a boxed value type. + /// The type has a single field of the type of the valuetype it represents. /// private class BoxedValueType : MetadataType { @@ -149,6 +205,8 @@ public override string Namespace { get { + // Mangle the namespace in the hopes that it won't conflict with anything else. + StringBuilder sb = new StringBuilder(); ArrayBuilder prefixes = new ArrayBuilder(); @@ -259,6 +317,9 @@ public override IEnumerable GetFields() yield return BoxedValue; } + /// + /// Synthetic field on . + /// private class BoxedValueField : FieldDesc { private BoxedValueType _owningType; @@ -281,6 +342,9 @@ public BoxedValueField(BoxedValueType owningType) } } + /// + /// Represents a thunk to call shared instance method on boxed valuetypes. + /// private class GenericUnboxingThunk : ILStubMethod { private MethodDesc _targetMethod; @@ -321,23 +385,24 @@ public override MethodIL EmitIL() FieldDesc eeTypeField = Context.GetWellKnownType(WellKnownType.Object).GetKnownField("m_pEEType"); FieldDesc boxedValueField = _owningType.BoxedValue.InstantiateAsOpen(); + // Load ByRef to the field with the value of the boxed valuetype codeStream.EmitLdArg(0); codeStream.Emit(ILOpcode.ldflda, emit.NewToken(boxedValueField)); + // Load the EEType of the boxed valuetype (this is the hidden generic context parameter expected + // by the (canonical) instance method, but normally not part of the signature in IL). codeStream.EmitLdArg(0); codeStream.Emit(ILOpcode.ldfld, emit.NewToken(eeTypeField)); + // Load rest of the arguments for (int i = 0; i < _targetMethod.Signature.Length; i++) { codeStream.EmitLdArg(i + 1); } - TypeDesc[] targetTypeInstantiation = new TypeDesc[OwningType.Instantiation.Length]; - for (int i = 0; i < OwningType.Instantiation.Length; i++) - targetTypeInstantiation[i] = Context.GetSignatureVariable(i, false); - - InstantiatedType targetType = Context.GetInstantiatedType((MetadataType)_nakedTargetMethod.OwningType, new Instantiation(targetTypeInstantiation)); - + // Call an instance method on the target valuetype that has a fake instantiation parameter + // in it's signature. This will be swapped by the actual instance method after codegen is done. + InstantiatedType targetType = (InstantiatedType)_nakedTargetMethod.OwningType.InstantiateAsOpen(); MethodDesc targetMethod = Context.GetMethodForInstantiatedType(_nakedTargetMethod, targetType); codeStream.Emit(ILOpcode.call, emit.NewToken(targetMethod)); @@ -347,6 +412,11 @@ public override MethodIL EmitIL() } } + /// + /// Represents an instance method on a generic valuetype with an explicit instantiation parameter in the + /// signature. This is so that we can refer to the parameter from IL. References to this method will + /// be replaced by the actual instance method after codegen is done. + /// internal class ValueTypeInstanceMethodWithHiddenParameter : MethodDesc { private MethodDesc _methodRepresented; @@ -362,7 +432,9 @@ public ValueTypeInstanceMethodWithHiddenParameter(MethodDesc methodRepresented) public MethodDesc MethodRepresented => _methodRepresented; + // We really don't want this method to be inlined. public override bool IsNoInlining => true; + public override TypeSystemContext Context => _methodRepresented.Context; public override TypeDesc OwningType => _methodRepresented.OwningType; @@ -376,9 +448,11 @@ public override MethodSignature Signature { TypeDesc[] parameters = new TypeDesc[_methodRepresented.Signature.Length + 1]; + // Shared instance methods on generic valuetypes have a hidden parameter with the generic context. + // We add it to the signature so that we can refer to it from IL. parameters[0] = Context.GetWellKnownType(WellKnownType.Object).GetKnownField("m_pEEType").FieldType; for (int i = 0; i < _methodRepresented.Signature.Length; i++) - parameters[i + 1] = _methodRepresented.Signature[0]; + parameters[i + 1] = _methodRepresented.Signature[i]; _signature = new MethodSignature(_methodRepresented.Signature.Flags, _methodRepresented.Signature.GenericParameterCount, From a5c381e264d380d513275a6f6048538748e4ac66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Wed, 25 Jan 2017 18:57:20 -0800 Subject: [PATCH 3/6] Comments --- .../src/Compiler/CompilerTypeSystemContext.BoxedTypes.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/ILCompiler.Compiler/src/Compiler/CompilerTypeSystemContext.BoxedTypes.cs b/src/ILCompiler.Compiler/src/Compiler/CompilerTypeSystemContext.BoxedTypes.cs index c4e888f682d..fbf148f56f2 100644 --- a/src/ILCompiler.Compiler/src/Compiler/CompilerTypeSystemContext.BoxedTypes.cs +++ b/src/ILCompiler.Compiler/src/Compiler/CompilerTypeSystemContext.BoxedTypes.cs @@ -379,6 +379,9 @@ public override string Name public override MethodIL EmitIL() { + // Generate the unboxing stub. This loosely corresponds to following C#: + // return BoxedValue.InstanceMethod(this.m_pEEType, [rest of parameters]) + ILEmitter emit = new ILEmitter(); ILCodeStream codeStream = emit.NewCodeStream(); From 1376755219132c136b5c3bcea568ab6267fabd37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Wed, 25 Jan 2017 19:04:31 -0800 Subject: [PATCH 4/6] Use InstantiateAsOpen --- .../src/Compiler/CompilerTypeSystemContext.BoxedTypes.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/ILCompiler.Compiler/src/Compiler/CompilerTypeSystemContext.BoxedTypes.cs b/src/ILCompiler.Compiler/src/Compiler/CompilerTypeSystemContext.BoxedTypes.cs index fbf148f56f2..174b9e4759b 100644 --- a/src/ILCompiler.Compiler/src/Compiler/CompilerTypeSystemContext.BoxedTypes.cs +++ b/src/ILCompiler.Compiler/src/Compiler/CompilerTypeSystemContext.BoxedTypes.cs @@ -405,10 +405,7 @@ public override MethodIL EmitIL() // Call an instance method on the target valuetype that has a fake instantiation parameter // in it's signature. This will be swapped by the actual instance method after codegen is done. - InstantiatedType targetType = (InstantiatedType)_nakedTargetMethod.OwningType.InstantiateAsOpen(); - MethodDesc targetMethod = Context.GetMethodForInstantiatedType(_nakedTargetMethod, targetType); - - codeStream.Emit(ILOpcode.call, emit.NewToken(targetMethod)); + codeStream.Emit(ILOpcode.call, emit.NewToken(_nakedTargetMethod.InstantiateAsOpen())); codeStream.Emit(ILOpcode.ret); return emit.Link(this); From 26faeaa78ebe4f7162d4c7106647237e1576b6e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Thu, 26 Jan 2017 11:10:08 -0800 Subject: [PATCH 5/6] Don't generate instantiating unboxing stubs for generic methods --- .../Compiler/CompilerTypeSystemContext.BoxedTypes.cs | 1 + .../Compiler/DependencyAnalysis/RyuJitNodeFactory.cs | 10 ++++++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/ILCompiler.Compiler/src/Compiler/CompilerTypeSystemContext.BoxedTypes.cs b/src/ILCompiler.Compiler/src/Compiler/CompilerTypeSystemContext.BoxedTypes.cs index 174b9e4759b..2ab75555558 100644 --- a/src/ILCompiler.Compiler/src/Compiler/CompilerTypeSystemContext.BoxedTypes.cs +++ b/src/ILCompiler.Compiler/src/Compiler/CompilerTypeSystemContext.BoxedTypes.cs @@ -62,6 +62,7 @@ public MethodDesc GetSpecialUnboxingThunk(MethodDesc targetMethod, ModuleDesc ow { Debug.Assert(targetMethod.IsSharedByGenericInstantiations); Debug.Assert(!targetMethod.Signature.IsStatic); + Debug.Assert(!targetMethod.HasInstantiation); TypeDesc owningType = targetMethod.OwningType; Debug.Assert(owningType.IsValueType); diff --git a/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/RyuJitNodeFactory.cs b/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/RyuJitNodeFactory.cs index 868aeed725b..a768f85cd9e 100644 --- a/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/RyuJitNodeFactory.cs +++ b/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/RyuJitNodeFactory.cs @@ -51,13 +51,19 @@ protected override IMethodNode CreateMethodEntrypointNode(MethodDesc method) protected override IMethodNode CreateUnboxingStubNode(MethodDesc method) { - if (method.IsCanonicalMethod(CanonicalFormKind.Specific)) + Debug.Assert(!method.Signature.IsStatic); + + if (method.IsCanonicalMethod(CanonicalFormKind.Specific) && !method.HasInstantiation) { - // We need an unboxing and instantiating stub. + // Unboxing stubs to canonical instance methods need a special unboxing stub that unboxes + // 'this' and also provides an instantiation argument (we do a calling convention conversion). + // We don't do this for generic instance methods though because they don't use the EEType + // for the generic context anyway. return new MethodCodeNode(TypeSystemContext.GetSpecialUnboxingThunk(method, CompilationModuleGroup.GeneratedAssembly)); } else { + // Otherwise we just unbox 'this' and don't touch anything else. return new UnboxingStubNode(method); } } From b9111bebd9c0242561b2aae7349ee021742c370a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Thu, 26 Jan 2017 11:21:38 -0800 Subject: [PATCH 6/6] Add test --- tests/src/Simple/Generics/Generics.cs | 47 +++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/tests/src/Simple/Generics/Generics.cs b/tests/src/Simple/Generics/Generics.cs index 603d5b792e3..55d6fb6a333 100644 --- a/tests/src/Simple/Generics/Generics.cs +++ b/tests/src/Simple/Generics/Generics.cs @@ -19,6 +19,7 @@ static int Main() TestDelegateInterfaceMethod.Run(); TestThreadStaticFieldAccess.Run(); TestConstrainedMethodCalls.Run(); + TestInstantiatingUnboxingStubs.Run(); TestNameManglingCollisionRegression.Run(); TestUnusedGVMsDoNotCrashCompiler.Run(); @@ -420,6 +421,52 @@ public static void Run() } } + class TestInstantiatingUnboxingStubs + { + static volatile IFoo s_foo; + + interface IFoo + { + bool IsInst(object o); + + void Set(int value); + } + + struct Foo : IFoo + { + public int Value; + + public bool IsInst(object o) + { + return o is T; + } + + public void Set(int value) + { + Value = value; + } + } + + public static void Run() + { + s_foo = new Foo(); + + // Make sure the instantiation argument is properly passed + if (!s_foo.IsInst("ab")) + throw new Exception(); + + if (s_foo.IsInst(new object())) + throw new Exception(); + + // Make sure the byref to 'this' is properly passed + s_foo.Set(42); + + var foo = (Foo)s_foo; + if (foo.Value != 42) + throw new Exception(); + } + } + // // Regression test for issue https://github.com/dotnet/corert/issues/1964 //