Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extend preinitialization interpreter to support calling string::Length #78680

Merged
merged 1 commit into from
Nov 23, 2022

Conversation

MichalStrehovsky
Copy link
Member

  • Support callvirt to a non-virtual method
  • Support accessing string's fields

A bit of progress towards #78093 (comment).

Cc @dotnet/ilc-contrib

* Support `callvirt` to a non-virtual method
* Support accessing string's fields
@ghost
Copy link

ghost commented Nov 22, 2022

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Issue Details
  • Support callvirt to a non-virtual method
  • Support accessing string's fields

A bit of progress towards #78093 (comment).

Cc @dotnet/ilc-contrib

Author: MichalStrehovsky
Assignees: -
Labels:

area-NativeAOT-coreclr

Milestone: -

@@ -598,6 +607,11 @@ private Status TryScanMethod(MethodIL methodIL, Value[] parameters, Stack<Method
return Status.Fail(methodIL.OwningMethod, opcode, "Reference field");
}

if (field.FieldType.IsByRef)
{
return Status.Fail(methodIL.OwningMethod, opcode, "Byref field");
Copy link
Member Author

Choose a reason for hiding this comment

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

This is not strictly necessary because we would throw/catch exception later, but ideally we should only do throw/catch on invalid programs.

Debug.Assert(firstCharField.FieldType.IsWellKnownType(WellKnownType.Char)
&& firstCharField.Offset.AsInt == pointerSize + sizeof(int) /* length */);

value.CopyTo(MemoryMarshal.Cast<byte, char>(((Span<byte>)bytes).Slice(firstCharField.Offset.AsInt)));
Copy link
Member

Choose a reason for hiding this comment

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

This would not work for little-endian/big-endian cross-compilation, but it is not the first problem of this kind in the aot interpreter.

Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@jkotas jkotas merged commit 8020db5 into dotnet:main Nov 23, 2022
@MichalStrehovsky MichalStrehovsky deleted the morepreinit branch November 23, 2022 03:26
@ghost ghost locked as resolved and limited conversation to collaborators Dec 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants