diff --git a/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs b/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs index 17ffb5aee0722..b69c4f1cef38b 100644 --- a/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs +++ b/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs @@ -33,7 +33,6 @@ using System.Diagnostics.CodeAnalysis; using System.Linq; using System.Reflection.Runtime.TypeParsing; -using System.Runtime.CompilerServices; using System.Text.RegularExpressions; using Mono.Cecil; using Mono.Cecil.Cil; @@ -57,6 +56,7 @@ public partial class MarkStep : IStep protected List _unreachableBodies; readonly List<(TypeDefinition Type, MethodBody Body, Instruction Instr)> _pending_isinst_instr; + UnreachableBlocksOptimizer _unreachableBlocksOptimizer; #if DEBUG static readonly DependencyKind[] _entireTypeReasons = new DependencyKind[] { @@ -182,6 +182,7 @@ public MarkStep () public virtual void Process (LinkContext context) { _context = context; + _unreachableBlocksOptimizer = new UnreachableBlocksOptimizer (_context); Initialize (); Process (); @@ -2529,6 +2530,8 @@ protected virtual void ProcessMethod (MethodDefinition method, in DependencyInfo if (CheckProcessed (method)) return; + _unreachableBlocksOptimizer.ProcessMethod (method); + if (!markedForCall) MarkType (method.DeclaringType, new DependencyInfo (DependencyKind.DeclaringType, method), method); MarkCustomAttributes (method, new DependencyInfo (DependencyKind.CustomAttribute, method), method); diff --git a/src/tools/illink/src/linker/Linker.Steps/RemoveUnreachableBlocksStep.cs b/src/tools/illink/src/linker/Linker.Steps/UnreachableBlocksOptimizer.cs similarity index 91% rename from src/tools/illink/src/linker/Linker.Steps/RemoveUnreachableBlocksStep.cs rename to src/tools/illink/src/linker/Linker.Steps/UnreachableBlocksOptimizer.cs index 5ccb40e440934..47839d664f70d 100644 --- a/src/tools/illink/src/linker/Linker.Steps/RemoveUnreachableBlocksStep.cs +++ b/src/tools/illink/src/linker/Linker.Steps/UnreachableBlocksOptimizer.cs @@ -10,12 +10,13 @@ namespace Mono.Linker.Steps { // - // This steps evaluates simple properties or methods for constant expressions and + // Evaluates simple properties or methods for constant expressions and // then uses this information to remove unreachable conditional blocks. It does // not do any inlining-like code changes. // - public class RemoveUnreachableBlocksStep : BaseStep + public class UnreachableBlocksOptimizer { + readonly LinkContext _context; MethodDefinition IntPtrSize, UIntPtrSize; readonly struct ProcessingNode @@ -39,7 +40,7 @@ public ProcessingNode (in ProcessingNode other, int newLastAttempStackVersion) // Stack of method nodes which are currently being processed. // Implemented as linked list to allow easy referal to nodes and efficient moving of nodes within the list. // The top of the stack is the first item in the list. - LinkedList processingStack; + readonly LinkedList _processingStack; // Each time an item is added or removed from the processing stack this value is incremented. // Moving items in the stack doesn't increment. @@ -50,11 +51,11 @@ public ProcessingNode (in ProcessingNode other, int newLastAttempStackVersion) // if we get around to process the method again and the version of the stack didn't change, then there's a loop // (nothing changed in the stack - order is unimportant, as such no new information has been added and so // we can't resolve the situation with just the info at hand). - int processingStackVersion; + int _processingStackVersion; // Just a fast lookup from method to the node on the stack. This is needed to be able to quickly // access the node and move it to the top of the stack. - Dictionary> processingMethods; + readonly Dictionary> _processingMethods; // Stores results of method processing. This state is kept forever to avoid reprocessing of methods. // If method is not in the dictionary it has not yet been processed. @@ -64,54 +65,17 @@ public ProcessingNode (in ProcessingNode other, int newLastAttempStackVersion) // - Instruction instance - method has been processed and it has a constant return value (the value of the instruction) // Note: ProcessedUnchangedSentinel is used as an optimization. running constant value analysis on a method is relatively expensive // and so we delay it and only do it for methods where the value is asked for (or in case of changed methods upfront due to implementation detailds) - Dictionary processedMethods; + readonly Dictionary _processedMethods; static readonly Instruction ProcessedUnchangedSentinel = Instruction.Create (OpCodes.Ldstr, "ProcessedUnchangedSentinel"); static readonly Instruction NonConstSentinel = Instruction.Create (OpCodes.Ldstr, "NonConstSentinel"); - protected override void Process () + public UnreachableBlocksOptimizer (LinkContext context) { - var assemblies = Context.Annotations.GetAssemblies ().ToArray (); + _context = context; - processingStack = new LinkedList (); - processingMethods = new Dictionary> (); - processedMethods = new Dictionary (); - - foreach (var assembly in assemblies) { - if (Annotations.GetAction (assembly) != AssemblyAction.Link) - continue; - - ProcessMethods (assembly.MainModule.Types); - } - } - - void ProcessMethods (Collection types) - { - foreach (var type in types) { - if (type.IsInterface) - continue; - - if (!type.HasMethods) - continue; - - foreach (var method in type.Methods) { - if (!method.HasBody) - continue; - - // - // Block methods which rewrite does not support - // - switch (method.ReturnType.MetadataType) { - case MetadataType.ByReference: - case MetadataType.FunctionPointer: - continue; - } - - ProcessMethod (method); - } - - if (type.HasNestedTypes) - ProcessMethods (type.NestedTypes); - } + _processingStack = new LinkedList (); + _processingMethods = new Dictionary> (); + _processedMethods = new Dictionary (); } /// @@ -120,52 +84,75 @@ void ProcessMethods (Collection types) /// It may optimize other methods as well - those are remembered for future reuse. /// /// The method to process - void ProcessMethod (MethodDefinition method) + public void ProcessMethod (MethodDefinition method) { - Debug.Assert (processingStack.Count == 0 && processingMethods.Count == 0); - processingStackVersion = 0; + if (!IsMethodSupported (method)) + return; + + if (_context.Annotations.GetAction (method.Module.Assembly) != AssemblyAction.Link) + return; + + Debug.Assert (_processingStack.Count == 0 && _processingMethods.Count == 0); + _processingStackVersion = 0; - if (!processedMethods.ContainsKey (method)) { + if (!_processedMethods.ContainsKey (method)) { AddMethodForProcessing (method); ProcessStack (); } - Debug.Assert (processedMethods.ContainsKey (method)); + Debug.Assert (_processedMethods.ContainsKey (method)); + } + + static bool IsMethodSupported (MethodDefinition method) + { + if (!method.HasBody) + return false; + + // + // Block methods which rewrite does not support + // + switch (method.ReturnType.MetadataType) { + case MetadataType.ByReference: + case MetadataType.FunctionPointer: + return false; + } + + return true; } void AddMethodForProcessing (MethodDefinition method) { - Debug.Assert (!processedMethods.ContainsKey (method)); + Debug.Assert (!_processedMethods.ContainsKey (method)); var processingNode = new ProcessingNode (method, -1); - var stackNode = processingStack.AddFirst (processingNode); - processingMethods.Add (method, stackNode); - processingStackVersion++; + var stackNode = _processingStack.AddFirst (processingNode); + _processingMethods.Add (method, stackNode); + _processingStackVersion++; } void StoreMethodAsProcessedAndRemoveFromQueue (LinkedListNode stackNode, Instruction methodValue) { - Debug.Assert (stackNode.List == processingStack); + Debug.Assert (stackNode.List == _processingStack); Debug.Assert (methodValue != null); var method = stackNode.Value.Method; - processingMethods.Remove (method); - processingStack.Remove (stackNode); - processingStackVersion++; + _processingMethods.Remove (method); + _processingStack.Remove (stackNode); + _processingStackVersion++; - processedMethods[method] = methodValue; + _processedMethods[method] = methodValue; } void ProcessStack () { - while (processingStack.Count > 0) { - var stackNode = processingStack.First; + while (_processingStack.Count > 0) { + var stackNode = _processingStack.First; var method = stackNode.Value.Method; bool treatUnprocessedDependenciesAsNonConst = false; - if (stackNode.Value.LastAttemptStackVersion == processingStackVersion) { + if (stackNode.Value.LastAttemptStackVersion == _processingStackVersion) { // Loop was detected - the stack hasn't changed since the last time we tried to process this method // as such there's no way to resolve the situation (running the code below would produce the exact same result). @@ -180,18 +167,18 @@ void ProcessStack () // To fix this go over the stack and find the "oldest" node with the current version - the "oldest" node which // is part of the loop: LinkedListNode lastNodeWithCurrentVersion = null; - var candidateNodeToMoveToTop = processingStack.Last; + var candidateNodeToMoveToTop = _processingStack.Last; bool foundNodesWithNonCurrentVersion = false; while (candidateNodeToMoveToTop != stackNode) { var previousNode = candidateNodeToMoveToTop.Previous; - if (candidateNodeToMoveToTop.Value.LastAttemptStackVersion == processingStackVersion) { + if (candidateNodeToMoveToTop.Value.LastAttemptStackVersion == _processingStackVersion) { lastNodeWithCurrentVersion = candidateNodeToMoveToTop; } else if (lastNodeWithCurrentVersion != null) { // We've found the "oldest" node with current version and the current node is not of that version // so it's older version. Move this node to the top of the stack. - processingStack.Remove (candidateNodeToMoveToTop); - processingStack.AddFirst (candidateNodeToMoveToTop); + _processingStack.Remove (candidateNodeToMoveToTop); + _processingStack.AddFirst (candidateNodeToMoveToTop); foundNodesWithNonCurrentVersion = true; } @@ -215,14 +202,14 @@ void ProcessStack () treatUnprocessedDependenciesAsNonConst = true; } - stackNode.Value = new ProcessingNode (stackNode.Value, processingStackVersion); + stackNode.Value = new ProcessingNode (stackNode.Value, _processingStackVersion); - if (!method.HasBody) { + if (!IsMethodSupported (method)) { StoreMethodAsProcessedAndRemoveFromQueue (stackNode, ProcessedUnchangedSentinel); continue; } - var reducer = new BodyReducer (method.Body, Context); + var reducer = new BodyReducer (method.Body, _context); // // Temporary inlines any calls which return contant expression. @@ -232,7 +219,7 @@ void ProcessStack () if (!TryInlineBodyDependencies (ref reducer, treatUnprocessedDependenciesAsNonConst, out bool changed)) { // Method has unprocessed dependencies - so back off and try again later // Leave it in the stack on its current position (it should not be on the first position anymore) - Debug.Assert (processingStack.First != stackNode); + Debug.Assert (_processingStack.First != stackNode); continue; } @@ -251,7 +238,7 @@ void ProcessStack () // branch is replaced with nops. // if (reducer.RewriteBody ()) - Context.LogMessage ($"Reduced '{reducer.InstructionsReplaced}' instructions in conditional branches for [{method.DeclaringType.Module.Assembly.Name}] method {method.GetDisplayName ()}"); + _context.LogMessage ($"Reduced '{reducer.InstructionsReplaced}' instructions in conditional branches for [{method.DeclaringType.Module.Assembly.Name}] method {method.GetDisplayName ()}"); // Even if the rewriter doesn't find any branches to fold the inlining above may have changed the method enough // such that we can now deduce its return value. @@ -272,7 +259,7 @@ void ProcessStack () AnalyzeMethodForConstantResult (method, reducer.FoldedInstructions) ?? NonConstSentinel); } - Debug.Assert (processingMethods.Count == 0); + Debug.Assert (_processingMethods.Count == 0); } Instruction AnalyzeMethodForConstantResult (MethodDefinition method, Collection instructions) @@ -283,17 +270,17 @@ Instruction AnalyzeMethodForConstantResult (MethodDefinition method, Collection< if (method.ReturnType.MetadataType == MetadataType.Void) return null; - switch (Context.Annotations.GetAction (method)) { + switch (_context.Annotations.GetAction (method)) { case MethodAction.ConvertToThrow: return null; case MethodAction.ConvertToStub: - return CodeRewriterStep.CreateConstantResultInstruction (Context, method); + return CodeRewriterStep.CreateConstantResultInstruction (_context, method); } if (method.IsIntrinsic () || method.NoInlining) return null; - if (!Context.IsOptimizationEnabled (CodeOptimizations.IPConstantPropagation, method)) + if (!_context.IsOptimizationEnabled (CodeOptimizations.IPConstantPropagation, method)) return null; var analyzer = new ConstantExpressionMethodAnalyzer (method, instructions ?? method.Body.Instructions); @@ -318,12 +305,12 @@ Instruction AnalyzeMethodForConstantResult (MethodDefinition method, Collection< /// bool TryGetConstantResultForMethod (MethodDefinition method, out Instruction constantResultInstruction) { - if (!processedMethods.TryGetValue (method, out Instruction methodValue)) { - if (processingMethods.TryGetValue (method, out var stackNode)) { + if (!_processedMethods.TryGetValue (method, out Instruction methodValue)) { + if (_processingMethods.TryGetValue (method, out var stackNode)) { // Method is already in the stack - not yet processed // Move it to the top of the stack - processingStack.Remove (stackNode); - processingStack.AddFirst (stackNode); + _processingStack.Remove (stackNode); + _processingStack.AddFirst (stackNode); // Note that stack version is not changing - we're just postponing work, not resolving anything. // There's no result available for this method, so return false. @@ -342,7 +329,7 @@ bool TryGetConstantResultForMethod (MethodDefinition method, out Instruction con // Also its value has not been needed yet. Now we need to know if it's constant, so run the analyzer on it var result = AnalyzeMethodForConstantResult (method, instructions: null); Debug.Assert (result is Instruction || result == null); - processedMethods[method] = result ?? NonConstSentinel; + _processedMethods[method] = result ?? NonConstSentinel; constantResultInstruction = result; } else if (methodValue == NonConstSentinel) { // Method was processed and found to not have a constant value @@ -376,7 +363,7 @@ bool TryInlineBodyDependencies (ref BodyReducer reducer, bool treatUnprocessedDe if (md.CallingConvention == MethodCallingConvention.VarArg) break; - bool explicitlyAnnotated = Annotations.GetAction (md) == MethodAction.ConvertToStub; + bool explicitlyAnnotated = _context.Annotations.GetAction (md) == MethodAction.ConvertToStub; // Allow inlining results of instance methods which are explicitly annotated // but don't allow inling results of any other instance method. @@ -432,7 +419,7 @@ bool TryInlineBodyDependencies (ref BodyReducer reducer, bool treatUnprocessedDe if (field == null) break; - if (Context.Annotations.TryGetFieldUserValue (field, out object value)) { + if (_context.Annotations.TryGetFieldUserValue (field, out object value)) { targetResult = CodeRewriterStep.CreateConstantResultInstruction (field.FieldType, value); if (targetResult == null) break; diff --git a/src/tools/illink/src/linker/Linker/Driver.cs b/src/tools/illink/src/linker/Linker/Driver.cs index 2894e284e877c..1ddecb5003245 100644 --- a/src/tools/illink/src/linker/Linker/Driver.cs +++ b/src/tools/illink/src/linker/Linker/Driver.cs @@ -652,7 +652,6 @@ protected int SetupContext (ILogger customLogger = null) if (removeCAS) p.AddStepBefore (typeof (MarkStep), new RemoveSecurityStep ()); - p.AddStepBefore (typeof (MarkStep), new RemoveUnreachableBlocksStep ()); p.AddStepBefore (typeof (MarkStep), new MarkExportedTypesTargetStep ()); p.AddStepBefore (typeof (OutputStep), new SealerStep ()); diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/UnreachableBlock/SimpleConditionalProperty.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/UnreachableBlock/SimpleConditionalProperty.cs index 0ba573202e7a1..ef53264beb6e5 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/UnreachableBlock/SimpleConditionalProperty.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/UnreachableBlock/SimpleConditionalProperty.cs @@ -22,6 +22,7 @@ public static void Main () TestProperty_null_1 (); TestProperty_SignedComparisons (); TestProperty_UnsignedComparisons (); + TestDefaultInterface (); } [Kept] @@ -264,5 +265,42 @@ enum TestEnum [Kept] C = 2 } + + [Kept] + interface IDefaultInterface + { + [Kept] + public void NonDefault (); + + [Kept] + [ExpectBodyModified] + public void Default () + { + if (SimpleConditionalProperty.PropBool) + SimpleConditionalProperty.NeverReached_1 (); + } + } + + [Kept] + [KeptMember (".ctor()")] + [KeptInterface (typeof (IDefaultInterface))] + class ImplementsDefaultInterface : IDefaultInterface + { + [Kept] + [ExpectBodyModified] + public void NonDefault () + { + if (PropBool) + NeverReached_1 (); + } + } + + [Kept] + static void TestDefaultInterface () + { + IDefaultInterface i = new ImplementsDefaultInterface (); + i.NonDefault (); + i.Default (); + } } } \ No newline at end of file