Implement TypedReference in CoreRT #3000

MichalStrehovsky · 2017-03-16T22:19:14Z

This is basically three things:

ByReference<T> support
General support infra in the type system for Byref-like types (to be
reused for e.g. Span<T>).
TypedReference

Fixes #367.

This is basically two things: 1. `ByReference<T>` support 2. General support infra in the type system for Byref-like types (to be reused for e.g. `Span<T>`). 3. TypedReference

MichalStrehovsky · 2017-03-16T22:26:22Z

This turned out to be more work than I thought it would.

Known missing things:

Block byref-like types in places such as Array.CreateInstance, Activator.CreateInstance (the non-generic one) and Type.MakeGenericType. I assume with the building blocks in this PR, @atsushikan can do this later.
Support in Project N. This needs tweaks in NUTC and the binder. I assume @davidwrighton will take care of that. I don't volunteer for binder work if I can help it.
Support in the runtime type loader. @davidwrighton?

@davidwrighton @jkotas Can you please do the "where did Michal forget to handle/block the new things" exercise?

MichalStrehovsky · 2017-03-16T22:29:14Z

src/Common/src/TypeSystem/Common/TypeDesc.cs

+        {
+            get
+            {
+                // TODO: should we key this off of the fact that there's an instance field


(This could also be an algorithm.)

Yes it should. ByRefLike should be set to true for Span, ByReference, any generic structure that has a Span or ByReference in it.

davidwrighton

This change is missing handling of the runtime impacts and ByRefLike virality

ReflectionInvoke of a method that takes a parameter or returns a TypedByreference, ByReference, or any other ByRefLike type should be prohibited
Dynamic type loader should be updated to properly set the ByRefLike flag in generated EETypes based on the template. The pattern to follow is to either make the type system api work in the runtime scenario, or to add a bool property to TypeBuilderState which does the right thing.
ByRefLike is viral, if a type is ByRefLike, then a type with an instance field of such a type is also ByRefLike.

davidwrighton · 2017-03-16T23:12:07Z

src/Common/src/TypeSystem/Common/TypeDesc.cs

+        {
+            get
+            {
+                // TODO: should we key this off of the fact that there's an instance field


Yes it should. ByRefLike should be set to true for Span, ByReference, any generic structure that has a Span or ByReference in it.

jkotas · 2017-03-17T13:45:44Z

src/Common/src/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs

+                if (fieldType.IsValueType && !fieldType.IsPrimitive)
+                {
+                    DefType fieldDefType = (DefType)fieldType;
+                    if (fieldDefType.IsByRefLike || fieldDefType.IsByReferenceOfT)


ByReference<T> should have IsByRefLike set, so this should be just if (fieldDefType.IsByRefLike)

jkotas · 2017-03-17T13:50:41Z

src/Common/src/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs

+                // ByRef=like instance fields on reference types are not allowed.
+                if (fieldType.IsValueType && !type.IsValueType)
+                {
+                    if (((DefType)fieldType).IsByRefLike || fieldType.IsByReferenceOfT)


IsByReferenceOfT check should not be needed.

jkotas · 2017-03-17T14:07:07Z

src/Common/src/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs

@@ -223,6 +241,12 @@ public override bool ComputeContainsGCPointers(DefType type)
        {
            bool someFieldContainsPointers = false;

+            if (type.IsValueType)


IsGCPointer returns false for ByReference<T>, so this not quite self-consistent.

FWIW, the CoreCLR implementation does not treat ByReference<T> as GCPointer. The only place where it really matters is the fast path on getClassGClayout - it checks for IsByRefLike to take the slow path there.

jkotas · 2017-03-17T14:19:23Z

src/Common/src/TypeSystem/Common/DefType.FieldLayout.cs

+                {
+                    ComputeIsByRefLike();
+                }
+                return _fieldLayoutFlags.HasFlags(FieldLayoutFlags.IsByRefLike);


BTW: I expect that the ref-like types will be eventually identified by attribute, and so ComputeIsByRefLike will be just a check of the attribute; not a property of the field layout.

davidwrighton

Looksgoodtome

fadimounir · 2017-03-23T19:44:13Z

src/System.Private.CoreLib/src/Internal/Runtime/CompilerHelpers/TypedReferenceHelpers.cs

+    /// </summary>
+    internal static class TypedReferenceHelpers
+    {
+        public unsafe static RuntimeTypeHandle TypeHandleToRuntimeTypeMaybeNull(RuntimeTypeHandle typeHandle)


@MichalStrehovsky What does this function do exactly? Is it a compiler intrinsic?

It's what we give to RyuJIT as CORINFO_HELP_TYPEHANDLE_TO_RUNTIMETYPE_MAYBENULL.

You don't have to worry about it because this is something RyuJIT needs as part of compiling refanytype IL instruction. The UTC team showed little to no enthusiasm about implementing that so what we'll have to do on Project N is update DR (or GateKeeper) to mark all methods that have this (and related instructions) as unsupported IL and bail.

Implement TypedReference in CoreRT

c939680

This is basically two things: 1. `ByReference<T>` support 2. General support infra in the type system for Byref-like types (to be reused for e.g. `Span<T>`). 3. TypedReference

dnfclas added the cla-already-signed label Mar 16, 2017

MichalStrehovsky requested review from jkotas and davidwrighton March 16, 2017 22:27

MichalStrehovsky commented Mar 16, 2017

View reviewed changes

davidwrighton suggested changes Mar 16, 2017

View reviewed changes

IsByRefLike Algorithm

e97bcc2

jkotas reviewed Mar 17, 2017

View reviewed changes

MichalStrehovsky added 6 commits March 20, 2017 10:17

Merge branch 'master' into typedReference

dc489c9

Disallow reflection invoking methods that return byref like types

3a2fdba

Set ByRefLike flag on dynamically loaded types

e1f93aa

Switch validation around

98dd47d

Jan's CR feedback

1edf8b3

Fixes for TFS-land

dbfb5a4

davidwrighton approved these changes Mar 23, 2017

View reviewed changes

MichalStrehovsky merged commit f4407ca into dotnet:master Mar 23, 2017

MichalStrehovsky deleted the typedReference branch March 23, 2017 19:28

fadimounir reviewed Mar 23, 2017

View reviewed changes

MichalStrehovsky mentioned this pull request Dec 21, 2018

Moves ByReference to shared CoreLib dotnet/coreclr#21633

Merged

ghost mentioned this pull request Jan 31, 2020

Need api to determine if a type is "byref-like." dotnet/runtime#20673

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement TypedReference in CoreRT #3000

Implement TypedReference in CoreRT #3000

MichalStrehovsky commented Mar 16, 2017 •

edited

Loading

MichalStrehovsky commented Mar 16, 2017 •

edited

Loading

MichalStrehovsky Mar 16, 2017

davidwrighton Mar 16, 2017

davidwrighton left a comment

davidwrighton Mar 16, 2017

jkotas Mar 17, 2017

jkotas Mar 17, 2017

jkotas Mar 17, 2017

jkotas Mar 17, 2017

davidwrighton left a comment

fadimounir Mar 23, 2017

MichalStrehovsky Mar 23, 2017

MichalStrehovsky Mar 23, 2017

Implement TypedReference in CoreRT #3000

Implement TypedReference in CoreRT #3000

Conversation

MichalStrehovsky commented Mar 16, 2017 • edited Loading

MichalStrehovsky commented Mar 16, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidwrighton left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidwrighton left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MichalStrehovsky commented Mar 16, 2017 •

edited

Loading

MichalStrehovsky commented Mar 16, 2017 •

edited

Loading