Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Detect ByRefLike types using attribute #15745

Merged
merged 2 commits into from
Jan 5, 2018
Merged

Conversation

jkotas
Copy link
Member

@jkotas jkotas commented Jan 5, 2018

Detect ByRefLike types using attribute and improve error messages for their invalid use

Fixes #11371 and #15458

@mikedn
Copy link

mikedn commented Jan 5, 2018

Hmm, I wonder if there are sufficient guarantees that by ref like types only live on the stack for the JIT to be able to eliminate GC write barriers when writing to fields of such types.

Copy link
Member

@MichalStrehovsky MichalStrehovsky left a comment

Choose a reason for hiding this comment

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

Looks good apart from the minor comments.

@@ -4085,8 +4089,10 @@ VOID MethodTableBuilder::InitializeFieldDescs(FieldDesc *pFieldDescList,
// Non-value-classes cannot contain by-ref-like instance fields
BuildMethodTableThrowException(IDS_CLASSLOAD_BYREFLIKE_NOTVALUECLASSFIELD);
}

bmtFP->fIsByRefLikeType = true;
if (!bmtFP->fIsByRefLikeType)
Copy link
Member

Choose a reason for hiding this comment

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

Maybe update the comment above (// Inherit IsByRefLike characteristic from fields) since we're not inheriting it anymore, but merely checking?

.maxstack 1
ldc.i4.1
newarr valuetype MyByRefLikeType
ret
Copy link
Member

Choose a reason for hiding this comment

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

Non-empty stack but returns void?

.method private hidebysig static void ByRefLikeGenericInstantiation() cil managed
{
.maxstack 1
newobj instance void class [System.Runtime]System.Collections.Generic.List`1<valuetype MyByRefLikeType>::.ctor()
Copy link
Member

Choose a reason for hiding this comment

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

Non-empty stack but returns void?

@jkotas
Copy link
Member Author

jkotas commented Jan 5, 2018

Hmm, I wonder if there are sufficient guarantees that by ref like types only live on the stack for the JIT to be able to eliminate GC write barriers when writing to fields of such types.

Yes, there are. It would be nice optimization to have.

Opened https://github.com/dotnet/coreclr/issues/15755

@jkotas
Copy link
Member Author

jkotas commented Jan 5, 2018

@dotnet-bot test OSX10.12 x64 Checked Innerloop Build and Test please

@kouvel
Copy link
Member

kouvel commented Jan 5, 2018

LGTM

@jkotas
Copy link
Member Author

jkotas commented Jan 5, 2018

@dotnet-bot test Tizen armel Cross Checked Innerloop Build and Test please
@dotnet-bot test Windows_NT arm64 Cross Checked Innerloop Build and Test please

@jkotas jkotas merged commit 6c12105 into dotnet:master Jan 5, 2018
@jkotas jkotas deleted the byref-like branch January 5, 2018 23:09
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants