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.

Sign up for GitHub

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

Jump to bottom

Enable R2R for NarrowUtf16ToAscii / WidentAsciiToUtf16 #90361

Merged
merged 2 commits into from
Aug 11, 2023
Merged

Enable R2R for NarrowUtf16ToAscii / WidentAsciiToUtf16 #90361

merged 2 commits into from
Aug 11, 2023

Conversation

stephentoub
Copy link
Member

Currently both of these methods on System.Text.Ascii are on the startup path and are being JIT'd:

   1: JIT compiled System.Runtime.CompilerServices.CastHelpers:StelemRef(System.Array,long,System.Object) [Tier1, IL size=88, code size=93]
   2: JIT compiled System.Runtime.CompilerServices.CastHelpers:LdelemaRef(System.Array,long,ulong) [Tier1, IL size=44, code size=44]
   3: JIT compiled System.Guid:FormatGuidVector128Utf8(System.Guid,bool) [Tier0, IL size=322, code size=584]
   4: JIT compiled System.HexConverter:AsciiToHexVector128(System.Runtime.Intrinsics.Vector128`1[ubyte],System.Runtime.Intrinsics.Vector128`1[ubyte]) [Tier0, IL size=78, code size=359]
   5: JIT compiled System.Runtime.Intrinsics.Vector128:ShuffleUnsafe(System.Runtime.Intrinsics.Vector128`1[ubyte],System.Runtime.Intrinsics.Vector128`1[ubyte]) [Tier0, IL size=41, code size=50]
   6: JIT compiled System.Text.Unicode.Utf16Utility:GetPointerToFirstInvalidChar(ulong,int,byref,byref) [Instrumented Tier0, IL size=994, code size=1292]
   7: JIT compiled System.Text.Ascii:NarrowUtf16ToAscii(ulong,ulong,ulong) [Instrumented Tier0, IL size=558, code size=1327]
   8: JIT compiled System.RuntimeType+RuntimeTypeCache+MemberInfoCache`1[System.__Canon]:AddMethod(System.RuntimeType,long,int) [Instrumented Tier0, IL size=354, code size=1672]
   9: JIT compiled System.Text.Ascii:WidenAsciiToUtf16(ulong,ulong,ulong) [Instrumented Tier0, IL size=738, code size=1741]

They're not using R2R because of use of Vector<T>. But the Vector<T> block will only be used if either on big-endian or if Vector128 isn't accelerated. If Vector128 isn't accelerated, then given the current state of our target platforms and backends, Vector<T> won't be used, either, which means the Vector<T> block is only for big endian. By adding a guard clause for big endian, then, R2R starts being used for these methods:

   1: JIT compiled System.Runtime.CompilerServices.CastHelpers:StelemRef(System.Array,long,System.Object) [Tier1, IL size=88, code size=93]
   2: JIT compiled System.Runtime.CompilerServices.CastHelpers:LdelemaRef(System.Array,long,ulong) [Tier1, IL size=44, code size=44]
   3: JIT compiled System.Guid:FormatGuidVector128Utf8(System.Guid,bool) [Tier0, IL size=322, code size=584]
   4: JIT compiled System.HexConverter:AsciiToHexVector128(System.Runtime.Intrinsics.Vector128`1[ubyte],System.Runtime.Intrinsics.Vector128`1[ubyte]) [Tier0, IL size=78, code size=359]
   5: JIT compiled System.Runtime.Intrinsics.Vector128:ShuffleUnsafe(System.Runtime.Intrinsics.Vector128`1[ubyte],System.Runtime.Intrinsics.Vector128`1[ubyte]) [Tier0, IL size=41, code size=50]
   6: JIT compiled System.Text.Unicode.Utf16Utility:GetPointerToFirstInvalidChar(ulong,int,byref,byref) [Instrumented Tier0, IL size=994, code size=1292]
   7: JIT compiled System.RuntimeType+RuntimeTypeCache+MemberInfoCache`1[System.__Canon]:AddMethod(System.RuntimeType,long,int) [Instrumented Tier0, IL size=354, code size=1672]

@EgorBo, I am curious why they were showing up as "Instrumented Tier0" without another "Tier0" entry for them first. What causes a non-instrumented tier0 to be skipped?

@ghost ghost assigned stephentoub Aug 11, 2023
@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Aug 11, 2023
@stephentoub stephentoub requested a review from jkotas August 11, 2023 01:11
@stephentoub stephentoub added this to the 8.0.0 milestone Aug 11, 2023
@jkotas
Copy link
Member

jkotas commented Aug 11, 2023

given the current state of our target platforms and backends, Vector won't be used, either, which means the Vector block is only for big endian

s390x that is only big endian arch does not support Vector acceleration. Is the Vector block effectively dead code given the state of our platforms and backends?

@stephentoub
Copy link
Member Author

s390x that is only big endian arch does not support Vector acceleration.

In that case, yes, I believe these paths are dead. I'll delete 'em instead.

@stephentoub stephentoub added area-System.Runtime and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Aug 11, 2023
@ghost
Copy link

ghost commented Aug 11, 2023

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

Issue Details

Currently both of these methods on System.Text.Ascii are on the startup path and are being JIT'd:

   1: JIT compiled System.Runtime.CompilerServices.CastHelpers:StelemRef(System.Array,long,System.Object) [Tier1, IL size=88, code size=93]
   2: JIT compiled System.Runtime.CompilerServices.CastHelpers:LdelemaRef(System.Array,long,ulong) [Tier1, IL size=44, code size=44]
   3: JIT compiled System.Guid:FormatGuidVector128Utf8(System.Guid,bool) [Tier0, IL size=322, code size=584]
   4: JIT compiled System.HexConverter:AsciiToHexVector128(System.Runtime.Intrinsics.Vector128`1[ubyte],System.Runtime.Intrinsics.Vector128`1[ubyte]) [Tier0, IL size=78, code size=359]
   5: JIT compiled System.Runtime.Intrinsics.Vector128:ShuffleUnsafe(System.Runtime.Intrinsics.Vector128`1[ubyte],System.Runtime.Intrinsics.Vector128`1[ubyte]) [Tier0, IL size=41, code size=50]
   6: JIT compiled System.Text.Unicode.Utf16Utility:GetPointerToFirstInvalidChar(ulong,int,byref,byref) [Instrumented Tier0, IL size=994, code size=1292]
   7: JIT compiled System.Text.Ascii:NarrowUtf16ToAscii(ulong,ulong,ulong) [Instrumented Tier0, IL size=558, code size=1327]
   8: JIT compiled System.RuntimeType+RuntimeTypeCache+MemberInfoCache`1[System.__Canon]:AddMethod(System.RuntimeType,long,int) [Instrumented Tier0, IL size=354, code size=1672]
   9: JIT compiled System.Text.Ascii:WidenAsciiToUtf16(ulong,ulong,ulong) [Instrumented Tier0, IL size=738, code size=1741]

They're not using R2R because of use of Vector<T>. But the Vector<T> block will only be used if either on big-endian or if Vector128 isn't accelerated. If Vector128 isn't accelerated, then given the current state of our target platforms and backends, Vector<T> won't be used, either, which means the Vector<T> block is only for big endian. By adding a guard clause for big endian, then, R2R starts being used for these methods:

   1: JIT compiled System.Runtime.CompilerServices.CastHelpers:StelemRef(System.Array,long,System.Object) [Tier1, IL size=88, code size=93]
   2: JIT compiled System.Runtime.CompilerServices.CastHelpers:LdelemaRef(System.Array,long,ulong) [Tier1, IL size=44, code size=44]
   3: JIT compiled System.Guid:FormatGuidVector128Utf8(System.Guid,bool) [Tier0, IL size=322, code size=584]
   4: JIT compiled System.HexConverter:AsciiToHexVector128(System.Runtime.Intrinsics.Vector128`1[ubyte],System.Runtime.Intrinsics.Vector128`1[ubyte]) [Tier0, IL size=78, code size=359]
   5: JIT compiled System.Runtime.Intrinsics.Vector128:ShuffleUnsafe(System.Runtime.Intrinsics.Vector128`1[ubyte],System.Runtime.Intrinsics.Vector128`1[ubyte]) [Tier0, IL size=41, code size=50]
   6: JIT compiled System.Text.Unicode.Utf16Utility:GetPointerToFirstInvalidChar(ulong,int,byref,byref) [Instrumented Tier0, IL size=994, code size=1292]
   7: JIT compiled System.RuntimeType+RuntimeTypeCache+MemberInfoCache`1[System.__Canon]:AddMethod(System.RuntimeType,long,int) [Instrumented Tier0, IL size=354, code size=1672]

@EgorBo, I am curious why they were showing up as "Instrumented Tier0" without another "Tier0" entry for them first. What causes a non-instrumented tier0 to be skipped?

Author: stephentoub
Assignees: stephentoub
Labels:

area-System.Runtime
Milestone: 8.0.0

jkotas
jkotas approved these changes Aug 11, 2023
View reviewed changes
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.

Thank you!

@jkotas jkotas merged commit 336e122 into dotnet:main Aug 11, 2023
@EgorBo
Copy link
Member

EgorBo commented Aug 11, 2023

@EgorBo, I am curious why they were showing up as "Instrumented Tier0" without another "Tier0" entry for them first. What causes a non-instrumented tier0 to be skipped?

Methods with loops bypass Tier0 (but not R2R) and go straight to Tier0-instrumented - it's done to avoid cases when a cold method with a hot loop won't benefit from PGO as we can't have two OSR versions

@EgorBo
Copy link
Member

EgorBo commented Aug 11, 2023 •
edited
Loading

By adding a guard clause for big endian, then, R2R starts being used for these methods:

   1: JIT compiled System.Runtime.CompilerServices.CastHelpers:StelemRef(System.Array,long,System.Object) [Tier1, IL size=88, code size=93]
   2: JIT compiled System.Runtime.CompilerServices.CastHelpers:LdelemaRef(System.Array,long,ulong) [Tier1, IL size=44, code size=44]
   3: JIT compiled System.Guid:FormatGuidVector128Utf8(System.Guid,bool) [Tier0, IL size=322, code size=584]
   4: JIT compiled System.HexConverter:AsciiToHexVector128(System.Runtime.Intrinsics.Vector128`1[ubyte],System.Runtime.Intrinsics.Vector128`1[ubyte]) [Tier0, IL size=78, code size=359]
   5: JIT compiled System.Runtime.Intrinsics.Vector128:ShuffleUnsafe(System.Runtime.Intrinsics.Vector128`1[ubyte],System.Runtime.Intrinsics.Vector128`1[ubyte]) [Tier0, IL size=41, code size=50]
   6: JIT compiled System.Text.Unicode.Utf16Utility:GetPointerToFirstInvalidChar(ulong,int,byref,byref) [Instrumented Tier0, IL size=994, code size=1292]
   7: JIT compiled System.RuntimeType+RuntimeTypeCache+MemberInfoCache`1[System.__Canon]:AddMethod(System.RuntimeType,long,int) [Instrumented Tier0, IL size=354, code size=1672]

#90326 fixes 3rd ,4th, 5th methods. 7th method is weird, cg throws an exception when I try to prejit it 😐

@stephentoub stephentoub deleted the r2rascii branch August 11, 2023 11:07
@SamMonoRT
Copy link
Member

@LeVladIonescu - can you run a before/after on the Mono Interpreter config to make sure this is not a cause of the regressions seen in dotnet/perf-autofiling-issues#20737.

@radekdoulik radekdoulik mentioned this pull request Aug 30, 2023
Closed
@ghost ghost locked as resolved and limited conversation to collaborators Sep 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jkotas jkotas jkotas approved these changes

Assignees

@stephentoub stephentoub

Labels
area-System.Runtime
Projects
None yet
Milestone
8.0.0
Development

Successfully merging this pull request may close these issues.
4 participants
@stephentoub @jkotas @EgorBo @SamMonoRT