Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Consume Roslyn withref fields support#71498

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
AaronRobinsonMSFT merged 25 commits intomainfromfeature/csharp_ref_fields
Jul 2, 2022

Conversation

AaronRobinsonMSFT
Copy link
Member

@AaronRobinsonMSFTAaronRobinsonMSFT commentedJun 30, 2022•
edited
Loading

Moves to a Roslyn version that supportref field support and thescoped keyword.

RemovesByReference<T> from all runtimes.

Contributes to#63768

am11, jkoritzinsky, ysmgthntt, and steveharter reacted with thumbs up emojiSergio0694 reacted with heart emoji
Update public APIs
@ghost
Copy link

Note regarding thenew-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@ghost
Copy link

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly onearea label.

@ghost
Copy link

Tagging subscribers to this area: @dotnet/area-meta
See info inarea-owners.md if you want to be subscribed.

Issue Details

Moves to a Roslyn version that supportref field support and thescoped keyword.

RemovesByReference<T> from all runtimes.

Author:AaronRobinsonMSFT
Assignees:AaronRobinsonMSFT
Labels:

area-Meta,CoreClr,Mono

Milestone:-

@AaronRobinsonMSFT
Copy link
MemberAuthor

@AaronRobinsonMSFT I'm not seeing any interpreter issues on the tests. Is there anything else that needs fixing ?

@BrzVlad I think we've made it past most of the legs that were failing so I perhaps this is solved. I applied your suggestion and verified it locally fixed the issue I was hitting above - Thank you.

@BrzVlad or@lambdageek Can I get one of you to sign-off on the mono changes?

@AaronRobinsonMSFTAaronRobinsonMSFT deleted the feature/csharp_ref_fields branchJuly 2, 2022 21:07
@EgorBo
Copy link
Member

EgorBo commentedJul 5, 2022•
edited
Loading

publicSpan<byte>Test(byte[]array)=>array.AsSpan(0,10);

codegen before this PR:

; Method Program:ArrayAsSpanStartLength(System.Byte[]):System.Span`1[Byte]:thisG_M63029_IG01:       4883EC28subrsp,40G_M63029_IG02:       4D85C0testr8,r8       741Dje       SHORT G_M63029_IG04       418378080Acmp      dword ptr[r8+8],107216jb       SHORT G_M63029_IG04       4983C010addr8,16       4C8902mov      bword ptr[rdx],r8       C742080A000000mov      dword ptr[rdx+8],10       488BC2movrax,rdxG_M63029_IG03:       4883C428addrsp,40       C3retG_M63029_IG04:       FF15247D2700call[System.ThrowHelper:ThrowArgumentOutOfRangeException()]       CCint3; Total bytes of code: 45

Codegen after this PR:

; Method Program:ArrayAsSpanStartLength(System.Byte[]):System.Span`1[Byte]:thisG_M63029_IG01:57pushrdi56pushrsi53pushrbx       4883EC30subrsp,48       C5F877vzeroupper       488BDAmovrbx,rdxG_M63029_IG02:       C5F857C0             vxorpsxmm0,xmm0       C5FA7F442420         vmovdqu  xmmword ptr[rsp+20H],xmm0       4D85C0testr8,r87432je       SHORT G_M63029_IG04       418378080Acmp      dword ptr[r8+8],10       722Bjb       SHORT G_M63029_IG04       4983C010addr8,16       4C89442420mov      bword ptr[rsp+20H],r8       C74424280A000000mov      dword ptr[rsp+28H],10       488BFBmovrdi,rbx       488D742420learsi, bword ptr[rsp+20H]       E8DF738B5Fcall     CORINFO_HELP_ASSIGN_BYREF       48A5movsq       488BC3movrax,rbxG_M63029_IG03:       4883C430addrsp,48       5Bpoprbx       5Epoprsi       5Fpoprdi       C3retG_M63029_IG04:       FF158CDB0B00call[System.ThrowHelper:ThrowArgumentOutOfRangeException()]       CCint3; Total bytes of code: 85

perf-triage team (@DrewScoggins,@tannergooding,@kunalspathak) noticed that this PR caused a lot of perf regressions

@tannergooding
Copy link
Member

tannergooding commentedJul 5, 2022•
edited
Loading

NotablySuperPMI says "zero diffs" which is interesting. Possibly two changes not playing well together or maybe something SuperPMI doesn't catch?

@kunalspathak
Copy link
Contributor

or maybe something SuperPMI doesn't catch?

@BruceForstall

@jakobbotsch
Copy link
Member

NotablySuperPMI says "zero diffs" which is interesting. Possibly two changes not playing well together or maybe something SuperPMI doesn't catch?

Since all the significant changes here are on the VM side that is to be expected. SPMI will only reflect JIT changes.

tannergooding reacted with thumbs up emoji

@DrewScoggins
Copy link
Member

We are also seeing over 10% regressions in plaintext TE

image

This was referencedJul 18, 2022
@ghostghost locked asresolvedand limited conversation to collaboratorsAug 12, 2022
@jeffhandleyjeffhandley added runtime-coreclrspecific to the CoreCLR runtime and removed CoreClr labelsDec 28, 2022
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers

@steveharterstevehartersteveharter left review comments

@Sergio0694Sergio0694Sergio0694 left review comments

@lambdageeklambdageeklambdageek approved these changes

@stephentoubstephentoubstephentoub approved these changes

@BrzVladBrzVladAwaiting requested review from BrzVladBrzVlad is a code owner

@vargazvargazAwaiting requested review from vargaz

@SamMonoRTSamMonoRTAwaiting requested review from SamMonoRT

@thaystgthaystgAwaiting requested review from thaystgthaystg is a code owner

@marek-safarmarek-safarAwaiting requested review from marek-safar

@MichalStrehovskyMichalStrehovskyAwaiting requested review from MichalStrehovskyMichalStrehovsky is a code owner

Labels
area-Metaruntime-coreclrspecific to the CoreCLR runtime
Projects
None yet
Milestone
7.0.0
Development

Successfully merging this pull request may close these issues.

12 participants
@AaronRobinsonMSFT@lambdageek@stephentoub@BrzVlad@EgorBo@tannergooding@kunalspathak@jakobbotsch@DrewScoggins@steveharter@Sergio0694@jeffhandley
[8]ページ先頭
©2009-2025 Movatter.jp