From 78b45adbf20ab365f86d3b81c067d90719eaa7d0 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Michal=20Strehovsk=C3=BD?= Date: Mon, 24 Jul 2023 10:54:30 +0900 Subject: [PATCH] Don't always do optimistic HW intrinsic expansion (#89282) "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 | 12 ++++++++++-- src/coreclr/tools/aot/ILCompiler/Program.cs | 3 ++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/coreclr/tools/Common/InstructionSetHelpers.cs b/src/coreclr/tools/Common/InstructionSetHelpers.cs index 124d71e..089f112 100644 --- a/src/coreclr/tools/Common/InstructionSetHelpers.cs +++ b/src/coreclr/tools/Common/InstructionSetHelpers.cs @@ -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 diff --git a/src/coreclr/tools/aot/ILCompiler/Program.cs b/src/coreclr/tools/aot/ILCompiler/Program.cs index e8e8080..ed7b013 100644 --- a/src/coreclr/tools/aot/ILCompiler/Program.cs +++ b/src/coreclr/tools/aot/ILCompiler/Program.cs @@ -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); -- 2.7.4