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

Ensure that Vector<T> is tracked as "optimistic" for crossgen2 #87240

Merged
merged 1 commit into from
Jun 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 21 additions & 8 deletions src/coreclr/tools/Common/Compiler/InstructionSetSupport.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ public bool IsInstructionSetSupported(InstructionSet instructionSet)
return _supportedInstructionSets.HasInstructionSet(instructionSet);
}

public bool IsInstructionSetOptimisticallySupported(InstructionSet instructionSet)
{
return _optimisticInstructionSets.HasInstructionSet(instructionSet);
}

public bool IsInstructionSetExplicitlyUnsupported(InstructionSet instructionSet)
{
return _unsupportedInstructionSets.HasInstructionSet(instructionSet);
Expand Down Expand Up @@ -101,14 +106,14 @@ public SimdVectorLength GetVectorTSimdVector()
Debug.Assert(InstructionSet.X64_VectorT512 == InstructionSet.X86_VectorT512);

// TODO-XArch: Add support for 512-bit Vector<T>
Debug.Assert(!IsInstructionSetSupported(InstructionSet.X64_VectorT512));
Debug.Assert(!IsInstructionSetOptimisticallySupported(InstructionSet.X64_VectorT512));

if (IsInstructionSetSupported(InstructionSet.X64_VectorT256))
if (IsInstructionSetOptimisticallySupported(InstructionSet.X64_VectorT256))
{
Debug.Assert(!IsInstructionSetSupported(InstructionSet.X64_VectorT128));
Debug.Assert(!IsInstructionSetOptimisticallySupported(InstructionSet.X64_VectorT128));
return SimdVectorLength.Vector256Bit;
}
else if (IsInstructionSetSupported(InstructionSet.X64_VectorT128))
else if (IsInstructionSetOptimisticallySupported(InstructionSet.X64_VectorT128))
{
return SimdVectorLength.Vector128Bit;
}
Expand All @@ -119,7 +124,7 @@ public SimdVectorLength GetVectorTSimdVector()
}
else if (_targetArchitecture == TargetArchitecture.ARM64)
{
if (IsInstructionSetSupported(InstructionSet.ARM64_VectorT128))
if (IsInstructionSetOptimisticallySupported(InstructionSet.ARM64_VectorT128))
{
return SimdVectorLength.Vector128Bit;
}
Expand Down Expand Up @@ -274,6 +279,7 @@ public bool RemoveInstructionSetSupport(string instructionSet)
/// </summary>
/// <returns>returns "false" if instruction set isn't valid on this architecture</returns>
public bool ComputeInstructionSetFlags(int maxVectorTBitWidth,
bool skipAddingVectorT,
out InstructionSetFlags supportedInstructionSets,
out InstructionSetFlags unsupportedInstructionSets,
Action<string, string> invalidInstructionSetImplication)
Expand Down Expand Up @@ -317,6 +323,16 @@ public bool ComputeInstructionSetFlags(int maxVectorTBitWidth,
}
}

if (skipAddingVectorT)
{
// For partial AOT scenarios, we need to skip adding Vector<T>
// in the supported set so it doesn't cause the entire image
// to be thrown away due to the host machine supporting a larger
// size.

return true;
}

switch (_architecture)
{
case TargetArchitecture.X64:
Expand All @@ -343,9 +359,6 @@ public bool ComputeInstructionSetFlags(int maxVectorTBitWidth,
{
supportedInstructionSets.RemoveInstructionSet(InstructionSet.X86_VectorT128);
supportedInstructionSets.AddInstructionSet(InstructionSet.X86_VectorT256);

unsupportedInstructionSets.AddInstructionSet(InstructionSet.X86_VectorT128);
unsupportedInstructionSets.AddInstructionSet(InstructionSet.X86_VectorT512);
}

// TODO-XArch: Add support for 512-bit Vector<T>
Expand Down
20 changes: 17 additions & 3 deletions src/coreclr/tools/Common/InstructionSetHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ namespace System.CommandLine
{
internal static partial class Helpers
{
public static InstructionSetSupport ConfigureInstructionSetSupport(string instructionSet, int maxVectorTBitWidth, TargetArchitecture targetArchitecture, TargetOS targetOS,
public static InstructionSetSupport ConfigureInstructionSetSupport(string instructionSet, int maxVectorTBitWidth, bool isVectorTOptimistic, TargetArchitecture targetArchitecture, TargetOS targetOS,
string mustNotBeMessage, string invalidImplicationMessage)
{
InstructionSetSupportBuilder instructionSetSupportBuilder = new(targetArchitecture);
Expand Down Expand Up @@ -74,7 +74,20 @@ public static InstructionSetSupport ConfigureInstructionSetSupport(string instru
}
}

instructionSetSupportBuilder.ComputeInstructionSetFlags(maxVectorTBitWidth, out var supportedInstructionSet, out var unsupportedInstructionSet,
// When we are in a fully AOT scenario, such as NativeAOT, then Vector<T>
// can be directly part of the supported ISAs. This is because we are targeting
// an exact machine and we won't have any risk of a more capable machine supporting
// a larger Vector<T> and needing to invalidate any methods pre-compiled targeting
// smaller sizes.
//
// However, when we are in a partial AOT scenario, such as Crossgen2, then
// Vector<T> must only appear in the optimistic set since the size supported
// by the pre-compiled code may be smaller (or larger) than what is actually
// supported at runtime.

bool skipAddingVectorT = isVectorTOptimistic;

instructionSetSupportBuilder.ComputeInstructionSetFlags(maxVectorTBitWidth, skipAddingVectorT, out var supportedInstructionSet, out var unsupportedInstructionSet,
(string specifiedInstructionSet, string impliedInstructionSet) =>
throw new CommandLineException(string.Format(invalidImplicationMessage, specifiedInstructionSet, impliedInstructionSet)));

Expand Down Expand Up @@ -144,7 +157,8 @@ public static InstructionSetSupport ConfigureInstructionSetSupport(string instru
optimisticInstructionSetSupportBuilder.AddSupportedInstructionSet("rcpc");
}

optimisticInstructionSetSupportBuilder.ComputeInstructionSetFlags(maxVectorTBitWidth, out var optimisticInstructionSet, out _,
// Vector<T> can always be part of the optimistic set, we only want to optionally exclude it from the supported set
optimisticInstructionSetSupportBuilder.ComputeInstructionSetFlags(maxVectorTBitWidth, skipAddingVectorT: false, out var optimisticInstructionSet, out _,
(string specifiedInstructionSet, string impliedInstructionSet) => throw new NotSupportedException());
optimisticInstructionSet.Remove(unsupportedInstructionSet);
optimisticInstructionSet.Add(supportedInstructionSet);
Expand Down
10 changes: 9 additions & 1 deletion src/coreclr/tools/aot/ILCompiler/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,17 @@ public int Run()
if (outputFilePath == null)
throw new CommandLineException("Output filename must be specified (/out <file>)");

// NativeAOT is full AOT and its pre-compiled methods can not be
// thrown away at runtime if they mismatch in required ISAs or
// computed layouts of structs. The worst case scenario is simply
// that the image targets a higher machine than the user has and
// it fails to launch. Thus we want to have usage of Vector<T>
// directly encoded as part of the required ISAs.
bool isVectorTOptimistic = false;

TargetArchitecture targetArchitecture = Get(_command.TargetArchitecture);
TargetOS targetOS = Get(_command.TargetOS);
InstructionSetSupport instructionSetSupport = Helpers.ConfigureInstructionSetSupport(Get(_command.InstructionSet), Get(_command.MaxVectorTBitWidth), targetArchitecture, targetOS,
InstructionSetSupport instructionSetSupport = Helpers.ConfigureInstructionSetSupport(Get(_command.InstructionSet), Get(_command.MaxVectorTBitWidth), isVectorTOptimistic, targetArchitecture, targetOS,
"Unrecognized instruction set {0}", "Unsupported combination of instruction sets: {0}/{1}");

string systemModuleName = Get(_command.SystemModuleName);
Expand Down
9 changes: 8 additions & 1 deletion src/coreclr/tools/aot/crossgen2/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,16 @@ public int Run()
if (_singleFileCompilation && !_outNearInput)
throw new CommandLineException(SR.MissingOutNearInput);

// Crossgen2 is partial AOT and its pre-compiled methods can be
// thrown away at runtime if they mismatch in required ISAs or
// computed layouts of structs. Thus we want to ensure that usage
// of Vector<T> is only optimistic and doesn't hard code a dependency
// that would cause the entire image to be invalidated.
bool isVectorTOptimistic = true;

TargetArchitecture targetArchitecture = Get(_command.TargetArchitecture);
TargetOS targetOS = Get(_command.TargetOS);
InstructionSetSupport instructionSetSupport = Helpers.ConfigureInstructionSetSupport(Get(_command.InstructionSet), Get(_command.MaxVectorTBitWidth), targetArchitecture, targetOS,
InstructionSetSupport instructionSetSupport = Helpers.ConfigureInstructionSetSupport(Get(_command.InstructionSet), Get(_command.MaxVectorTBitWidth), isVectorTOptimistic, targetArchitecture, targetOS,
SR.InstructionSetMustNotBe, SR.InstructionSetInvalidImplication);
SharedGenericsMode genericsMode = SharedGenericsMode.CanonicalReferenceTypes;
var targetDetails = new TargetDetails(targetArchitecture, targetOS, Crossgen2RootCommand.IsArmel ? TargetAbi.NativeAotArmel : TargetAbi.NativeAot, instructionSetSupport.GetVectorTSimdVector());
Expand Down