Don't always do optimistic HW intrinsic expansion (#89282)
authorMichal Strehovský <MichalStrehovsky@users.noreply.github.com>
Mon, 24 Jul 2023 01:54:30 +0000 (10:54 +0900)
committerGitHub <noreply@github.com>
Mon, 24 Jul 2023 01:54:30 +0000 (10:54 +0900)
"Optimistic" expansion means we generate a method body for `IsSupported` that returns true/false depending on the currently running CPU. Don't do this when `--instruction-set:native` was specified because native should mean "exactly this". Also don't do this when optimizing for size (saves 0.6% on hello world). I was going back and forth whether to do this when user specifies instruction sets manually, but decided it would be a "breaking" change.

src/coreclr/tools/Common/InstructionSetHelpers.cs
src/coreclr/tools/aot/ILCompiler/Program.cs

index 124d71e..089f112 100644 (file)
@@ -16,7 +16,7 @@ namespace System.CommandLine
     internal static partial class Helpers
     {
         public static InstructionSetSupport ConfigureInstructionSetSupport(string instructionSet, int maxVectorTBitWidth, bool isVectorTOptimistic, TargetArchitecture targetArchitecture, TargetOS targetOS,
-            string mustNotBeMessage, string invalidImplicationMessage, Logger logger)
+            string mustNotBeMessage, string invalidImplicationMessage, Logger logger, bool optimizingForSize = false)
         {
             InstructionSetSupportBuilder instructionSetSupportBuilder = new(targetArchitecture);
 
@@ -38,8 +38,16 @@ namespace System.CommandLine
                 }
             }
 
+            // Whether to allow optimistically expanding the instruction sets beyond what was specified.
+            // We seed this from optimizingForSize - if we're size-optimizing, we don't want to unnecessarily
+            // compile both branches of IsSupported checks.
+            bool allowOptimistic = !optimizingForSize;
+
             if (instructionSet == "native")
             {
+                // We're compiling for a specific chip
+                allowOptimistic = false;
+
                 if (GetTargetArchitecture(null) != targetArchitecture)
                 {
                     throw new CommandLineException("Instruction set 'native' not supported when cross-compiling to a different architecture.");
@@ -123,7 +131,7 @@ namespace System.CommandLine
             InstructionSetSupportBuilder optimisticInstructionSetSupportBuilder = new InstructionSetSupportBuilder(instructionSetSupportBuilder);
 
             // Optimistically assume some instruction sets are present.
-            if (targetArchitecture == TargetArchitecture.X86 || targetArchitecture == TargetArchitecture.X64)
+            if (allowOptimistic && (targetArchitecture == TargetArchitecture.X86 || targetArchitecture == TargetArchitecture.X64))
             {
                 // We set these hardware features as opportunistically enabled as most of hardware in the wild supports them.
                 // Note that we do not indicate support for AVX, or any other instruction set which uses the VEX encodings as
index e8e8080..ed7b013 100644 (file)
@@ -88,7 +88,8 @@ namespace ILCompiler
             TargetArchitecture targetArchitecture = Get(_command.TargetArchitecture);
             TargetOS targetOS = Get(_command.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}", logger);
+                "Unrecognized instruction set {0}", "Unsupported combination of instruction sets: {0}/{1}", logger,
+                optimizingForSize: _command.OptimizationMode == OptimizationMode.PreferSize);
 
             string systemModuleName = Get(_command.SystemModuleName);
             string reflectionData = Get(_command.ReflectionData);