Skip to content

Commit 6fd3636

Browse files
authored
[r2r] Fix rule for skipping compilation of methods with CompExactlyDependsOn (#125372)
If a method has `CompExactlyDependsOn` attribute, it means the correctness of its code depends on matching support for the instruction set between r2r compilation time and run time jit compilation. For the example of ShuffleNativeModified: ``` [CompExactlyDependsOn(typeof(Ssse3))] internal static Vector128<byte> ShuffleNativeModified(Vector128<byte> vector, Vector128<byte> indices) { if (Ssse3.IsSupported) return Ssse3.Shuffle(vector, indices); return Vector128.Shuffle(vector, indices); } ``` Ssse3 is an intel specific instruction set that is never used on arm64. The check in code was returning an ILLEGAL instruction set. This case was treated as `continue` instead of setting `doBypass` to false. Because of this, this method was skipped from r2r compilation. We now treat the ILLEGAL case as known compile time support, so we don't skip the method from r2r compilation. The fix avoids interpreting this method on ios-arm64, making JSON serialization/deserialization 2x faster.
1 parent fc52927 commit 6fd3636

File tree

5 files changed

+44
-16
lines changed

5 files changed

+44
-16
lines changed

src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -594,6 +594,7 @@ public static bool ShouldCodeNotBeCompiledIntoFinalImage(InstructionSetSupport i
594594
var handle = ecmaMethod.Handle;
595595

596596
List<TypeDesc> compExactlyDependsOnList = null;
597+
bool hasCompHasFallback = false;
597598

598599
foreach (var attributeHandle in metadataReader.GetMethodDefinition(handle).GetCustomAttributes())
599600
{
@@ -627,35 +628,41 @@ public static bool ShouldCodeNotBeCompiledIntoFinalImage(InstructionSetSupport i
627628
compExactlyDependsOnList.Add(typeForBypass);
628629
}
629630
}
631+
else if (metadataReader.StringComparer.Equals(nameHandle, "CompHasFallbackAttribute"))
632+
{
633+
hasCompHasFallback = true;
634+
}
630635
}
631636
}
632637

633638
if (compExactlyDependsOnList != null && compExactlyDependsOnList.Count > 0)
634639
{
635-
// Default to true, and set to false if at least one of the types is actually supported in the current environment, and none of the
636-
// intrinsic types are in an opportunistic state.
637-
bool doBypass = true;
640+
bool anySupported = false;
638641

639642
foreach (var intrinsicType in compExactlyDependsOnList)
640643
{
641644
InstructionSet instructionSet = InstructionSetParser.LookupPlatformIntrinsicInstructionSet(intrinsicType.Context.Target.Architecture, intrinsicType);
642-
if (instructionSet == InstructionSet.ILLEGAL)
643-
{
644-
// This instruction set isn't supported on the current platform at all.
645-
continue;
646-
}
647-
if (instructionSetSupport.IsInstructionSetSupported(instructionSet) || instructionSetSupport.IsInstructionSetExplicitlyUnsupported(instructionSet))
648-
{
649-
doBypass = false;
650-
}
651-
else
645+
// If the instruction set is ILLEGAL, it means it is never supported by the current architecture so the behavior at runtime is known
646+
if (instructionSet != InstructionSet.ILLEGAL)
652647
{
653-
// If we reach here this is an instruction set generally supported on this platform, but we don't know what the behavior will be at runtime
654-
return true;
648+
if (instructionSetSupport.IsInstructionSetSupported(instructionSet))
649+
{
650+
anySupported = true;
651+
}
652+
else if (!instructionSetSupport.IsInstructionSetExplicitlyUnsupported(instructionSet))
653+
{
654+
// If we reach here this is an instruction set generally supported on this platform, but we don't know what the behavior will be at runtime
655+
return true;
656+
}
655657
}
656658
}
657659

658-
return doBypass;
660+
if (!anySupported && !hasCompHasFallback)
661+
{
662+
// If none of the instruction sets are supported (all are either illegal or explicitly unsupported),
663+
// skip compilation unless the method has a functional fallback path
664+
return true;
665+
}
659666
}
660667

661668
// No reason to bypass compilation and code generation.

src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -849,6 +849,7 @@
849849
<Compile Include="$(MSBuildThisFileDirectory)System\Runtime\CompilerServices\CallingConventions.cs" />
850850
<Compile Include="$(MSBuildThisFileDirectory)System\Runtime\CompilerServices\CollectionBuilderAttribute.cs" />
851851
<Compile Include="$(MSBuildThisFileDirectory)System\Runtime\CompilerServices\CompExactlyDependsOnAttribute.cs" />
852+
<Compile Include="$(MSBuildThisFileDirectory)System\Runtime\CompilerServices\CompHasFallbackAttribute.cs" />
852853
<Compile Include="$(MSBuildThisFileDirectory)System\Runtime\CompilerServices\CompilationRelaxations.cs" />
853854
<Compile Include="$(MSBuildThisFileDirectory)System\Runtime\CompilerServices\CompilationRelaxationsAttribute.cs" />
854855
<Compile Include="$(MSBuildThisFileDirectory)System\Runtime\CompilerServices\CompilerFeatureRequiredAttribute.cs" />
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System.Diagnostics;
5+
6+
namespace System.Runtime.CompilerServices
7+
{
8+
// Use this attribute alongside CompExactlyDependsOnAttribute to indicate that a method has
9+
// a functional fallback path and should still be compiled into the Ready2Run image even when
10+
// none of the CompExactlyDependsOn instruction sets are supported.
11+
[AttributeUsage(AttributeTargets.Method | AttributeTargets.Constructor, AllowMultiple = false, Inherited = false)]
12+
#if MONO
13+
[Conditional("unnecessary")] // Mono doesn't use Ready2Run so we can remove this attribute to reduce size
14+
#endif
15+
internal sealed class CompHasFallbackAttribute : Attribute
16+
{
17+
}
18+
}

src/libraries/System.Private.CoreLib/src/System/SearchValues/SearchValues.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,7 @@ internal interface IRuntimeConst
301301
/// </summary>
302302
[MethodImpl(MethodImplOptions.AggressiveInlining)]
303303
[CompExactlyDependsOn(typeof(Ssse3))]
304+
[CompHasFallback]
304305
internal static Vector128<byte> ShuffleNativeModified(Vector128<byte> vector, Vector128<byte> indices)
305306
{
306307
if (Ssse3.IsSupported)

src/libraries/System.Private.CoreLib/src/System/Text/Ascii.Utility.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1632,6 +1632,7 @@ private static bool AllCharsInVectorAreAscii<T>(Vector128<T> vector)
16321632

16331633
[MethodImpl(MethodImplOptions.AggressiveInlining)]
16341634
[CompExactlyDependsOn(typeof(Avx))]
1635+
[CompHasFallback]
16351636
private static bool AllCharsInVectorAreAscii<T>(Vector256<T> vector)
16361637
where T : unmanaged
16371638
{

0 commit comments

Comments
 (0)