Fix #1430: Improve performance in ILInlining.

ICSharpCode.Decompiler/IL/Transforms/ILInlining.cs

@@ -80,19 +80,16 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 		{
 			bool modified = false;
 			var instructions = block.Instructions;
-			for (int i = 0; i < instructions.Count;) {
+			for (int i = instructions.Count - 1; i >= 0; i--) {
 				if (instructions[i] is StLoc inst) {
 					InliningOptions options = InliningOptions.None;
 					if (IsCatchWhenBlock(block) || IsInConstructorInitializer(function, inst, ref ctorCallStart))
 						options = InliningOptions.Aggressive;
 					if (InlineOneIfPossible(block, i, options, context)) {
 						modified = true;
-						i = Math.Max(0, i - 1);
-						// Go back one step
 						continue;
 					}
 				}
-				i++;
 			}
 			return modified;
 		}
@@ -217,20 +214,26 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 		/// </summary>
 		static bool DoInline(ILVariable v, ILInstruction inlinedExpression, ILInstruction next, InliningOptions options, ILTransformContext context)
 		{
-			var r = FindLoadInNext(next, v, inlinedExpression, out var loadInst);
-			if (r == FindResult.Found) {
+			var r = FindLoadInNext(next, v, inlinedExpression, options);
+			if (r.Type == FindResultType.Found || r.Type == FindResultType.NamedArgument) {
+				var loadInst = r.LoadInst;
 				if (loadInst.OpCode == OpCode.LdLoca) {
-					if (!IsGeneratedValueTypeTemporary(next, (LdLoca)loadInst, v, inlinedExpression))
+					if (!IsGeneratedValueTypeTemporary((LdLoca)loadInst, v, inlinedExpression))
 						return false;
 				} else {
 					Debug.Assert(loadInst.OpCode == OpCode.LdLoc);
 					bool aggressive = (options & InliningOptions.Aggressive) != 0;
 					if (!aggressive && v.Kind != VariableKind.StackSlot
-						&& !NonAggressiveInlineInto(next, loadInst, inlinedExpression, v)) {
+						&& !NonAggressiveInlineInto(next, r, inlinedExpression, v)) {
 						return false;
 					}
 				}
 
+				if (r.Type == FindResultType.NamedArgument) {
+					NamedArgumentTransform.IntroduceNamedArgument(r.CallArgument, context);
+					// Now that the argument is evaluated early, we can inline as usual
+				}
+
 				context.Step($"Inline variable '{v.Name}'", inlinedExpression);
 				// Assign the ranges of the ldloc instruction:
 				inlinedExpression.AddILRange(loadInst);
@@ -243,9 +246,6 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 					loadInst.ReplaceWith(inlinedExpression);
 				}
 				return true;
-			} else if (r == FindResult.NamedArgument && (options & InliningOptions.IntroduceNamedArguments) != 0) {
-				return NamedArgumentTransform.DoInline(v, (StLoc)inlinedExpression.Parent, (LdLoc)loadInst,
-					options, context);
 			}
 			return false;
 		}
@@ -253,10 +253,9 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 		/// <summary>
 		/// Is this a temporary variable generated by the C# compiler for instance method calls on value type values
 		/// </summary>
-		/// <param name="next">The next top-level expression</param>
 		/// <param name="loadInst">The load instruction (a descendant within 'next')</param>
 		/// <param name="v">The variable being inlined.</param>
-		static bool IsGeneratedValueTypeTemporary(ILInstruction next, LdLoca loadInst, ILVariable v, ILInstruction inlinedExpression)
+		static bool IsGeneratedValueTypeTemporary(LdLoca loadInst, ILVariable v, ILInstruction inlinedExpression)
 		{
 			Debug.Assert(loadInst.Variable == v);
 			// Inlining a value type variable is allowed only if the resulting code will maintain the semantics
@@ -321,8 +320,15 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 		/// <param name="next">The next top-level expression</param>
 		/// <param name="loadInst">The load within 'next'</param>
 		/// <param name="inlinedExpression">The expression being inlined</param>
-		static bool NonAggressiveInlineInto(ILInstruction next, ILInstruction loadInst, ILInstruction inlinedExpression, ILVariable v)
+		static bool NonAggressiveInlineInto(ILInstruction next, FindResult findResult, ILInstruction inlinedExpression, ILVariable v)
 		{
+			if (findResult.Type == FindResultType.NamedArgument) {
+				var originalStore = (StLoc)inlinedExpression.Parent;
+				return !originalStore.ILStackWasEmpty;
+			}
+			Debug.Assert(findResult.Type == FindResultType.Found);
+
+			var loadInst = findResult.LoadInst;
 			Debug.Assert(loadInst.IsDescendantOf(next));
 			
 			// decide based on the source expression being inlined
@@ -406,10 +412,10 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 		/// </summary>
 		public static bool CanInlineInto(ILInstruction expr, ILVariable v, ILInstruction expressionBeingMoved)
 		{
-			return FindLoadInNext(expr, v, expressionBeingMoved, out _) == FindResult.Found;
+			return FindLoadInNext(expr, v, expressionBeingMoved, InliningOptions.None).Type == FindResultType.Found;
 		}
 
-		internal enum FindResult
+		internal enum FindResultType
 		{
 			/// <summary>
 			/// Found a load; inlining is possible.
@@ -432,19 +438,47 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 			NamedArgument,
 		}
 
+		internal readonly struct FindResult
+		{
+			public readonly FindResultType Type;
+			public readonly ILInstruction LoadInst; // ldloc or ldloca instruction that loads the variable to be inlined
+			public readonly ILInstruction CallArgument; // argument of call that needs to be promoted to a named argument
+
+			private FindResult(FindResultType type, ILInstruction loadInst, ILInstruction callArg)
+			{
+				this.Type = type;
+				this.LoadInst = loadInst;
+				this.CallArgument = callArg;
+			}
+
+			public static readonly FindResult Stop = new FindResult(FindResultType.Stop, null, null);
+			public static readonly FindResult Continue = new FindResult(FindResultType.Continue, null, null);
+
+			public static FindResult Found(ILInstruction loadInst)
+			{
+				Debug.Assert(loadInst.OpCode == OpCode.LdLoc || loadInst.OpCode == OpCode.LdLoca);
+				return new FindResult(FindResultType.Found, loadInst, null);
+			}
+
+			public static FindResult NamedArgument(ILInstruction loadInst, ILInstruction callArg)
+			{
+				Debug.Assert(loadInst.OpCode == OpCode.LdLoc || loadInst.OpCode == OpCode.LdLoca);
+				Debug.Assert(callArg.Parent is CallInstruction);
+				return new FindResult(FindResultType.NamedArgument, loadInst, callArg);
+			}
+		}
+
 		/// <summary>
 		/// Finds the position to inline to.
 		/// </summary>
 		/// <returns>true = found; false = cannot continue search; null = not found</returns>
-		internal static FindResult FindLoadInNext(ILInstruction expr, ILVariable v, ILInstruction expressionBeingMoved, out ILInstruction loadInst)
+		internal static FindResult FindLoadInNext(ILInstruction expr, ILVariable v, ILInstruction expressionBeingMoved, InliningOptions options)
 		{
-			loadInst = null;
 			if (expr == null)
 				return FindResult.Stop;
 			if (expr.MatchLdLoc(v) || expr.MatchLdLoca(v)) {
 				// Match found, we can inline
-				loadInst = expr;
-				return FindResult.Found;
+				return FindResult.Found(expr);
 			} else if (expr is Block block) {
 				// Inlining into inline-blocks?
 				switch (block.Kind) {
@@ -455,17 +489,17 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 						// Allow inlining into the first instruction of the block
 						if (block.Instructions.Count == 0)
 							return FindResult.Stop;
-						return NoContinue(FindLoadInNext(block.Instructions[0], v, expressionBeingMoved, out loadInst));
+						return NoContinue(FindLoadInNext(block.Instructions[0], v, expressionBeingMoved, options));
 						// If FindLoadInNext() returns null, we still can't continue searching
 						// because we can't inline over the remainder of the block.
 					case BlockKind.CallWithNamedArgs:
-						return NamedArgumentTransform.CanExtendNamedArgument(block, v, expressionBeingMoved, out loadInst);
+						return NamedArgumentTransform.CanExtendNamedArgument(block, v, expressionBeingMoved);
 					default:
 						return FindResult.Stop;
 				}
 			} else if (expr is BlockContainer container && container.EntryPoint.IncomingEdgeCount == 1) {
 				// Possibly a switch-container, allow inlining into the switch instruction:
-				return NoContinue(FindLoadInNext(container.EntryPoint.Instructions[0], v, expressionBeingMoved, out loadInst));
+				return NoContinue(FindLoadInNext(container.EntryPoint.Instructions[0], v, expressionBeingMoved, options));
 				// If FindLoadInNext() returns null, we still can't continue searching
 				// because we can't inline over the remainder of the blockcontainer.
 			} else if (expr is NullableRewrap) {
@@ -479,10 +513,10 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 					return FindResult.Stop;
 				
 				// Recursively try to find the load instruction
-				FindResult r = FindLoadInNext(child, v, expressionBeingMoved, out loadInst);
-				if (r != FindResult.Continue) {
-					if (r == FindResult.Stop && expr is CallInstruction call)
-						return NamedArgumentTransform.CanIntroduceNamedArgument(call, child, v, out loadInst);
+				FindResult r = FindLoadInNext(child, v, expressionBeingMoved, options);
+				if (r.Type != FindResultType.Continue) {
+					if (r.Type == FindResultType.Stop && (options & InliningOptions.IntroduceNamedArguments) != 0 && expr is CallInstruction call)
+						return NamedArgumentTransform.CanIntroduceNamedArgument(call, child, v, expressionBeingMoved);
 					return r;
 				}
 			}
@@ -494,7 +528,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 
 		private static FindResult NoContinue(FindResult findResult)
 		{
-			if (findResult == FindResult.Continue)
+			if (findResult.Type == FindResultType.Continue)
 				return FindResult.Stop;
 			else
 				return findResult;

ICSharpCode.Decompiler/IL/Transforms/NamedArgumentTransform.cs

@@ -8,12 +8,12 @@ using ICSharpCode.Decompiler.TypeSystem;
 namespace ICSharpCode.Decompiler.IL.Transforms
 {
 	using FindResult = ILInlining.FindResult;
+	using FindResultType = ILInlining.FindResultType;
 
 	class NamedArgumentTransform : IStatementTransform
 	{
-		public static FindResult CanIntroduceNamedArgument(CallInstruction call, ILInstruction child, ILVariable v, out ILInstruction loadInst)
+		public static FindResult CanIntroduceNamedArgument(CallInstruction call, ILInstruction child, ILVariable v, ILInstruction expressionBeingMoved)
 		{
-			loadInst = null;
 			Debug.Assert(child.Parent == call);
 			if (call.IsInstanceCall && child.ChildIndex == 0)
 				return FindResult.Stop; // cannot use named arg to move expressionBeingMoved before this pointer
@@ -29,27 +29,27 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 			if (call.Method.Parameters.Any(p => string.IsNullOrEmpty(p.Name)))
 				return FindResult.Stop; // cannot use named arguments
 			for (int i = child.ChildIndex; i < call.Arguments.Count; i++) {
-				if (call.Arguments[i] is LdLoc ldloc && ldloc.Variable == v) {
-					loadInst = ldloc;
-					return FindResult.NamedArgument;
+				var r = ILInlining.FindLoadInNext(call.Arguments[i], v, expressionBeingMoved, InliningOptions.None);
+				if (r.Type == FindResultType.Found) {
+					return FindResult.NamedArgument(r.LoadInst, call.Arguments[i]);
 				}
 			}
 			return FindResult.Stop;
 		}
 
-		internal static FindResult CanExtendNamedArgument(Block block, ILVariable v, ILInstruction expressionBeingMoved, out ILInstruction loadInst)
+		internal static FindResult CanExtendNamedArgument(Block block, ILVariable v, ILInstruction expressionBeingMoved)
 		{
 			Debug.Assert(block.Kind == BlockKind.CallWithNamedArgs);
 			var firstArg = ((StLoc)block.Instructions[0]).Value;
-			var r = ILInlining.FindLoadInNext(firstArg, v, expressionBeingMoved, out loadInst);
-			if (r == FindResult.Found || r == FindResult.NamedArgument) {
+			var r = ILInlining.FindLoadInNext(firstArg, v, expressionBeingMoved, InliningOptions.IntroduceNamedArguments);
+			if (r.Type == FindResultType.Found || r.Type == FindResultType.NamedArgument) {
 				return r; // OK, inline into first instruction of block
 			}
 			var call = (CallInstruction)block.FinalInstruction;
 			if (call.IsInstanceCall) {
 				// For instance calls, block.Instructions[0] is the argument
 				// for the 'this' pointer. We can only insert at position 1.
-				if (r == FindResult.Stop) {
+				if (r.Type == FindResultType.Stop) {
 					// error: can't move expressionBeingMoved after block.Instructions[0]
 					return FindResult.Stop;
 				}
@@ -57,27 +57,30 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 				// it's possible that the place we actually need to inline into
 				// is within block.Instructions[1]:
 				if (block.Instructions.Count > 1) {
-					r = ILInlining.FindLoadInNext(block.Instructions[1], v, expressionBeingMoved, out loadInst);
-					if (r == FindResult.Found || r == FindResult.NamedArgument) {
+					r = ILInlining.FindLoadInNext(block.Instructions[1], v, expressionBeingMoved, InliningOptions.IntroduceNamedArguments);
+					if (r.Type == FindResultType.Found || r.Type == FindResultType.NamedArgument) {
 						return r; // OK, inline into block.Instructions[1]
 					}
 				}
 			}
 			foreach (var arg in call.Arguments) {
 				if (arg.MatchLdLoc(v)) {
-					loadInst = arg;
-					return FindResult.NamedArgument;
+					return FindResult.NamedArgument(arg, arg);
 				}
 			}
 			return FindResult.Stop;
 		}
 
-		internal static bool DoInline(ILVariable v, StLoc originalStore, LdLoc loadInst, InliningOptions options, ILTransformContext context)
+		/// <summary>
+		/// Introduce a named argument for 'arg' and evaluate it before the other arguments
+		/// (except for the "this" pointer)
+		/// </summary>
+		internal static void IntroduceNamedArgument(ILInstruction arg, ILTransformContext context)
 		{
-			if ((options & InliningOptions.Aggressive) == 0 && originalStore.ILStackWasEmpty)
-				return false;
-			context.Step($"Introduce named argument '{v.Name}'", originalStore);
-			var call = (CallInstruction)loadInst.Parent;
+			var call = (CallInstruction)arg.Parent;
+			Debug.Assert(context.Function == call.Ancestors.OfType<ILFunction>().First());
+			var v = context.Function.RegisterVariable(VariableKind.NamedArgument, arg.InferType(context.TypeSystem));
+			context.Step($"Introduce named argument '{v.Name}'", arg);
 			if (!(call.Parent is Block namedArgBlock) || namedArgBlock.Kind != BlockKind.CallWithNamedArgs) {
 				// create namedArgBlock:
 				namedArgBlock = new Block(BlockKind.CallWithNamedArgs);
@@ -88,15 +91,15 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 					if (CallInstruction.ExpectedTypeForThisPointer(thisVarType) == StackType.Ref) {
 						thisVarType = new ByReferenceType(thisVarType);
 					}
-					var function = call.Ancestors.OfType<ILFunction>().First();
-					var thisArgVar = function.RegisterVariable(VariableKind.NamedArgument, thisVarType, "this_arg");
+					var thisArgVar = context.Function.RegisterVariable(VariableKind.NamedArgument, thisVarType, "this_arg");
 					namedArgBlock.Instructions.Add(new StLoc(thisArgVar, call.Arguments[0]));
 					call.Arguments[0] = new LdLoc(thisArgVar);
 				}
 			}
-			v.Kind = VariableKind.NamedArgument;
-			namedArgBlock.Instructions.Insert(call.IsInstanceCall ? 1 : 0, originalStore);
-			return true;
+			int argIndex = arg.ChildIndex;
+			Debug.Assert(call.Arguments[argIndex] == arg);
+			namedArgBlock.Instructions.Insert(call.IsInstanceCall ? 1 : 0, new StLoc(v, arg));
+			call.Arguments[argIndex] = new LdLoc(v);
 		}
 
 		public void Run(Block block, int pos, StatementTransformContext context)