From 689cf795574806929be152cdcb3aadcd632be544 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Thu, 14 Mar 2024 13:36:36 +0100 Subject: [PATCH 1/7] Fix SPMI collect (#99762) --- src/coreclr/inc/jiteeversionguid.h | 10 +++++----- src/coreclr/tools/superpmi/superpmi-shared/agnostic.h | 4 ++-- .../tools/superpmi/superpmi-shared/methodcontext.cpp | 7 ++++--- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/coreclr/inc/jiteeversionguid.h b/src/coreclr/inc/jiteeversionguid.h index 9022ceff18bc54..41e58ca24537b8 100644 --- a/src/coreclr/inc/jiteeversionguid.h +++ b/src/coreclr/inc/jiteeversionguid.h @@ -43,11 +43,11 @@ typedef const GUID *LPCGUID; #define GUID_DEFINED #endif // !GUID_DEFINED -constexpr GUID JITEEVersionIdentifier = { /* 35afdf61-5417-4bd7-9302-48efa2507603 */ - 0x35afdf61, - 0x5417, - 0x4bd7, - {0x93, 0x02, 0x48, 0xef, 0xa2, 0x50, 0x76, 0x03} +constexpr GUID JITEEVersionIdentifier = { /* 6fd660c7-96be-4832-a84c-4200141f7d08 */ + 0x6fd660c7, + 0x96be, + 0x4832, + {0xa8, 0x4c, 0x42, 0x00, 0x14, 0x1f, 0x7d, 0x08} }; ////////////////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/src/coreclr/tools/superpmi/superpmi-shared/agnostic.h b/src/coreclr/tools/superpmi/superpmi-shared/agnostic.h index ffd657bea503f0..104a7ae1fabd39 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/agnostic.h +++ b/src/coreclr/tools/superpmi/superpmi-shared/agnostic.h @@ -429,8 +429,8 @@ struct Agnostic_EmbedGenericHandle struct Agnostic_ExpandRawHandleIntrinsic { - Agnostic_CORINFO_RESOLVED_TOKEN ResolvedToken; - DWORDLONG hCallerHandle; + Agnostic_CORINFO_RESOLVED_TOKENin ResolvedToken; + DWORDLONG hCallerHandle; }; struct Agnostic_CORINFO_GENERICHANDLE_RESULT diff --git a/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp b/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp index 6b5931a826374c..70c5c627bd404c 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp @@ -1677,7 +1677,7 @@ void MethodContext::recExpandRawHandleIntrinsic(CORINFO_RESOLVED_TOKEN* pResolve Agnostic_ExpandRawHandleIntrinsic key; ZeroMemory(&key, sizeof(key)); // Zero key including any struct padding - key.ResolvedToken = SpmiRecordsHelper::StoreAgnostic_CORINFO_RESOLVED_TOKEN(pResolvedToken, ExpandRawHandleIntrinsic); + key.ResolvedToken = SpmiRecordsHelper::CreateAgnostic_CORINFO_RESOLVED_TOKENin(pResolvedToken); key.hCallerHandle = CastHandle(callerHandle); Agnostic_CORINFO_GENERICHANDLE_RESULT value; @@ -1691,7 +1691,7 @@ void MethodContext::recExpandRawHandleIntrinsic(CORINFO_RESOLVED_TOKEN* pResolve void MethodContext::dmpExpandRawHandleIntrinsic(const Agnostic_ExpandRawHandleIntrinsic& key, const Agnostic_CORINFO_GENERICHANDLE_RESULT& result) { printf("ExpandRawHandleIntrinsic key: %s, value %s cth-%016" PRIx64 " ht-%u", - SpmiDumpHelper::DumpAgnostic_CORINFO_RESOLVED_TOKEN(key.ResolvedToken).c_str(), + SpmiDumpHelper::DumpAgnostic_CORINFO_RESOLVED_TOKENin(key.ResolvedToken).c_str(), SpmiDumpHelper::DumpAgnostic_CORINFO_LOOKUP(result.lookup).c_str(), result.compileTimeHandle, result.handleType); @@ -1701,7 +1701,7 @@ void MethodContext::repExpandRawHandleIntrinsic(CORINFO_RESOLVED_TOKEN* pResolve Agnostic_ExpandRawHandleIntrinsic key; ZeroMemory(&key, sizeof(key)); // Zero key including any struct padding - key.ResolvedToken = SpmiRecordsHelper::RestoreAgnostic_CORINFO_RESOLVED_TOKEN(pResolvedToken, ExpandRawHandleIntrinsic); + key.ResolvedToken = SpmiRecordsHelper::CreateAgnostic_CORINFO_RESOLVED_TOKENin(pResolvedToken); key.hCallerHandle = CastHandle(callerHandle); Agnostic_CORINFO_GENERICHANDLE_RESULT value = @@ -3153,6 +3153,7 @@ void MethodContext::repEmbedGenericHandle(CORINFO_RESOLVED_TOKEN* pResolve ZeroMemory(&key, sizeof(key)); // Zero key including any struct padding key.ResolvedToken = SpmiRecordsHelper::RestoreAgnostic_CORINFO_RESOLVED_TOKEN(pResolvedToken, EmbedGenericHandle); key.fEmbedParent = (DWORD)fEmbedParent; + key.hCallerHandle = CastHandle(callerHandle); Agnostic_CORINFO_GENERICHANDLE_RESULT value = LookupByKeyOrMissNoMessage(EmbedGenericHandle, key); From 70d3cee5f622a634c7a454197be15ce4bcdf26f1 Mon Sep 17 00:00:00 2001 From: Filip Navara Date: Thu, 14 Mar 2024 14:54:09 +0100 Subject: [PATCH 2/7] Use x64->x86 MSVC cross-compiler if possible (#99748) --- src/coreclr/nativeaot/BuildIntegration/findvcvarsall.bat | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/coreclr/nativeaot/BuildIntegration/findvcvarsall.bat b/src/coreclr/nativeaot/BuildIntegration/findvcvarsall.bat index efee6316785f66..70294486de4500 100644 --- a/src/coreclr/nativeaot/BuildIntegration/findvcvarsall.bat +++ b/src/coreclr/nativeaot/BuildIntegration/findvcvarsall.bat @@ -33,6 +33,9 @@ IF /I "%~1"=="arm64" ( SET vcEnvironment=x86_arm64 IF /I "%procArch%"=="AMD64" SET vcEnvironment=amd64_arm64 ) +IF /I "%~1"=="x86" ( + IF /I "%procArch%"=="AMD64" SET vcEnvironment=amd64_x86 +) CALL "%vsBase%\vc\Auxiliary\Build\vcvarsall.bat" %vcEnvironment% > NUL From 4707b43380c9ae95ad95897678a5d51bddd72579 Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Thu, 14 Mar 2024 08:59:56 -0500 Subject: [PATCH 3/7] Improve perf\allocs of ActivatorUtilities.CreateInstance() (#99383) --- .../src/ActivatorUtilities.cs | 231 +++++++++++++----- 1 file changed, 175 insertions(+), 56 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs index 8c8fcbd5f18b45..a7d8701061e5a1 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs @@ -9,6 +9,7 @@ using System.Reflection; using System.Runtime.CompilerServices; using System.Runtime.ExceptionServices; +using System.Runtime.InteropServices; using Microsoft.Extensions.Internal; #if NETCOREAPP @@ -28,9 +29,7 @@ public static class ActivatorUtilities // Support caching of constructor metadata for types in collectible assemblies. private static readonly Lazy> s_collectibleConstructorInfos = new(); -#endif -#if NET8_0_OR_GREATER // Maximum number of fixed arguments for ConstructorInvoker.Invoke(arg1, etc). private const int FixedArgumentThreshold = 4; #endif @@ -66,10 +65,29 @@ public static object CreateInstance( { constructors = GetOrAddConstructors(instanceType); } + + // Attempt to use the stack allocated arg values if <= 4 ctor args. + Span values; + StackAllocatedObjects stackValues = default; + int maxArgs = GetMaxArgCount(); + if (maxArgs <= StackAllocatedObjects.MaxStackAllocArgCount / 2) + { + values = MemoryMarshal.CreateSpan(ref stackValues._args._arg0, maxArgs * 2); + } + else + { + values = new Span(new object?[maxArgs * 2], 0, maxArgs * 2); + } + + Span ctorArgs = values.Slice(0, maxArgs); + Span bestCtorArgs = values.Slice(maxArgs); #else constructors = CreateConstructorInfoExs(instanceType); + object?[]? ctorArgs = null; + object?[]? bestCtorArgs = null; #endif + ConstructorMatcher matcher = default; ConstructorInfoEx? constructor; IServiceProviderIsService? serviceProviderIsService = provider.GetService(); // if container supports using IServiceProviderIsService, we try to find the longest ctor that @@ -79,48 +97,70 @@ public static object CreateInstance( // instance if all parameters given to CreateInstance only match with a single ctor if (serviceProviderIsService != null) { - int bestLength = -1; - bool seenPreferred = false; - - ConstructorMatcher bestMatcher = default; - bool multipleBestLengthFound = false; - + // Handle the case where the attribute is used. for (int i = 0; i < constructors.Length; i++) { constructor = constructors[i]; - ConstructorMatcher matcher = new(constructor); - bool isPreferred = constructor.IsPreferred; - int length = matcher.Match(parameters, serviceProviderIsService); - if (isPreferred) + if (constructor.IsPreferred) { - if (seenPreferred) + for (int j = i + 1; j < constructors.Length; j++) { - ThrowMultipleCtorsMarkedWithAttributeException(); + if (constructors[j].IsPreferred) + { + ThrowMultipleCtorsMarkedWithAttributeException(); + } } - if (length == -1) + InitializeCtorArgValues(ref ctorArgs, constructor.Parameters.Length); + matcher = new ConstructorMatcher(constructor, ctorArgs); + if (matcher.Match(parameters, serviceProviderIsService) == -1) { ThrowMarkedCtorDoesNotTakeAllProvidedArguments(); } - seenPreferred = true; - bestLength = length; - bestMatcher = matcher; - multipleBestLengthFound = false; + return matcher.CreateInstance(provider); } - else if (!seenPreferred) + } + + int bestLength = -1; + ConstructorMatcher bestMatcher = default; + bool multipleBestLengthFound = false; + + // Find the constructor with the most matches. + for (int i = 0; i < constructors.Length; i++) + { + constructor = constructors[i]; + + InitializeCtorArgValues(ref ctorArgs, constructor.Parameters.Length); + matcher = new ConstructorMatcher(constructor, ctorArgs); + int length = matcher.Match(parameters, serviceProviderIsService); + + Debug.Assert(!constructor.IsPreferred); + + if (bestLength < length) { - if (bestLength < length) + bestLength = length; +#if NETCOREAPP + ctorArgs.CopyTo(bestCtorArgs); +#else + if (i == constructors.Length - 1) { - bestLength = length; - bestMatcher = matcher; - multipleBestLengthFound = false; + // We can prevent an alloc for the last case. + bestCtorArgs = ctorArgs; } - else if (bestLength == length) + else { - multipleBestLengthFound = true; + bestCtorArgs = new object?[length]; + ctorArgs.CopyTo(bestCtorArgs, 0); } +#endif + bestMatcher = new ConstructorMatcher(matcher.ConstructorInfo, bestCtorArgs); + multipleBestLengthFound = false; + } + else if (bestLength == length) + { + multipleBestLengthFound = true; } } @@ -149,24 +189,43 @@ public static object CreateInstance( } } - FindApplicableConstructor(instanceType, argumentTypes, out ConstructorInfo constructorInfo, out int?[] parameterMap); + FindApplicableConstructor(instanceType, argumentTypes, constructors, out ConstructorInfo constructorInfo, out int?[] parameterMap); + constructor = FindConstructorEx(constructorInfo, constructors); + + InitializeCtorArgValues(ref ctorArgs, constructor.Parameters.Length); + matcher = new ConstructorMatcher(constructor, ctorArgs); + matcher.MapParameters(parameterMap, parameters); + return matcher.CreateInstance(provider); - // Find the ConstructorInfoEx from the given constructorInfo. - constructor = null; - foreach (ConstructorInfoEx ctor in constructors) +#if NETCOREAPP + int GetMaxArgCount() { - if (ReferenceEquals(ctor.Info, constructorInfo)) + int max = 0; + for (int i = 0; i < constructors.Length; i++) { - constructor = ctor; - break; + max = int.Max(max, constructors[i].Parameters.Length); } - } - Debug.Assert(constructor != null); + return max; + } - var constructorMatcher = new ConstructorMatcher(constructor); - constructorMatcher.MapParameters(parameterMap, parameters); - return constructorMatcher.CreateInstance(provider); + static void InitializeCtorArgValues(ref Span ctorArgs, int _) + { + ctorArgs.Clear(); + } +#else + static void InitializeCtorArgValues(ref object[] ctorArgs, int length) + { + if (ctorArgs is not null && ctorArgs.Length == length) + { + Array.Clear(ctorArgs, 0, length); + } + else + { + ctorArgs = new object?[length]; + } + } +#endif } #if NETCOREAPP @@ -280,7 +339,7 @@ public static ObjectFactory private static void CreateFactoryInternal([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] Type instanceType, Type[] argumentTypes, out ParameterExpression provider, out ParameterExpression argumentArray, out Expression factoryExpressionBody) { - FindApplicableConstructor(instanceType, argumentTypes, out ConstructorInfo constructor, out int?[] parameterMap); + FindApplicableConstructor(instanceType, argumentTypes, constructors: null, out ConstructorInfo constructor, out int?[] parameterMap); provider = Expression.Parameter(typeof(IServiceProvider), "provider"); argumentArray = Expression.Parameter(typeof(object[]), "argumentArray"); @@ -401,10 +460,10 @@ private static ObjectFactory CreateFactoryReflection( [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] Type instanceType, Type?[] argumentTypes) { - FindApplicableConstructor(instanceType, argumentTypes, out ConstructorInfo constructor, out int?[] parameterMap); + FindApplicableConstructor(instanceType, argumentTypes, constructors: null, out ConstructorInfo constructor, out int?[] parameterMap); Type declaringType = constructor.DeclaringType!; -#if NET8_0_OR_GREATER +#if NETCOREAPP ConstructorInvoker invoker = ConstructorInvoker.Create(constructor); ParameterInfo[] constructorParameters = constructor.GetParameters(); @@ -473,7 +532,7 @@ ObjectFactory InvokeCanonical() FactoryParameterContext[] parameters = GetFactoryParameterContext(); return (serviceProvider, arguments) => ReflectionFactoryCanonical(constructor, parameters, declaringType, serviceProvider, arguments); -#endif // NET8_0_OR_GREATER +#endif // NETCOREAPP FactoryParameterContext[] GetFactoryParameterContext() { @@ -518,13 +577,14 @@ public FactoryParameterContext(Type parameterType, bool hasDefaultValue, object? private static void FindApplicableConstructor( [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] Type instanceType, Type?[] argumentTypes, + ConstructorInfoEx[]? constructors, out ConstructorInfo matchingConstructor, out int?[] matchingParameterMap) { ConstructorInfo? constructorInfo; int?[]? parameterMap; - if (!TryFindPreferredConstructor(instanceType, argumentTypes, out constructorInfo, out parameterMap) && + if (!TryFindPreferredConstructor(instanceType, argumentTypes, constructors, out constructorInfo, out parameterMap) && !TryFindMatchingConstructor(instanceType, argumentTypes, out constructorInfo, out parameterMap)) { throw new InvalidOperationException(SR.Format(SR.CtorNotLocated, instanceType)); @@ -534,6 +594,21 @@ private static void FindApplicableConstructor( matchingParameterMap = parameterMap; } + // Find the ConstructorInfoEx from the given constructorInfo. + private static ConstructorInfoEx FindConstructorEx(ConstructorInfo constructorInfo, ConstructorInfoEx[] constructorExs) + { + for (int i = 0; i < constructorExs.Length; i++) + { + if (ReferenceEquals(constructorExs[i].Info, constructorInfo)) + { + return constructorExs[i]; + } + } + + Debug.Assert(false); + return null!; + } + // Tries to find constructor based on provided argument types private static bool TryFindMatchingConstructor( [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] Type instanceType, @@ -571,6 +646,7 @@ private static bool TryFindMatchingConstructor( private static bool TryFindPreferredConstructor( [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] Type instanceType, Type?[] argumentTypes, + ConstructorInfoEx[]? constructors, [NotNullWhen(true)] out ConstructorInfo? matchingConstructor, [NotNullWhen(true)] out int?[]? parameterMap) { @@ -578,21 +654,33 @@ private static bool TryFindPreferredConstructor( matchingConstructor = null; parameterMap = null; - foreach (ConstructorInfo? constructor in instanceType.GetConstructors()) + if (constructors is null) { - if (constructor.IsDefined(typeof(ActivatorUtilitiesConstructorAttribute), false)) +#if NETCOREAPP + if (!s_constructorInfos.TryGetValue(instanceType, out constructors)) + { + constructors = GetOrAddConstructors(instanceType); + } +#else + constructors = CreateConstructorInfoExs(instanceType); +#endif + } + + foreach (ConstructorInfoEx constructor in constructors) + { + if (constructor.IsPreferred) { if (seenPreferred) { ThrowMultipleCtorsMarkedWithAttributeException(); } - if (!TryCreateParameterMap(constructor.GetParameters(), argumentTypes, out int?[] tempParameterMap)) + if (!TryCreateParameterMap(constructor.Info.GetParameters(), argumentTypes, out int?[] tempParameterMap)) { ThrowMarkedCtorDoesNotTakeAllProvidedArguments(); } - matchingConstructor = constructor; + matchingConstructor = constructor.Info; parameterMap = tempParameterMap; seenPreferred = true; } @@ -649,6 +737,17 @@ private sealed class ConstructorInfoEx public readonly ParameterInfo[] Parameters; public readonly bool IsPreferred; private readonly object?[]? _parameterKeys; +#if NETCOREAPP + public ConstructorInvoker? _invoker; + public ConstructorInvoker Invoker + { + get + { + _invoker ??= ConstructorInvoker.Create(Info); + return _invoker; + } + } +#endif public ConstructorInfoEx(ConstructorInfo constructor) { @@ -710,17 +809,24 @@ public bool IsService(IServiceProviderIsService serviceProviderIsService, int pa } } - private readonly struct ConstructorMatcher + private readonly ref struct ConstructorMatcher { private readonly ConstructorInfoEx _constructor; - private readonly object?[] _parameterValues; - public ConstructorMatcher(ConstructorInfoEx constructor) +#if NETCOREAPP + private readonly Span _parameterValues; + public ConstructorMatcher(ConstructorInfoEx constructor, Span parameterValues) +#else + private readonly object?[] _parameterValues; + public ConstructorMatcher(ConstructorInfoEx constructor, object?[] parameterValues) +#endif { _constructor = constructor; - _parameterValues = new object[constructor.Parameters.Length]; + _parameterValues = parameterValues; } + public ConstructorInfoEx ConstructorInfo => _constructor; + public int Match(object[] givenParameters, IServiceProviderIsService serviceProviderIsService) { for (int givenIndex = 0; givenIndex < givenParameters.Length; givenIndex++) @@ -790,7 +896,9 @@ public object CreateInstance(IServiceProvider provider) } } -#if NETFRAMEWORK || NETSTANDARD2_0 +#if NETCOREAPP + return _constructor.Invoker.Invoke(_parameterValues.Slice(0, _constructor.Parameters.Length)); +#else try { return _constructor.Info.Invoke(_parameterValues); @@ -801,8 +909,6 @@ public object CreateInstance(IServiceProvider provider) // The above line will always throw, but the compiler requires we throw explicitly. throw; } -#else - return _constructor.Info.Invoke(BindingFlags.DoNotWrapExceptions, binder: null, parameters: _parameterValues, culture: null); #endif } @@ -828,7 +934,7 @@ private static void ThrowMarkedCtorDoesNotTakeAllProvidedArguments() throw new InvalidOperationException(SR.Format(SR.MarkedCtorMissingArgumentTypes, nameof(ActivatorUtilitiesConstructorAttribute))); } -#if NET8_0_OR_GREATER // Use the faster ConstructorInvoker which also has alloc-free APIs when <= 4 parameters. +#if NETCOREAPP // Use the faster ConstructorInvoker which also has alloc-free APIs when <= 4 parameters. private static object ReflectionFactoryServiceOnlyFixed( ConstructorInvoker invoker, FactoryParameterContext[] parameters, @@ -1101,7 +1207,7 @@ private static object ReflectionFactoryCanonical( return constructor.Invoke(BindingFlags.DoNotWrapExceptions, binder: null, constructorArguments, culture: null); } -#endif // NET8_0_OR_GREATER +#endif #if NETCOREAPP internal static class ActivatorUtilitiesUpdateHandler @@ -1116,6 +1222,19 @@ public static void ClearCache(Type[]? _) } } } + + [StructLayout(LayoutKind.Sequential)] + private ref struct StackAllocatedObjects + { + internal const int MaxStackAllocArgCount = 8; + internal StackAllocatedObjectValues _args; + + [InlineArray(MaxStackAllocArgCount)] + internal struct StackAllocatedObjectValues + { + internal object? _arg0; + } + } #endif private static object? GetKeyedService(IServiceProvider provider, Type type, object? serviceKey) From fef9a5f45dd02e7e8a08d18dd923c30ab5dca1f8 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Thu, 14 Mar 2024 07:15:08 -0700 Subject: [PATCH 4/7] Use IntermediateAssembly instead of hard-coding the equivalent path (#99732) Fixes https://github.com/dotnet/runtime/issues/99731 --- .../nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets b/src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets index 2fa194a477c9bf..75f579fc4a8617 100644 --- a/src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets +++ b/src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets @@ -168,7 +168,7 @@ The .NET Foundation licenses this file to you under the MIT license. - + From 61332935ac1b69460fa6dbc7e269c878f7150ec1 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Thu, 14 Mar 2024 07:23:10 -0700 Subject: [PATCH 5/7] Backport VMR patch for cross builds (#99702) --- eng/DotNetBuild.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eng/DotNetBuild.props b/eng/DotNetBuild.props index a6350c7fea93fa..c5da9ec68e4517 100644 --- a/eng/DotNetBuild.props +++ b/eng/DotNetBuild.props @@ -47,7 +47,7 @@ Properties that control flags from the VMR build, and the expected output for the VMR build should be added to this file. --> $(InnerBuildArgs) $(FlagParameterPrefix)arch $(TargetArch) $(InnerBuildArgs) $(FlagParameterPrefix)os $(TargetOS) - $(InnerBuildArgs) $(FlagParameterPrefix)cross + $(InnerBuildArgs) $(FlagParameterPrefix)cross $(InnerBuildArgs) $(FlagParameterPrefix)configuration $(Configuration) $(InnerBuildArgs) $(FlagParameterPrefix)allconfigurations $(InnerBuildArgs) $(FlagParameterPrefix)verbosity $(LogVerbosity) From b54c555256a46229537ea3d78f203aacca58dac8 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Thu, 14 Mar 2024 07:26:13 -0700 Subject: [PATCH 6/7] Add option to --produce_repro in superpmi.py script (#99723) * Add option to --produce_repro in superpmi.py script * feedback * Remove changes in save_repro_mc_files --- src/coreclr/scripts/superpmi.py | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/src/coreclr/scripts/superpmi.py b/src/coreclr/scripts/superpmi.py index ef5d2c70684d49..cff7695bed0de4 100644 --- a/src/coreclr/scripts/superpmi.py +++ b/src/coreclr/scripts/superpmi.py @@ -227,6 +227,10 @@ Compile only specified method contexts, e.g., `-compile 20,25`. This is passed directly to the superpmi.exe `-compile` argument. See `superpmi.exe -?` for full documentation about allowed formats. """ +produce_repro_help = """\ +If passed, produce the *.mc repro files. +""" + # Start of parser object creation. parser = argparse.ArgumentParser(description=description) @@ -323,6 +327,7 @@ def add_core_root_arguments(parser, build_type_default, build_type_help): replay_common_parser.add_argument("-jit_ee_version", help=jit_ee_version_help) replay_common_parser.add_argument("-private_store", action="append", help=private_store_help) replay_common_parser.add_argument("-compile", "-c", help=compile_help) +replay_common_parser.add_argument("--produce_repro", action="store_true", help=produce_repro_help) # subparser for replay replay_parser = subparsers.add_parser("replay", description=replay_description, parents=[core_root_parser, target_parser, superpmi_common_parser, replay_common_parser]) @@ -1500,6 +1505,7 @@ def save_repro_mc_files(temp_location, coreclr_args, artifacts_base_name, repro_ """ For commands that use the superpmi "-r" option to create "repro" .mc files, copy these to a location where they are saved (and not in a "temp" directory) for easy use by the user. """ + # If there are any .mc files, drop them into artifacts/repro/../*.mc mc_files = [os.path.join(temp_location, item) for item in os.listdir(temp_location) if item.endswith(".mc")] if len(mc_files) > 0: @@ -1643,10 +1649,14 @@ def replay(self): repro_flags = [] common_flags = [ - "-v", "ewi", # display errors, warnings, missing, jit info - "-r", os.path.join(temp_location, "repro") # Repro name prefix, create .mc repro files + "-v", "ewi" # display errors, warnings, missing, jit info ] + if self.coreclr_args.produce_repro: + common_flags += [ + "-r", os.path.join(temp_location, "repro") # Repro name prefix, create .mc repro files + ] + if self.coreclr_args.altjit: repro_flags += [ "-jitoption", "force", "AltJit=*", @@ -2151,8 +2161,12 @@ def replay_with_asm_diffs(self): "-v", "ewi", # display errors, warnings, missing, jit info "-f", fail_mcl_file, # Failing mc List "-details", detailed_info_file, # Detailed information about each context - "-r", os.path.join(temp_location, "repro"), # Repro name prefix, create .mc repro files ] + if self.coreclr_args.produce_repro: + flags += [ + "-r", os.path.join(temp_location, "repro") # Repro name prefix, create .mc repro files + ] + flags += altjit_asm_diffs_flags flags += base_option_flags flags += diff_option_flags @@ -4482,6 +4496,11 @@ def verify_replay_common_args(): lambda unused: True, "Method context not valid") + coreclr_args.verify(args, + "produce_repro", + lambda unused: True, + "Unable to set produce_repro") + coreclr_args.verify(args, "private_store", lambda item: True, From 713e69021b6a53a4f7e7affc2eb10965e41f8f3a Mon Sep 17 00:00:00 2001 From: Aman Khalid Date: Thu, 14 Mar 2024 14:45:29 +0000 Subject: [PATCH 7/7] Remove bbFallsThrough checks in fgNewBBbefore/after (#99636) Now that implicit fallthrough is gone, it doesn't make sense to mark a new block as rarely run if the previous block is rarely run, as there's no guarantee the previous block flows into the next. It would make more sense to rely on the weights of a block's predecessor edges to determine if it's rarely run. --- src/coreclr/jit/fgbasic.cpp | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/coreclr/jit/fgbasic.cpp b/src/coreclr/jit/fgbasic.cpp index 200c72a4d4e1c8..00b0c3faf91c4a 100644 --- a/src/coreclr/jit/fgbasic.cpp +++ b/src/coreclr/jit/fgbasic.cpp @@ -6012,11 +6012,6 @@ BasicBlock* Compiler::fgNewBBbefore(BBKinds jumpKind, BasicBlock* block, bool ex newBlk->bbRefs = 0; - if (newBlk->bbFallsThrough() && block->isRunRarely()) - { - newBlk->bbSetRunRarely(); - } - if (extendRegion) { fgExtendEHRegionBefore(block); @@ -6051,11 +6046,6 @@ BasicBlock* Compiler::fgNewBBafter(BBKinds jumpKind, BasicBlock* block, bool ext newBlk->bbRefs = 0; - if (block->bbFallsThrough() && block->isRunRarely()) - { - newBlk->bbSetRunRarely(); - } - if (extendRegion) { fgExtendEHRegionAfter(block);