Add ControlFlowSimplification pass

ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs

@@ -43,6 +43,7 @@ namespace ICSharpCode.Decompiler.CSharp
 		List<IILTransform> ilTransforms = new List<IILTransform> {
 			new OptimizingTransform(),
 			new LoopDetection(),
+			new ControlFlowSimplification(),
 			new TransformingVisitor(),
 			new TransformStackIntoVariables()
 		};

ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj

@@ -104,6 +104,7 @@
     <Compile Include="IL\Instructions\UnaryInstruction.cs" />
     <Compile Include="IL\IInlineContext.cs" />
     <Compile Include="IL\NRTypeExtensions.cs" />
+    <Compile Include="IL\Transforms\ControlFlowSimplification.cs" />
     <Compile Include="IL\Transforms\LoopDetection.cs" />
     <Compile Include="IL\Transforms\OptimizingTransform.cs" />
     <Compile Include="IL\Transforms\TransformingVisitor.cs" />

ICSharpCode.Decompiler/IL/Instructions/Block.cs

@@ -104,6 +104,15 @@ namespace ICSharpCode.Decompiler.IL
 			return clone;
 		}
 		
+		internal override void CheckInvariant()
+		{
+			base.CheckInvariant();
+			for (int i = 0; i < Instructions.Count - 1; i++) {
+				// only the last instruction may have an unreachable endpoint
+				Debug.Assert(!Instructions[i].HasFlag(InstructionFlags.EndPointUnreachable));
+			}
+		}
+		
 		public override StackType ResultType {
 			get {
 				return finalInstruction.ResultType;

ICSharpCode.Decompiler/IL/Instructions/ILInstruction.cs

@@ -39,15 +39,16 @@ namespace ICSharpCode.Decompiler.IL
 			this.OpCode = opCode;
 		}
 		
-		internal static void ValidateArgument(ILInstruction inst)
+		protected void ValidateArgument(ILInstruction inst)
 		{
 			if (inst == null)
 				throw new ArgumentNullException("inst");
 			if (inst.ResultType == StackType.Void)
 				throw new ArgumentException("Argument must not be of type void", "inst");
+			Debug.Assert(!this.IsDescendantOf(inst), "ILAst must form a tree");
 		}
 		
-		internal void ValidateChild(ILInstruction inst)
+		protected void ValidateChild(ILInstruction inst)
 		{
 			if (inst == null)
 				throw new ArgumentNullException("inst");
@@ -444,6 +445,7 @@ namespace ICSharpCode.Decompiler.IL
 		protected internal void InstructionCollectionAdded(ILInstruction newChild)
 		{
 			Debug.Assert(GetChild(newChild.ChildIndex) == newChild);
+			Debug.Assert(!this.IsDescendantOf(newChild), "ILAst must form a tree");
 			newChild.parent = this;
 			if (refCount > 0)
 				newChild.AddRef();

ICSharpCode.Decompiler/IL/Instructions/InstructionCollection.cs

@@ -298,5 +298,23 @@ namespace ICSharpCode.Decompiler.IL
 			}
 			return removed;
 		}
+		
+		// more efficient versions of some LINQ methods:
+		public T First()
+		{
+			return list[0];
+		}
+		
+		public T Last()
+		{
+			return list[list.Count - 1];
+		}
+		
+		public T ElementAtOrDefault(int index)
+		{
+			if (index >= 0 && index < list.Count)
+				return list[index];
+			return null;
+		}
 	}
 }

ICSharpCode.Decompiler/IL/Transforms/ControlFlowSimplification.cs

@@ -0,0 +1,172 @@
+// Copyright (c) 2014 Daniel Grunwald
+// 
+// Permission is hereby granted, free of charge, to any person obtaining a copy of this
+// software and associated documentation files (the "Software"), to deal in the Software
+// without restriction, including without limitation the rights to use, copy, modify, merge,
+// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons
+// to whom the Software is furnished to do so, subject to the following conditions:
+// 
+// The above copyright notice and this permission notice shall be included in all copies or
+// substantial portions of the Software.
+// 
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED,
+// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR
+// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE
+// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+// DEALINGS IN THE SOFTWARE.
+using System;
+using System.Collections;
+using System.Diagnostics;
+using System.Linq;
+using ICSharpCode.Decompiler.FlowAnalysis;
+
+namespace ICSharpCode.Decompiler.IL.Transforms
+{
+	/// <summary>
+	/// Detects 'if' structure and other non-loop aspects of control flow.
+	/// </summary>
+	/// <remarks>
+	/// Order dependency: should run after loop detection.
+	/// Blocks should be basic blocks prior to this transform.
+	/// After this transform, they will be extended basic blocks.
+	/// </remarks>
+	public class ControlFlowSimplification : IILTransform
+	{
+		public void Run(ILFunction function, ILTransformContext context)
+		{
+			foreach (var container in function.Descendants.OfType<BlockContainer>()) {
+				Run(container, context);
+			}
+		}
+		
+		BlockContainer currentContainer;
+		ControlFlowNode[] controlFlowGraph;
+		
+		void Run(BlockContainer container, ILTransformContext context)
+		{
+			currentContainer = container;
+			controlFlowGraph = LoopDetection.BuildCFG(container);
+			Dominance.ComputeDominance(controlFlowGraph[0], context.CancellationToken);
+			BuildConditionStructure(controlFlowGraph[0]);
+			controlFlowGraph = null;
+			currentContainer = null;
+			container.Blocks.RemoveAll(b => b.Parent != container || b.Instructions.Count == 0);
+		}
+
+		/// <summary>
+		/// Builds structured control flow for the block associated with the control flow node.
+		/// </summary>
+		/// <remarks>
+		/// After a block was processed, it should use structured control flow
+		/// and have just a single 'regular' exit point (last branch instruction in the block)
+		/// </remarks>
+		void BuildConditionStructure(ControlFlowNode cfgNode)
+		{
+			Block block = (Block)cfgNode.UserData;
+			// First, process the children in the dominator tree.
+			// This ensures that blocks being embedded into this block are already fully processed.
+			foreach (var child in cfgNode.DominatorTreeChildren)
+				BuildConditionStructure(child);
+			// Last instruction is one with unreachable endpoint
+			// (guaranteed by combination of BlockContainer and Block invariants)
+			Debug.Assert(block.Instructions.Last().HasFlag(InstructionFlags.EndPointUnreachable));
+			ILInstruction exitInst = block.Instructions.Last();
+			
+			// Previous-to-last instruction might have conditional control flow,
+			// usually an IfInstruction with a branch:
+			IfInstruction ifInst = block.Instructions.ElementAtOrDefault(block.Instructions.Count - 2) as IfInstruction;
+			if (ifInst != null && ifInst.FalseInst.OpCode == OpCode.Nop) {
+				ILInstruction trueExitInst;
+				if (IsUsableBranchToChild(cfgNode, ifInst.TrueInst)) {
+					// "if (...) goto targetBlock; exitInst;"
+					// -> "if (...) { targetBlock } exitInst;"
+					var targetBlock = ((Branch)ifInst.TrueInst).TargetBlock;
+					// The targetBlock was already processed, we can embed it into the if statement:
+					ifInst.TrueInst = targetBlock;
+					trueExitInst = targetBlock.Instructions.LastOrDefault();
+					if (CompatibleExitInstruction(exitInst, trueExitInst)) {
+						// "if (...) { ...; goto exitPoint } goto exitPoint;"
+						// -> "if (...) { ... } goto exitPoint;"
+						targetBlock.Instructions.RemoveAt(targetBlock.Instructions.Count - 1);
+						trueExitInst = null;
+					}
+				} else {
+					trueExitInst = ifInst.TrueInst;
+				}
+				if (IsUsableBranchToChild(cfgNode, exitInst)) {
+					var targetBlock = ((Branch)exitInst).TargetBlock;
+					var falseExitInst = targetBlock.Instructions.LastOrDefault();
+					if (CompatibleExitInstruction(trueExitInst, falseExitInst)) {
+						// if (...) { ...; goto exitPoint; } goto nextBlock; nextBlock: ...; goto exitPoint;
+						// -> if (...) { ... } else { ... } goto exitPoint;
+						targetBlock.Instructions.RemoveAt(targetBlock.Instructions.Count - 1);
+						ifInst.FalseInst = targetBlock;
+						exitInst = block.Instructions[block.Instructions.Count - 1] = falseExitInst;
+						Block trueBlock = ifInst.TrueInst as Block;
+						if (trueBlock != null) {
+							Debug.Assert(trueExitInst == trueBlock.Instructions.Last());
+							trueBlock.Instructions.RemoveAt(trueBlock.Instructions.Count - 1);
+						} else {
+							Debug.Assert(trueExitInst == ifInst.TrueInst);
+							ifInst.TrueInst = new Nop { ILRange = ifInst.TrueInst.ILRange };
+						}
+					}
+				}
+				if (ifInst.FalseInst.OpCode != OpCode.Nop && ifInst.FalseInst.ILRange.Start < ifInst.TrueInst.ILRange.Start
+				    || ifInst.TrueInst.OpCode == OpCode.Nop)
+				{
+					// swap true and false branches of if, to bring them in the same order as the IL code
+					var oldTrue = ifInst.TrueInst;
+					ifInst.TrueInst = ifInst.FalseInst;
+					ifInst.FalseInst = oldTrue;
+					if (ifInst.Condition.OpCode == OpCode.LogicNot)
+						ifInst.Condition = ifInst.Condition.Children.Single();
+					else
+						ifInst.Condition = new LogicNot(ifInst.Condition);
+				}
+			}
+			if (IsUsableBranchToChild(cfgNode, exitInst)) {
+				// "...; goto usableblock;"
+				// -> embed target block in this block
+				var targetBlock = ((Branch)exitInst).TargetBlock;
+				Debug.Assert(exitInst == block.Instructions.Last());
+				block.Instructions.RemoveAt(block.Instructions.Count - 1);
+				block.Instructions.AddRange(targetBlock.Instructions);
+				targetBlock.Instructions.Clear();
+			}
+		}
+		
+		bool IsUsableBranchToChild(ControlFlowNode cfgNode, ILInstruction potentialBranchInstruction)
+		{
+			Branch br = potentialBranchInstruction as Branch;
+			if (br == null || br.PopCount != 0)
+				return false;
+			var targetBlock = br.TargetBlock;
+			return targetBlock.Parent == currentContainer && cfgNode.Dominates(controlFlowGraph[targetBlock.ChildIndex])
+				&& targetBlock.IncomingEdgeCount == 1 && targetBlock.FinalInstruction.OpCode == OpCode.Nop;
+		}
+		
+		static bool CompatibleExitInstruction(ILInstruction exit1, ILInstruction exit2)
+		{
+			if (exit1 == null || exit2 == null || exit1.OpCode != exit2.OpCode)
+				return false;
+			switch (exit1.OpCode) {
+				case OpCode.Branch:
+					Branch br1 = (Branch)exit1;
+					Branch br2 = (Branch)exit2;
+					return br1.TargetBlock == br2.TargetBlock && br1.PopCount == br2.PopCount;
+				case OpCode.Leave:
+					Leave leave1 = (Leave)exit1;
+					Leave leave2 = (Leave)exit2;
+					return leave1.TargetContainer == leave2.TargetContainer && leave1.PopCount == leave2.PopCount;
+				case OpCode.Return:
+					Return ret1 = (Return)exit1;
+					Return ret2 = (Return)exit2;
+					return ret1.ReturnValue == null && ret2.ReturnValue == null;
+				default:
+					return false;
+			}
+		}
+	}
+}

ICSharpCode.Decompiler/IL/Transforms/LoopDetection.cs

@@ -39,18 +39,15 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 		/// <summary>
 		/// Constructs a control flow graph for the blocks in the given block container.
 		/// The graph nodes will have the same indices as the blocks in the block container.
-		/// An additional exit node is used to signal when a block potentially falls through
-		/// to the endpoint of the BlockContainer.
 		/// Return statements, exceptions, or branches leaving the block container are not
 		/// modeled by the control flow graph.
 		/// </summary>
-		static ControlFlowNode BuildCFG(BlockContainer bc)
+		internal static ControlFlowNode[] BuildCFG(BlockContainer bc)
 		{
 			ControlFlowNode[] nodes = new ControlFlowNode[bc.Blocks.Count];
 			for (int i = 0; i < nodes.Length; i++) {
 				nodes[i] = new ControlFlowNode { UserData = bc.Blocks[i] };
 			}
-			ControlFlowNode exit = new ControlFlowNode();
 			
 			// Create edges:
 			for (int i = 0; i < bc.Blocks.Count; i++) {
@@ -67,18 +64,9 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 						// like a return statement or exceptional exit.
 					}
 				}
-				if (!block.HasFlag(InstructionFlags.EndPointUnreachable))
-					sourceNode.AddEdgeTo(exit);
 			}
 			
-			if (nodes[0].Predecessors.Count != 0) {
+			return nodes;
-				// Create artificial entry point without predecessors:
-				ControlFlowNode entry = new ControlFlowNode();
-				entry.AddEdgeTo(nodes[0]);
-				return entry;
-			} else {
-				return nodes[0];
-			}
 		}
 		#endregion
 		
@@ -97,7 +85,8 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 		/// </summary>
 		public void Run(BlockContainer blockContainer, ILTransformContext context)
 		{
-			var entryPoint = BuildCFG(blockContainer);
+			var cfg = BuildCFG(blockContainer);
+			var entryPoint = cfg[0];
 			Dominance.ComputeDominance(entryPoint, context.CancellationToken);
 			FindLoops(entryPoint);
 		}

ICSharpCode.Decompiler/IL/Transforms/OptimizingTransform.cs

@@ -16,6 +16,8 @@
 // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
 // DEALINGS IN THE SOFTWARE.
 using System;
+using System.Collections.Generic;
+using System.Diagnostics;
 using System.Linq;
 
 namespace ICSharpCode.Decompiler.IL.Transforms
@@ -48,10 +50,16 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 					}
 				}
 			}
+			HashSet<Block> visitedBlocks = new HashSet<Block>();
 			foreach (var branch in function.Descendants.OfType<Branch>()) {
 				// Resolve indirect branches
 				var targetBlock = branch.TargetBlock;
+				visitedBlocks.Clear();
 				while (targetBlock.Instructions.Count == 1 && targetBlock.Instructions[0].OpCode == OpCode.Branch) {
+					if (!visitedBlocks.Add(targetBlock)) {
+						// prevent infinite loop when indirect branches point in infinite loop
+						break;
+					}
 					var nextBranch = (Branch)targetBlock.Instructions[0];
 					branch.TargetBlock = nextBranch.TargetBlock;
 					branch.PopCount += nextBranch.PopCount;
@@ -74,16 +82,9 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 				foreach (var block in container.Blocks) {
 					if (block.Instructions.Count == 0)
 						continue; // block is already marked for deletion
-					Branch br = block.Instructions.Last() as Branch;
+					while (CombineBlockWithNextBlock(container, block)) {
-					if (br != null && br.TargetBlock.Parent == container && br.TargetBlock.IncomingEdgeCount == 1) {
+						// repeat combining blocks until it is no longer possible
-						// We could inline the target block into this block
+						// (this loop terminates because a block is deleted in every iteration)
-						// Do so only if the block will stay a basic block -- we don't want extended basic blocks prior to LoopDetection.
-						var targetBlock = br.TargetBlock;
-						if (targetBlock.Instructions.Count == 1 || !targetBlock.Instructions[targetBlock.Instructions.Count - 2].HasFlag(InstructionFlags.MayBranch)) {
-							block.Instructions.Remove(br);
-							block.Instructions.AddRange(targetBlock.Instructions);
-							targetBlock.Instructions.Clear(); // mark targetBlock for deletion
-						}
 					}
 				}
 				// Remove return blocks that are no longer reachable:
@@ -98,5 +99,25 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 			var ret = targetBlock.Instructions[0] as Return;
 			return ret != null && (ret.ReturnValue == null || ret.ReturnValue.OpCode == OpCode.LdLoc);
 		}
+		
+		static bool CombineBlockWithNextBlock(BlockContainer container, Block block)
+		{
+			Debug.Assert(container == block.Parent);
+			// Ensure the block will stay a basic block -- we don't want extended basic blocks prior to LoopDetection.
+			// TODO: when LoopDetection is complete, check that it really can't handle EBBs
+			if (block.Instructions.Count > 1 && block.Instructions[block.Instructions.Count - 2].HasFlag(InstructionFlags.MayBranch))
+				return false;
+			Branch br = block.Instructions.Last() as Branch;
+			// Check whether we can combine the target block with this block
+			if (br == null || br.TargetBlock.Parent != container || br.TargetBlock.IncomingEdgeCount != 1)
+				return false;
+			if (br.TargetBlock == block)
+				return false; // don't inline block into itself
+			var targetBlock = br.TargetBlock;
+			block.Instructions.Remove(br);
+			block.Instructions.AddRange(targetBlock.Instructions);
+			targetBlock.Instructions.Clear(); // mark targetBlock for deletion
+			return true;
+		}
 	}
 }