Add an option to disable generation of reflection invoke thunks

Touches a lot of files, but most of this is just plumbing.

In the absence of invoke thunks, reflection will use the calling convention converter. Reflection invoke will get about 7x slower, but will still work. Saves about 1.5-2% in terms of size on disk. The default is still to use the thunks, since we tweak everything towards fast defaults.

Implemented for both Project N and Project X. On the X side, I decided to go with a "policy" class instead of bools. This will let us cleanly do further optimization in the future, such as selectively generating thunks only for specific methods.

[tfs-changeset: 1710656]

9 files changed, 119 insertions, 72 deletions

diff --git a/src/ILCompiler.Compiler/src/Compiler/AnalysisBasedMetadataManager.cs b/src/ILCompiler.Compiler/src/Compiler/AnalysisBasedMetadataManager.cs
index a484a4f9b..1f2224d5d 100644
--- a/src/ILCompiler.Compiler/src/Compiler/AnalysisBasedMetadataManager.cs
+++ b/src/ILCompiler.Compiler/src/Compiler/AnalysisBasedMetadataManager.cs
@@ -30,11 +30,12 @@ namespace ILCompiler
             ManifestResourceBlockingPolicy resourceBlockingPolicy,
             string logFile,
             StackTraceEmissionPolicy stackTracePolicy,
+            DynamicInvokeThunkGenerationPolicy invokeThunkGenerationPolicy,
             IEnumerable<ModuleDesc> modulesWithMetadata,
             IEnumerable<ReflectableEntity<TypeDesc>> reflectableTypes,
             IEnumerable<ReflectableEntity<MethodDesc>> reflectableMethods,
             IEnumerable<ReflectableEntity<FieldDesc>> reflectableFields)
-            : base(typeSystemContext, blockingPolicy, resourceBlockingPolicy, logFile, stackTracePolicy)
+            : base(typeSystemContext, blockingPolicy, resourceBlockingPolicy, logFile, stackTracePolicy, invokeThunkGenerationPolicy)
         {
             _modulesWithMetadata = new List<ModuleDesc>(modulesWithMetadata);
             
diff --git a/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/UtcNodeFactory.cs b/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/UtcNodeFactory.cs
index 66716587d..5a026e34a 100644
--- a/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/UtcNodeFactory.cs
+++ b/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/UtcNodeFactory.cs
@@ -59,7 +59,7 @@ namespace ILCompiler
             return null;
         }
 
-        private static MetadataManager PickMetadataManager(CompilerTypeSystemContext context, CompilationModuleGroup compilationModuleGroup, IEnumerable<ModuleDesc> inputModules, IEnumerable<ModuleDesc> inputMetadataOnlyAssemblies, string metadataFile, bool emitStackTraceMetadata, bool disableExceptionMessages)
+        private static MetadataManager PickMetadataManager(CompilerTypeSystemContext context, CompilationModuleGroup compilationModuleGroup, IEnumerable<ModuleDesc> inputModules, IEnumerable<ModuleDesc> inputMetadataOnlyAssemblies, string metadataFile, bool emitStackTraceMetadata, bool disableExceptionMessages, bool disableInvokeThunks)
         {
             if (metadataFile == null)
             {
@@ -88,7 +88,7 @@ namespace ILCompiler
                     resourceBlockingPolicy = new NoManifestResourceBlockingPolicy();
                 }
 
-                return new PrecomputedMetadataManager(compilationModuleGroup, context, FindMetadataDescribingModuleInInputSet(inputModules), inputModules, inputMetadataOnlyAssemblies, ReadBytesFromFile(metadataFile), stackTraceEmissionPolicy , resourceBlockingPolicy);
+                return new PrecomputedMetadataManager(compilationModuleGroup, context, FindMetadataDescribingModuleInInputSet(inputModules), inputModules, inputMetadataOnlyAssemblies, ReadBytesFromFile(metadataFile), stackTraceEmissionPolicy , resourceBlockingPolicy, disableInvokeThunks);
             }
         }
 
@@ -109,11 +109,12 @@ namespace ILCompiler
             bool buildMRT, 
             bool emitStackTraceMetadata,
             bool disableExceptionMessages,
+            bool allowInvokeThunks,
             DictionaryLayoutProvider dictionaryLayoutProvider,
             ImportedNodeProvider importedNodeProvider) 
             : base(context, 
                   compilationModuleGroup, 
-                  PickMetadataManager(context, compilationModuleGroup, inputModules, inputMetadataOnlyAssemblies, metadataFile, emitStackTraceMetadata, disableExceptionMessages), 
+                  PickMetadataManager(context, compilationModuleGroup, inputModules, inputMetadataOnlyAssemblies, metadataFile, emitStackTraceMetadata, disableExceptionMessages, allowInvokeThunks),
                   NewEmptyInteropStubManager(context, compilationModuleGroup), 
                   nameMangler, 
                   new AttributeDrivenLazyGenericsPolicy(), 
diff --git a/src/ILCompiler.Compiler/src/Compiler/DynamicInvokeThunkGenerationPolicy.cs b/src/ILCompiler.Compiler/src/Compiler/DynamicInvokeThunkGenerationPolicy.cs
new file mode 100644
index 000000000..e4f3f67d7
--- /dev/null
+++ b/src/ILCompiler.Compiler/src/Compiler/DynamicInvokeThunkGenerationPolicy.cs
@@ -0,0 +1,48 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using Internal.TypeSystem;
+
+namespace ILCompiler
+{
+    /// <summary>
+    /// Controls the way calling convention conversion is performed in
+    /// <see cref="System.Reflection.MethodBase.Invoke(object, object[])"/>
+    /// scenarios.
+    /// </summary>
+    public abstract class DynamicInvokeThunkGenerationPolicy
+    {
+        /// <summary>
+        /// Gets a value indicating whether reflection-invokable method '<paramref name="targetMethod"/>'
+        /// should get a static calling convention conversion thunk. Static calling convention
+        /// conversion thunks speed up reflection invoke of the method at the cost of extra code generation.
+        /// </summary>
+        public abstract bool HasStaticInvokeThunk(MethodDesc targetMethod);
+    }
+
+    /// <summary>
+    /// A thunk generation policy that generates no static invocation thunks.
+    /// </summary>
+    public sealed class NoDynamicInvokeThunkGenerationPolicy : DynamicInvokeThunkGenerationPolicy
+    {
+        public override bool HasStaticInvokeThunk(MethodDesc targetMethod) => false;
+    }
+
+    /// <summary>
+    /// A thunk generation policy that uses static invocation thunks whenever possible.
+    /// </summary>
+    public sealed class DefaultDynamicInvokeThunkGenerationPolicy : DynamicInvokeThunkGenerationPolicy
+    {
+        public override bool HasStaticInvokeThunk(MethodDesc targetMethod)
+        {
+            // Place an upper limit on how many parameters a method can have to still get a static stub.
+            // From the past experience, methods taking 8000+ parameters get a stub that can hit various limitations
+            // in the codegen. On Project N, we were limited to 256 parameters because of MDIL limitations.
+            // We don't have such limitations here, but it's a nice round number.
+            // Reflection invoke will still work, but will go through the calling convention converter.
+
+            return targetMethod.Signature.Length <= 256;
+        }
+    }
+}
diff --git a/src/ILCompiler.Compiler/src/Compiler/EmptyMetadataManager.cs b/src/ILCompiler.Compiler/src/Compiler/EmptyMetadataManager.cs
index 5217ec935..16d547abe 100644
--- a/src/ILCompiler.Compiler/src/Compiler/EmptyMetadataManager.cs
+++ b/src/ILCompiler.Compiler/src/Compiler/EmptyMetadataManager.cs
@@ -17,7 +17,7 @@ namespace ILCompiler
         public override bool SupportsReflection => false;
 
         public EmptyMetadataManager(CompilerTypeSystemContext typeSystemContext)
-            : base(typeSystemContext, new FullyBlockedMetadataPolicy(), new FullyBlockedManifestResourcePolicy())
+            : base(typeSystemContext, new FullyBlockedMetadataPolicy(), new FullyBlockedManifestResourcePolicy(), new NoDynamicInvokeThunkGenerationPolicy())
         {
         }
 
@@ -62,15 +62,6 @@ namespace ILCompiler
         }
 
         /// <summary>
-        /// Is there a reflection invoke stub for a method that is invokable?
-        /// </summary>
-        public override bool HasReflectionInvokeStubForInvokableMethod(MethodDesc method)
-        {
-            Debug.Assert(IsReflectionInvokable(method));
-            return false;
-        }
-
-        /// <summary>
         /// Gets a stub that can be used to reflection-invoke a method with a given signature.
         /// </summary>
         public override MethodDesc GetCanonicalReflectionInvokeStub(MethodDesc method)
diff --git a/src/ILCompiler.Compiler/src/Compiler/GeneratingMetadataManager.cs b/src/ILCompiler.Compiler/src/Compiler/GeneratingMetadataManager.cs
index 03879ed0b..044842f81 100644
--- a/src/ILCompiler.Compiler/src/Compiler/GeneratingMetadataManager.cs
+++ b/src/ILCompiler.Compiler/src/Compiler/GeneratingMetadataManager.cs
@@ -14,8 +14,6 @@ using Internal.Metadata.NativeFormat.Writer;
 using ILCompiler.Metadata;
 using ILCompiler.DependencyAnalysis;
 
-using Debug = System.Diagnostics.Debug;
-
 namespace ILCompiler
 {
     /// <summary>
@@ -27,8 +25,10 @@ namespace ILCompiler
         protected readonly StackTraceEmissionPolicy _stackTraceEmissionPolicy;
         private readonly ModuleDesc _generatedAssembly;
 
-        public GeneratingMetadataManager(CompilerTypeSystemContext typeSystemContext, MetadataBlockingPolicy blockingPolicy, ManifestResourceBlockingPolicy resourceBlockingPolicy, string logFile, StackTraceEmissionPolicy stackTracePolicy)
-            : base(typeSystemContext, blockingPolicy, resourceBlockingPolicy)
+        public GeneratingMetadataManager(CompilerTypeSystemContext typeSystemContext, MetadataBlockingPolicy blockingPolicy,
+            ManifestResourceBlockingPolicy resourceBlockingPolicy, string logFile, StackTraceEmissionPolicy stackTracePolicy,
+            DynamicInvokeThunkGenerationPolicy invokeThunkGenerationPolicy)
+            : base(typeSystemContext, blockingPolicy, resourceBlockingPolicy, invokeThunkGenerationPolicy)
         {
             _metadataLogFile = logFile;
             _stackTraceEmissionPolicy = stackTracePolicy;
@@ -169,22 +169,6 @@ namespace ILCompiler
         protected abstract IEnumerable<FieldDesc> GetFieldsWithRuntimeMapping();
 
         /// <summary>
-        /// Is there a reflection invoke stub for a method that is invokable?
-        /// </summary>
-        public sealed override bool HasReflectionInvokeStubForInvokableMethod(MethodDesc method)
-        {
-            Debug.Assert(IsReflectionInvokable(method));
-
-            // Place an upper limit on how many parameters a method can have to still get a static stub.
-            // From the past experience, methods taking 8000+ parameters get a stub that can hit various limitations
-            // in the codegen. On Project N, we were limited to 256 parameters because of MDIL limitations.
-            // We don't have such limitations here, but it's a nice round number.
-            // Reflection invoke will still work, but will go through the calling convention converter.
-
-            return method.Signature.Length <= 256;
-        }
-
-        /// <summary>
         /// Gets a stub that can be used to reflection-invoke a method with a given signature.
         /// </summary>
         public sealed override MethodDesc GetCanonicalReflectionInvokeStub(MethodDesc method)
diff --git a/src/ILCompiler.Compiler/src/Compiler/MetadataManager.cs b/src/ILCompiler.Compiler/src/Compiler/MetadataManager.cs
index de1994958..31526a097 100644
--- a/src/ILCompiler.Compiler/src/Compiler/MetadataManager.cs
+++ b/src/ILCompiler.Compiler/src/Compiler/MetadataManager.cs
@@ -43,6 +43,7 @@ namespace ILCompiler
         protected readonly CompilerTypeSystemContext _typeSystemContext;
         protected readonly MetadataBlockingPolicy _blockingPolicy;
         protected readonly ManifestResourceBlockingPolicy _resourceBlockingPolicy;
+        protected readonly DynamicInvokeThunkGenerationPolicy _dynamicInvokeThunkGenerationPolicy;
 
         private List<NonGCStaticsNode> _cctorContextsGenerated = new List<NonGCStaticsNode>();
         private HashSet<TypeDesc> _typesWithEETypesGenerated = new HashSet<TypeDesc>();
@@ -56,11 +57,13 @@ namespace ILCompiler
         internal DynamicInvokeTemplateDataNode DynamicInvokeTemplateData { get; private set; }
         public virtual bool SupportsReflection => true;
 
-        public MetadataManager(CompilerTypeSystemContext typeSystemContext, MetadataBlockingPolicy blockingPolicy, ManifestResourceBlockingPolicy resourceBlockingPolicy)
+        public MetadataManager(CompilerTypeSystemContext typeSystemContext, MetadataBlockingPolicy blockingPolicy,
+            ManifestResourceBlockingPolicy resourceBlockingPolicy, DynamicInvokeThunkGenerationPolicy dynamicInvokeThunkGenerationPolicy)
         {
             _typeSystemContext = typeSystemContext;
             _blockingPolicy = blockingPolicy;
             _resourceBlockingPolicy = resourceBlockingPolicy;
+            _dynamicInvokeThunkGenerationPolicy = dynamicInvokeThunkGenerationPolicy;
         }
 
         public void AttachToDependencyGraph(DependencyAnalyzerBase<NodeFactory> graph)
@@ -375,7 +378,11 @@ namespace ILCompiler
         /// <summary>
         /// Given that a method is invokable, does there exist a reflection invoke stub?
         /// </summary>
-        public abstract bool HasReflectionInvokeStubForInvokableMethod(MethodDesc method);
+        public bool HasReflectionInvokeStubForInvokableMethod(MethodDesc method)
+        {
+            Debug.Assert(IsReflectionInvokable(method));
+            return _dynamicInvokeThunkGenerationPolicy.HasStaticInvokeThunk(method);
+        }
 
         /// <summary>
         /// Given that a method is invokable, if it is inserted into the reflection invoke table
diff --git a/src/ILCompiler.Compiler/src/Compiler/PrecomputedMetadataManager.cs b/src/ILCompiler.Compiler/src/Compiler/PrecomputedMetadataManager.cs
index 33dcd22cb..c51a88809 100644
--- a/src/ILCompiler.Compiler/src/Compiler/PrecomputedMetadataManager.cs
+++ b/src/ILCompiler.Compiler/src/Compiler/PrecomputedMetadataManager.cs
@@ -64,9 +64,14 @@ namespace ILCompiler
             IEnumerable<ModuleDesc> inputMetadataOnlyAssemblies,
             byte[] metadataBlob,
             StackTraceEmissionPolicy stackTraceEmissionPolicy,
-            ManifestResourceBlockingPolicy resourceBlockingPolicy)
-            : base(typeSystemContext, new AttributeSpecifiedBlockingPolicy(), resourceBlockingPolicy)
+            ManifestResourceBlockingPolicy resourceBlockingPolicy,
+            bool disableInvokeThunks)
+            : base(typeSystemContext, new AttributeSpecifiedBlockingPolicy(), resourceBlockingPolicy,
+                  disableInvokeThunks ? (DynamicInvokeThunkGenerationPolicy)new NoDynamicInvokeThunkGenerationPolicy() : new PrecomputedDynamicInvokeThunkGenerationPolicy())
         {
+            // Need to do this dance because C# won't let us access `this` in the `base()` expression above. Sigh.
+            (_dynamicInvokeThunkGenerationPolicy as PrecomputedDynamicInvokeThunkGenerationPolicy)?.SetParentWorkaround(this);
+
             _compilationModuleGroup = group;
             _metadataDescribingModule = metadataDescribingModule;
             _compilationModules = new HashSet<ModuleDesc>(compilationModules);
@@ -904,38 +909,6 @@ namespace ILCompiler
         }
 
         /// <summary>
-        /// Is there a reflection invoke stub for a method that is invokable?
-        /// </summary>
-        public override bool HasReflectionInvokeStubForInvokableMethod(MethodDesc method)
-        {
-            Debug.Assert(IsReflectionInvokable(method));
-
-            if (!ProjectNDependencyBehavior.EnableFullAnalysis)
-            {
-                if (method.IsCanonicalMethod(CanonicalFormKind.Any))
-                    return false;
-            }
-            else
-            {
-                if (method.IsCanonicalMethod(CanonicalFormKind.Universal))
-                    return false;
-            }
-
-            MethodDesc reflectionInvokeStub = GetCanonicalReflectionInvokeStub(method);
-
-            if (reflectionInvokeStub == null)
-                return false;
-
-            // TODO: Generate DynamicInvokeTemplateMap dependencies correctly. For now, force all canonical stubs to go through the 
-            // calling convention converter interpreter path.
-            if (reflectionInvokeStub.IsSharedByGenericInstantiations)
-                return false;
-
-            return true;
-        }
-
-
-        /// <summary>
         /// Gets a stub that can be used to reflection-invoke a method with a given signature.
         /// </summary>
         public override MethodDesc GetCanonicalReflectionInvokeStub(MethodDesc method)
@@ -1050,5 +1023,45 @@ namespace ILCompiler
             public bool IsBlocked(MethodDesc methodDef) => false;
             public ModuleDesc GetModuleOfType(MetadataType typeDef) => typeDef.Module;
         }
+
+        private sealed class PrecomputedDynamicInvokeThunkGenerationPolicy : DynamicInvokeThunkGenerationPolicy
+        {
+            private PrecomputedMetadataManager _parent;
+
+            public PrecomputedDynamicInvokeThunkGenerationPolicy()
+            {
+            }
+
+            public void SetParentWorkaround(PrecomputedMetadataManager parent)
+            {
+                _parent = parent;
+            }
+
+            public override bool HasStaticInvokeThunk(MethodDesc method)
+            {
+                if (!ProjectNDependencyBehavior.EnableFullAnalysis)
+                {
+                    if (method.IsCanonicalMethod(CanonicalFormKind.Any))
+                        return false;
+                }
+                else
+                {
+                    if (method.IsCanonicalMethod(CanonicalFormKind.Universal))
+                        return false;
+                }
+
+                MethodDesc reflectionInvokeStub = _parent.GetCanonicalReflectionInvokeStub(method);
+
+                if (reflectionInvokeStub == null)
+                    return false;
+
+                // TODO: Generate DynamicInvokeTemplateMap dependencies correctly. For now, force all canonical stubs to go through the 
+                // calling convention converter interpreter path.
+                if (reflectionInvokeStub.IsSharedByGenericInstantiations)
+                    return false;
+
+                return true;
+            }
+        }
     }
 }
diff --git a/src/ILCompiler.Compiler/src/Compiler/UsageBasedMetadataManager.cs b/src/ILCompiler.Compiler/src/Compiler/UsageBasedMetadataManager.cs
index c6a71b2b4..312a1a192 100644
--- a/src/ILCompiler.Compiler/src/Compiler/UsageBasedMetadataManager.cs
+++ b/src/ILCompiler.Compiler/src/Compiler/UsageBasedMetadataManager.cs
@@ -39,8 +39,9 @@ namespace ILCompiler
             ManifestResourceBlockingPolicy resourceBlockingPolicy,
             string logFile,
             StackTraceEmissionPolicy stackTracePolicy,
+            DynamicInvokeThunkGenerationPolicy invokeThunkGenerationPolicy,
             UsageBasedMetadataGenerationOptions generationOptions)
-            : base(typeSystemContext, blockingPolicy, resourceBlockingPolicy, logFile, stackTracePolicy)
+            : base(typeSystemContext, blockingPolicy, resourceBlockingPolicy, logFile, stackTracePolicy, invokeThunkGenerationPolicy)
         {
             // We use this to mark places that would behave differently if we tracked exact fields used. 
             _hasPreciseFieldUsageInformation = false;
@@ -359,7 +360,7 @@ namespace ILCompiler
             }
 
             return new AnalysisBasedMetadataManager(
-                _typeSystemContext, _blockingPolicy, _resourceBlockingPolicy, _metadataLogFile, _stackTraceEmissionPolicy,
+                _typeSystemContext, _blockingPolicy, _resourceBlockingPolicy, _metadataLogFile, _stackTraceEmissionPolicy, _dynamicInvokeThunkGenerationPolicy,
                 _modulesWithMetadata, reflectableTypes.ToEnumerable(), reflectableMethods.ToEnumerable(),
                 reflectableFields.ToEnumerable());
         }
diff --git a/src/ILCompiler.Compiler/src/ILCompiler.Compiler.csproj b/src/ILCompiler.Compiler/src/ILCompiler.Compiler.csproj
index b738314bc..be2d84a43 100644
--- a/src/ILCompiler.Compiler/src/ILCompiler.Compiler.csproj
+++ b/src/ILCompiler.Compiler/src/ILCompiler.Compiler.csproj
@@ -130,6 +130,7 @@
     <Compile Include="Compiler\DependencyAnalysis\LoopHijackFlagNode.cs" />
     <Compile Include="Compiler\DevirtualizationManager.cs" />
     <Compile Include="Compiler\DictionaryLayoutProvider.cs" />
+    <Compile Include="Compiler\DynamicInvokeThunkGenerationPolicy.cs" />
     <Compile Include="Compiler\EmptyInteropStubManager.cs" />
     <Compile Include="Compiler\DependencyAnalysis\ISortableNode.cs" />
     <Compile Include="Compiler\DependencyAnalysis\SortableDependencyNode.cs" />