From a7094a5cb2ce6498cc9dd0af55f6c2783347fbd3 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Thu, 8 Jun 2023 20:52:42 -0700 Subject: [PATCH] Ensure that Vector is tracked as "optimistic" for crossgen2 (#87240) --- .../tools/Common/Compiler/InstructionSetSupport.cs | 29 ++++++++++++++++------ src/coreclr/tools/Common/InstructionSetHelpers.cs | 20 ++++++++++++--- src/coreclr/tools/aot/ILCompiler/Program.cs | 10 +++++++- src/coreclr/tools/aot/crossgen2/Program.cs | 9 ++++++- 4 files changed, 55 insertions(+), 13 deletions(-) diff --git a/src/coreclr/tools/Common/Compiler/InstructionSetSupport.cs b/src/coreclr/tools/Common/Compiler/InstructionSetSupport.cs index 4652838..f2805e0 100644 --- a/src/coreclr/tools/Common/Compiler/InstructionSetSupport.cs +++ b/src/coreclr/tools/Common/Compiler/InstructionSetSupport.cs @@ -37,6 +37,11 @@ namespace ILCompiler return _supportedInstructionSets.HasInstructionSet(instructionSet); } + public bool IsInstructionSetOptimisticallySupported(InstructionSet instructionSet) + { + return _optimisticInstructionSets.HasInstructionSet(instructionSet); + } + public bool IsInstructionSetExplicitlyUnsupported(InstructionSet instructionSet) { return _unsupportedInstructionSets.HasInstructionSet(instructionSet); @@ -101,14 +106,14 @@ namespace ILCompiler Debug.Assert(InstructionSet.X64_VectorT512 == InstructionSet.X86_VectorT512); // TODO-XArch: Add support for 512-bit Vector - 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; } @@ -119,7 +124,7 @@ namespace ILCompiler } else if (_targetArchitecture == TargetArchitecture.ARM64) { - if (IsInstructionSetSupported(InstructionSet.ARM64_VectorT128)) + if (IsInstructionSetOptimisticallySupported(InstructionSet.ARM64_VectorT128)) { return SimdVectorLength.Vector128Bit; } @@ -274,6 +279,7 @@ namespace ILCompiler /// /// returns "false" if instruction set isn't valid on this architecture public bool ComputeInstructionSetFlags(int maxVectorTBitWidth, + bool skipAddingVectorT, out InstructionSetFlags supportedInstructionSets, out InstructionSetFlags unsupportedInstructionSets, Action invalidInstructionSetImplication) @@ -317,6 +323,16 @@ namespace ILCompiler } } + if (skipAddingVectorT) + { + // For partial AOT scenarios, we need to skip adding Vector + // 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: @@ -343,9 +359,6 @@ namespace ILCompiler { 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 diff --git a/src/coreclr/tools/Common/InstructionSetHelpers.cs b/src/coreclr/tools/Common/InstructionSetHelpers.cs index c969927..fb67260 100644 --- a/src/coreclr/tools/Common/InstructionSetHelpers.cs +++ b/src/coreclr/tools/Common/InstructionSetHelpers.cs @@ -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); @@ -74,7 +74,20 @@ namespace System.CommandLine } } - instructionSetSupportBuilder.ComputeInstructionSetFlags(maxVectorTBitWidth, out var supportedInstructionSet, out var unsupportedInstructionSet, + // When we are in a fully AOT scenario, such as NativeAOT, then Vector + // 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 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 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))); @@ -144,7 +157,8 @@ namespace System.CommandLine optimisticInstructionSetSupportBuilder.AddSupportedInstructionSet("rcpc"); } - optimisticInstructionSetSupportBuilder.ComputeInstructionSetFlags(maxVectorTBitWidth, out var optimisticInstructionSet, out _, + // Vector 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); diff --git a/src/coreclr/tools/aot/ILCompiler/Program.cs b/src/coreclr/tools/aot/ILCompiler/Program.cs index f83ad61..9f0af5d 100644 --- a/src/coreclr/tools/aot/ILCompiler/Program.cs +++ b/src/coreclr/tools/aot/ILCompiler/Program.cs @@ -65,9 +65,17 @@ namespace ILCompiler if (outputFilePath == null) throw new CommandLineException("Output filename must be specified (/out )"); + // 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 + // 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); diff --git a/src/coreclr/tools/aot/crossgen2/Program.cs b/src/coreclr/tools/aot/crossgen2/Program.cs index 1d43fc8..ff166ed 100644 --- a/src/coreclr/tools/aot/crossgen2/Program.cs +++ b/src/coreclr/tools/aot/crossgen2/Program.cs @@ -74,9 +74,16 @@ namespace ILCompiler 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 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()); -- 2.7.4