From 1bb85b989d9b01563df9eb83dce1c6a3415ce182 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Rylek?= Date: Tue, 6 Nov 2018 18:34:12 +0100 Subject: [PATCH] Initial support for running tests against CPAOT-built framework (#6530) * Initial support for running tests against CPAOT-built framework The pass rate is not super high right now, I'm seeing 109 failing tests or about 13% pass rate but the main point is that the pass rate is not zero, we can build on that. In the AVAILABLE_TYPES R2R node, I added provisions for emitting exported types. This was one of the issues we were hitting - the partial facade System.Runtime wasn't properly exporting its type forwards. In the PE builder, I dropped copying of all directories per JanK's suggestion, the latest impulse being a mismatch between the debug directory and the CPAOT-compiled executable. * Modify available types to be emitted based on metadata I have applied JanK's suggestion to change AVAILABLE_TYPES to be generated based on metadata rather than based on the results of dependency analysis. * Addressed Michal's PR feedback 1) Unified DefinedTypeInfo and ExportedTypeInfo to use a common generic class TypeInfo per Michal's suggestion. 2) Fixed exported type traversal to cater for nested forwards. 3) Added a tiny fix I noticed by comparing CPAOT and Crossgen code - querying method dictionary in a generic lookup should use the "METHOD_HANDLE", not "METHOD_DICTIONARY" fixup. This increases the pass rate of Top200 against CPAOT-built framework to about 82% (22 failures out of 126 tests). Thanks Tomas --- .../ReadyToRun/TypesTableNode.cs | 49 ++++++++++++------- .../ReadyToRunSymbolNodeFactory.cs | 2 +- .../src/Compiler/ReadyToRunHashCode.cs | 2 +- .../src/Compiler/ReadyToRunTableManager.cs | 42 +++++++++++----- .../src/ObjectWriter/R2RPEBuilder.cs | 17 ------- tests/CoreCLR/compile-framework.cmd | 8 ++- tests/runtest.cmd | 5 ++ 7 files changed, 76 insertions(+), 49 deletions(-) diff --git a/src/ILCompiler.ReadyToRun/src/Compiler/DependencyAnalysis/ReadyToRun/TypesTableNode.cs b/src/ILCompiler.ReadyToRun/src/Compiler/DependencyAnalysis/ReadyToRun/TypesTableNode.cs index 06ad2d1eb..26a204e83 100644 --- a/src/ILCompiler.ReadyToRun/src/Compiler/DependencyAnalysis/ReadyToRun/TypesTableNode.cs +++ b/src/ILCompiler.ReadyToRun/src/Compiler/DependencyAnalysis/ReadyToRun/TypesTableNode.cs @@ -6,6 +6,7 @@ using System; using System.Collections.Generic; using System.Diagnostics; using System.IO; +using System.Reflection.Metadata; using System.Reflection.Metadata.Ecma335; using Internal.NativeFormat; @@ -38,31 +39,45 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun VertexHashtable typesHashtable = new VertexHashtable(); section.Place(typesHashtable); - HashSet uniqueTypes = new HashSet(); + ReadyToRunTableManager r2rManager = (ReadyToRunTableManager)factory.MetadataManager; - foreach (TypeDesc type in ((ReadyToRunTableManager)factory.MetadataManager).GetTypesWithAvailableTypes()) + foreach (TypeInfo defTypeInfo in r2rManager.GetDefinedTypes()) { - int rid = 0; - if (type.GetTypeDefinition() is EcmaType ecmaType) + TypeDefinitionHandle defTypeHandle = defTypeInfo.Handle; + int hashCode = 0; + for (; ; ) { - if (uniqueTypes.Add(ecmaType)) + TypeDefinition defType = defTypeInfo.MetadataReader.GetTypeDefinition(defTypeHandle); + string namespaceName = defTypeInfo.MetadataReader.GetString(defType.Namespace); + string typeName = defTypeInfo.MetadataReader.GetString(defType.Name); + hashCode ^= ReadyToRunHashCode.NameHashCode(namespaceName, typeName); + if (!defType.Attributes.IsNested()) { - rid = MetadataTokens.GetToken(ecmaType.Handle) & 0x00FFFFFF; - Debug.Assert(rid != 0); - - int hashCode = ReadyToRunHashCode.TypeTableHashCode(ecmaType); - typesHashtable.Append(unchecked((uint)hashCode), section.Place(new UnsignedConstant((uint)rid << 1))); + break; } + defTypeHandle = defType.GetDeclaringType(); } - else if (type.IsArray || type.IsMdArray) + typesHashtable.Append(unchecked((uint)hashCode), section.Place(new UnsignedConstant(((uint)MetadataTokens.GetRowNumber(defTypeInfo.Handle) << 1) | 0))); + } + + foreach (TypeInfo expTypeInfo in r2rManager.GetExportedTypes()) + { + ExportedTypeHandle expTypeHandle = expTypeInfo.Handle; + int hashCode = 0; + for (; ;) { - // TODO: arrays in type table - should we have a recursive descent into composite types here - // and e.g. add the element type to the type table in case of arrays? - } - else - { - throw new NotImplementedException(); + ExportedType expType = expTypeInfo.MetadataReader.GetExportedType(expTypeHandle); + string namespaceName = expTypeInfo.MetadataReader.GetString(expType.Namespace); + string typeName = expTypeInfo.MetadataReader.GetString(expType.Name); + hashCode ^= ReadyToRunHashCode.NameHashCode(namespaceName, typeName); + if (expType.Implementation.Kind != HandleKind.ExportedType) + { + // Not a nested class + break; + } + expTypeHandle = (ExportedTypeHandle)expType.Implementation; } + typesHashtable.Append(unchecked((uint)hashCode), section.Place(new UnsignedConstant(((uint)MetadataTokens.GetRowNumber(expTypeInfo.Handle) << 1) | 1))); } MemoryStream writerContent = new MemoryStream(); diff --git a/src/ILCompiler.ReadyToRun/src/Compiler/DependencyAnalysis/ReadyToRunSymbolNodeFactory.cs b/src/ILCompiler.ReadyToRun/src/Compiler/DependencyAnalysis/ReadyToRunSymbolNodeFactory.cs index f991de4be..1c636ded7 100644 --- a/src/ILCompiler.ReadyToRun/src/Compiler/DependencyAnalysis/ReadyToRunSymbolNodeFactory.cs +++ b/src/ILCompiler.ReadyToRun/src/Compiler/DependencyAnalysis/ReadyToRunSymbolNodeFactory.cs @@ -739,7 +739,7 @@ namespace ILCompiler.DependencyAnalysis case ReadyToRunHelperId.MethodDictionary: return GenericLookupMethodHelper( runtimeLookupKind, - ReadyToRunFixupKind.READYTORUN_FIXUP_MethodDictionary, + ReadyToRunFixupKind.READYTORUN_FIXUP_MethodHandle, (MethodWithToken)helperArgument, contextType, signatureContext); diff --git a/src/ILCompiler.ReadyToRun/src/Compiler/ReadyToRunHashCode.cs b/src/ILCompiler.ReadyToRun/src/Compiler/ReadyToRunHashCode.cs index 74a9acad5..b6d7e2341 100644 --- a/src/ILCompiler.ReadyToRun/src/Compiler/ReadyToRunHashCode.cs +++ b/src/ILCompiler.ReadyToRun/src/Compiler/ReadyToRunHashCode.cs @@ -60,7 +60,7 @@ namespace ILCompiler /// /// Namespace name /// Type name within the namespace - static int NameHashCode(string namespacePart, string namePart) + public static int NameHashCode(string namespacePart, string namePart) { return NameHashCode(namespacePart) ^ NameHashCode(namePart); } diff --git a/src/ILCompiler.ReadyToRun/src/Compiler/ReadyToRunTableManager.cs b/src/ILCompiler.ReadyToRun/src/Compiler/ReadyToRunTableManager.cs index b75e5816e..a35031ef8 100644 --- a/src/ILCompiler.ReadyToRun/src/Compiler/ReadyToRunTableManager.cs +++ b/src/ILCompiler.ReadyToRun/src/Compiler/ReadyToRunTableManager.cs @@ -4,21 +4,34 @@ using System; using System.Collections.Generic; +using System.Reflection.Metadata; +using System.Reflection.Metadata.Ecma335; using ILCompiler.DependencyAnalysis; using ILCompiler.DependencyAnalysis.ReadyToRun; using ILCompiler.DependencyAnalysisFramework; using Internal.TypeSystem; +using Internal.TypeSystem.Ecma; using Debug = System.Diagnostics.Debug; namespace ILCompiler { + public struct TypeInfo + { + public readonly MetadataReader MetadataReader; + public readonly THandle Handle; + + public TypeInfo(MetadataReader metadataReader, THandle handle) + { + MetadataReader = metadataReader; + Handle = handle; + } + } + public class ReadyToRunTableManager : MetadataManager { - private readonly HashSet _typesWithAvailableTypesGenerated = new HashSet(); - public ReadyToRunTableManager(CompilerTypeSystemContext typeSystemContext) : base(typeSystemContext, new NoMetadataBlockingPolicy(), new NoManifestResourceBlockingPolicy(), new NoDynamicInvokeThunkGenerationPolicy()) {} @@ -27,21 +40,28 @@ namespace ILCompiler // We don't attach any metadata blobs. } - protected override void Graph_NewMarkedNode(DependencyNodeCore obj) + public IEnumerable> GetDefinedTypes() { - base.Graph_NewMarkedNode(obj); - - var eetypeNode = obj as AvailableType; - if (eetypeNode != null) + foreach (string inputFile in _typeSystemContext.InputFilePaths.Values) { - _typesWithAvailableTypesGenerated.Add(eetypeNode.Type); - return; + EcmaModule module = _typeSystemContext.GetModuleFromPath(inputFile); + foreach (TypeDefinitionHandle typeDefHandle in module.MetadataReader.TypeDefinitions) + { + yield return new TypeInfo(module.MetadataReader, typeDefHandle); + } } } - public IEnumerable GetTypesWithAvailableTypes() + public IEnumerable> GetExportedTypes() { - return _typesWithAvailableTypesGenerated; + foreach (string inputFile in _typeSystemContext.InputFilePaths.Values) + { + EcmaModule module = _typeSystemContext.GetModuleFromPath(inputFile); + foreach (ExportedTypeHandle exportedTypeHandle in module.MetadataReader.ExportedTypes) + { + yield return new TypeInfo(module.MetadataReader, exportedTypeHandle); + } + } } public override MethodDesc GetCanonicalReflectionInvokeStub(MethodDesc method) => throw new NotImplementedException(); diff --git a/src/ILCompiler.ReadyToRun/src/ObjectWriter/R2RPEBuilder.cs b/src/ILCompiler.ReadyToRun/src/ObjectWriter/R2RPEBuilder.cs index b47c39a0f..28b1d2879 100644 --- a/src/ILCompiler.ReadyToRun/src/ObjectWriter/R2RPEBuilder.cs +++ b/src/ILCompiler.ReadyToRun/src/ObjectWriter/R2RPEBuilder.cs @@ -235,23 +235,6 @@ namespace ILCompiler.PEWriter protected override PEDirectoriesBuilder GetDirectories() { PEDirectoriesBuilder builder = new PEDirectoriesBuilder(); - - // Don't copy over EntryPoint - // Don't copy over Export directory - // Don't copy over Import directory - builder.ResourceTable = RelocateDirectoryEntry(_peReader.PEHeaders.PEHeader.ResourceTableDirectory); - builder.ExceptionTable = RelocateDirectoryEntry(_peReader.PEHeaders.PEHeader.ExceptionTableDirectory); - // TODO - missing in PEDirectoriesBuilder - // builder.CertificateTable = RelocateDirectoryEntry(peHeaders.PEHeader.CertificateTableDirectory); - builder.BaseRelocationTable = RelocateDirectoryEntry(_peReader.PEHeaders.PEHeader.BaseRelocationTableDirectory); - builder.DebugTable = RelocateDirectoryEntry(_peReader.PEHeaders.PEHeader.DebugTableDirectory); - builder.CopyrightTable = RelocateDirectoryEntry(_peReader.PEHeaders.PEHeader.CopyrightTableDirectory); - builder.GlobalPointerTable = RelocateDirectoryEntry(_peReader.PEHeaders.PEHeader.GlobalPointerTableDirectory); - builder.ThreadLocalStorageTable = RelocateDirectoryEntry(_peReader.PEHeaders.PEHeader.ThreadLocalStorageTableDirectory); - builder.LoadConfigTable = RelocateDirectoryEntry(_peReader.PEHeaders.PEHeader.LoadConfigTableDirectory); - builder.BoundImportTable = RelocateDirectoryEntry(_peReader.PEHeaders.PEHeader.BoundImportTableDirectory); - // Don't copy over import address table - // Don't copy over delay import table builder.CorHeaderTable = RelocateDirectoryEntry(_peReader.PEHeaders.PEHeader.CorHeaderTableDirectory); if (_directoriesUpdater != null) diff --git a/tests/CoreCLR/compile-framework.cmd b/tests/CoreCLR/compile-framework.cmd index 780a31dda..3fe550a2c 100644 --- a/tests/CoreCLR/compile-framework.cmd +++ b/tests/CoreCLR/compile-framework.cmd @@ -7,13 +7,17 @@ @if not defined _echo @echo off setlocal EnableDelayedExpansion -rd /s /q %CoreRT_CoreCLRRuntimeDir%\native - if "%CoreRT_CoreCLRRuntimeDir%" == "" ( echo set CoreRT_CoreCLRRuntimeDir to CoreCLR folder or run from runtest.cmd exit /b 1 ) +rd /s /q %CoreRT_CoreCLRRuntimeDir%\native + +mkdir %CoreRT_CoreCLRRuntimeDir%\native +xcopy /Q /Y %CoreRT_CoreCLRRuntimeDir%\*.exe %CoreRT_CoreCLRRuntimeDir%\native\ +xcopy /Q /Y %CoreRT_CoreCLRRuntimeDir%\*.dll %CoreRT_CoreCLRRuntimeDir%\native\ + for %%x in (%CoreRT_CoreCLRRuntimeDir%\Microsoft.*.dll %CoreRT_CoreCLRRuntimeDir%\System.*.dll) do ( call :CompileAssembly %%x ) diff --git a/tests/runtest.cmd b/tests/runtest.cmd index 555ca6588..ad5b8e6d9 100644 --- a/tests/runtest.cmd +++ b/tests/runtest.cmd @@ -60,6 +60,7 @@ if /i "%1" == "/multimodule" (set CoreRT_MultiFileConfiguration=MultiModule&shif if /i "%1" == "/determinism" (set CoreRT_DeterminismMode=true&shift&goto ArgLoop) if /i "%1" == "/nocleanup" (set CoreRT_NoCleanup=true&shift&goto ArgLoop) if /i "%1" == "/r2rframework" (set CoreRT_R2RFramework=true&shift&goto ArgLoop) +if /i "%1" == "/user2rframework" (set CoreRT_UseR2RFramework=true&shift&goto ArgLoop) echo Invalid command line argument: %1 goto :Usage @@ -105,6 +106,10 @@ if "%CoreRT_MultiFileConfiguration%"=="MultiModule" ( set CoreRT_CoreCLRRuntimeDir=%CoreRT_TestRoot%..\bin\obj\%CoreRT_BuildOS%.%CoreRT_BuildArch%.%CoreRT_BuildType%\CoreClrRuntime set CoreRT_ReadyToRunTestHarness=%CoreRT_TestRoot%src\tools\ReadyToRun.TestHarness\bin\Debug\netcoreapp2.1\ReadyToRun.TestHarness.dll +rem When using pre-built R2R framework, switch the CoreCLRRuntime folder to its "native" subfolder +rem where we copy over the CoreCLRRuntime folder and emit the R2R versions of the framework assemblies. +if /i "%CoreRT_UseR2RFramework%" == "true" (set CoreRT_CoreCLRRuntimeDir=!CoreRT_CoreCLRRuntimeDir!\native) + if not exist %CoreRT_CoreCLRRuntimeDir% goto :BuildTests if not exist %CoreRT_ReadyToRunTestHarness% goto :BuildTests goto :SkipBuildTests