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

Do we still need [AggressiveOptimization] on so many methods? #71261

Closed
stephentoub opened this issue Jun 24, 2022 · 6 comments
Closed

Do we still need [AggressiveOptimization] on so many methods? #71261

stephentoub opened this issue Jun 24, 2022 · 6 comments
Assignees
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Milestone

Comments

@stephentoub
Copy link
Member

stephentoub commented Jun 24, 2022

dotnet/coreclr#22191 added a bunch of [AggressiveOptimization] attributes to methods throughout SpanHelpers and elsewhere that make heavy use of intrinsics, e.g.

[MethodImpl(MethodImplOptions.AggressiveOptimization)]
public static bool Contains(ref byte searchSpace, byte value, int length)

After all of the inlining improvements made in .NET 6 and 7, are these still necessary and helping more than they hurt?

category:design
theme:basic-cq
skill-level:beginner
cost:small
impact:small

@stephentoub stephentoub added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jun 24, 2022
@stephentoub stephentoub added this to the 7.0.0 milestone Jun 24, 2022
@ghost
Copy link

ghost commented Jun 24, 2022

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

Issue Details

dotnet/coreclr#22191 added a bunch of [AggressiveOptimization] attributes to methods throughout SpanHelpers and elsewhere that make heavy use of intrinsics, e.g.

[MethodImpl(MethodImplOptions.AggressiveOptimization)]
public static bool Contains(ref byte searchSpace, byte value, int length)

After all of the inlining improvements made in .NET 6 and 7, are these still necessary and helping more than they hurt?

Author: stephentoub
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: 7.0.0

@jkotas
Copy link
Member

jkotas commented Jun 24, 2022

These [AggressiveOptimization] attributes were added as a workaround for #9120

@EgorBo
Copy link
Member

EgorBo commented Jun 26, 2022

I think we can remove them at least from SpanHelpers because SPC.dll is always prejitted (R2R) so e.g. SpanHelpers will never see Tier0 and will be properly instrumented and optimized for current app (in Dynamic PGO mode), e.g. if current app mostly works with large inputs we will re-order "handle small-inputs" blocks, etc.

However, we might need to check that SpanHelpers is actually fully prejitted due to presence of Vector<T> (see #67398 (comment))

@EgorBo
Copy link
Member

EgorBo commented Jun 26, 2022

SpanHelpers members which refuse to precompile (because they use Vector<T> without Vector128 and Vector256 paths)

Info: Method `[S.P.CoreLib]System.SpanHelpers.Fill<__Canon>(__Canon&,native uint,__Canon)` was not compiled because `[S.P.CoreLib]System.Numerics.Vector`1<uint8>` requires runtime JIT
Info: Method `[S.P.CoreLib]System.SpanHelpers.UnalignedCountVector(char&)` was not compiled because `This function is using SIMD intrinsics, their size is machine specific` requires runtime JIT
Info: Method `[S.P.CoreLib]System.SpanHelpers.GetCharVectorSpanLength(native int,native int)` was not compiled because `This function is using SIMD intrinsics, their size is machine specific` requires runtime JIT
Info: Method `[S.P.CoreLib]System.SpanHelpers.UnalignedCountVectorFromEnd(uint8&,int32)` was not compiled because `This function is using SIMD intrinsics, their size is machine specific` requires runtime JIT
Info: Method `[S.P.CoreLib]System.SpanHelpers.UnalignedCountVector(uint8&)` was not compiled because `This function is using SIMD intrinsics, their size is machine specific` requires runtime JIT
Info: Method `[S.P.CoreLib]System.SpanHelpers.GetByteVectorSpanLength(native uint,int32)` was not compiled because `This function is using SIMD intrinsics, their size is machine specific` requires runtime JIT
Info: Method `[S.P.CoreLib]System.SpanHelpers.LastIndexOfAny(uint8&,uint8,uint8,uint8,int32)` was not compiled because `[S.P.CoreLib]System.Numerics.Vector`1<uint8>` requires runtime JIT
Info: Method `[S.P.CoreLib]System.SpanHelpers.LastIndexOfAny(uint8&,uint8,uint8,int32)` was not compiled because `[S.P.CoreLib]System.Numerics.Vector`1<uint8>` requires runtime JIT
Info: Method `[S.P.CoreLib]System.SpanHelpers.Fill<char>(char&,native uint,char)` was not compiled because `[S.P.CoreLib]System.Numerics.Vector`1<uint8>` requires runtime JIT
Info: Method `[S.P.CoreLib]System.SpanHelpers.Fill<GuidResult>(GuidResult&,native uint,GuidResult)` was not compiled because `[S.P.CoreLib]System.Numerics.Vector`1<uint8>` requires runtime JIT
Info: Method `[S.P.CoreLib]System.SpanHelpers.Fill<native int>(native int&,native uint,native int)` was not compiled because `[S.P.CoreLib]System.Numerics.Vector`1<uint8>` requires runtime JIT
Info: Method `[S.P.CoreLib]System.SpanHelpers.Fill<int32>(int32&,native uint,int32)` was not compiled because `[S.P.CoreLib]System.Numerics.Vector`1<uint8>` requires runtime JIT
Info: Method `[S.P.CoreLib]System.SpanHelpers.Fill<float32>(float32&,native uint,float32)` was not compiled because `[S.P.CoreLib]System.Numerics.Vector`1<uint8>` requires runtime JIT
Info: Method `[S.P.CoreLib]System.SpanHelpers.Fill<uint16>(uint16&,native uint,uint16)` was not compiled because `[S.P.CoreLib]System.Numerics.Vector`1<uint8>` requires runtime JIT
Info: Method `[S.P.CoreLib]System.SpanHelpers.Fill<uint64>(uint64&,native uint,uint64)` was not compiled because `[S.P.CoreLib]System.Numerics.Vector`1<uint8>` requires runtime JIT
Info: Method `[S.P.CoreLib]System.SpanHelpers.Fill<uint32>(uint32&,native uint,uint32)` was not compiled because `[S.P.CoreLib]System.Numerics.Vector`1<uint8>` requires runtime JIT
Info: Method `[S.P.CoreLib]System.SpanHelpers.Fill<int64>(int64&,native uint,int64)` was not compiled because `[S.P.CoreLib]System.Numerics.Vector`1<uint8>` requires runtime JIT
Info: Method `[S.P.CoreLib]System.SpanHelpers.Fill<int16>(int16&,native uint,int16)` was not compiled because `[S.P.CoreLib]System.Numerics.Vector`1<uint8>` requires runtime JIT
Info: Method `[S.P.CoreLib]System.SpanHelpers.IndexOfValueType<int32>(int32&,int32,int32)` was not compiled because `[S.P.CoreLib]System.Numerics.Vector`1<int32>` requires runtime JIT
Info: Method `[S.P.CoreLib]System.SpanHelpers.IndexOfValueType<int64>(int64&,int64,int32)` was not compiled because `[S.P.CoreLib]System.Numerics.Vector`1<int64>` requires runtime JIT
Info: Method `[S.P.CoreLib]System.SpanHelpers.Fill<KeyValuePair`2<__Canon,__Canon>>(KeyValuePair`2<__Canon,__Canon>&,native uint,KeyValuePair`2<__Canon,__Canon>)` was not compiled because `[S.P.CoreLib]System.Numerics.Vector`1<uint8>` requires runtime JIT
Info: Method `[S.P.CoreLib]System.SpanHelpers.Fill<CalendarId>(CalendarId&,native uint,CalendarId)` was not compiled because `[S.P.CoreLib]System.Numerics.Vector`1<uint8>` requires runtime JIT
Info: Method `[S.P.CoreLib]System.SpanHelpers.Fill<float64>(float64&,native uint,float64)` was not compiled because `[S.P.CoreLib]System.Numerics.Vector`1<uint8>` requires runtime JIT
Info: Method `[S.P.CoreLib]System.SpanHelpers.Fill<KeyValuePair`2<SessionInfo,bool>>(KeyValuePair`2<SessionInfo,bool>&,native uint,KeyValuePair`2<SessionInfo,bool>)` was not compiled because `[S.P.CoreLib]System.Numerics.Vector`1<uint8>` requires runtime JIT
Info: Method `[S.P.CoreLib]System.SpanHelpers.Fill<KeyValuePair`2<int32,__Canon>>(KeyValuePair`2<int32,__Canon>&,native uint,KeyValuePair`2<int32,__Canon>)` was not compiled because `[S.P.CoreLib]System.Numerics.Vector`1<uint8>` requires runtime JIT
Info: Method `[S.P.CoreLib]System.SpanHelpers.Fill<SessionInfo>(SessionInfo&,native uint,SessionInfo)` was not compiled because `[S.P.CoreLib]System.Numerics.Vector`1<uint8>` requires runtime JIT

@EgorBo
Copy link
Member

EgorBo commented Jul 6, 2022

Moving to 8.0.0 as it depends on the planned change to make crossgen avx2-aware by default.

@EgorBo
Copy link
Member

EgorBo commented May 6, 2023

I assume we can close this one as we now have only 3 [AO] in BCL:

  1. AsyncTaskMethodBuilder<TResult>.AwaitUnsafeOnCompleted the comment states that it allocates in T0 (presumably, when R2R doesn't hande a specific generic instance)
  2. (and 3) CastHelper's StelemRef and LdelemaRef -- these have to be [AO] because VM special case them to be direct calls (so there is no call counting in between) - we can make them indirect just like 99% of other methods but VSadov claimed it is needed for perf.

@EgorBo EgorBo closed this as completed May 6, 2023
@EgorBo EgorBo modified the milestones: Future, 8.0.0 May 6, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Jun 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

No branches or pull requests

3 participants