Browse Source

ConditionDetection: Try to prefer 'break;' over other gotos

pull/734/merge
Daniel Grunwald 8 years ago
parent
commit
cc33c27d9c
  1. 19
      ICSharpCode.Decompiler/IL/ControlFlow/ConditionDetection.cs
  2. 10
      ICSharpCode.Decompiler/IL/Instructions/BlockContainer.cs
  3. 8
      ICSharpCode.Decompiler/IL/Transforms/DelegateConstruction.cs
  4. 2
      ICSharpCode.Decompiler/Tests/TestCases/Pretty/Loops.cs

19
ICSharpCode.Decompiler/IL/ControlFlow/ConditionDetection.cs

@ -90,7 +90,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow @@ -90,7 +90,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
private void HandleIfInstruction(ControlFlowNode cfgNode, Block block, IfInstruction ifInst, ref ILInstruction exitInst)
{
if (IsBranchToLaterTarget(ifInst.TrueInst, exitInst)) {
if (ShouldSwapIfTargets(ifInst.TrueInst, exitInst)) {
// "if (c) goto lateBlock; goto earlierBlock;"
// -> "if (!c)" goto earlierBlock; goto lateBlock;
// This reordering should make the if structure correspond more closely to the original C# source code
@ -219,10 +219,11 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow @@ -219,10 +219,11 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
return inst;
}
bool IsBranchToLaterTarget(ILInstruction inst1, ILInstruction inst2)
bool ShouldSwapIfTargets(ILInstruction inst1, ILInstruction inst2)
{
Block block1 = null, block2 = null;
if (inst1.MatchBranch(out block1) && inst2.MatchBranch(out block2)) {
// prefer arranging stuff in IL order
return block1.ILRange.Start > block2.ILRange.Start;
}
BlockContainer container1, container2;
@ -236,6 +237,20 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow @@ -236,6 +237,20 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
container2 = block2?.Parent as BlockContainer;
return container2 == null || container2.IsDescendantOf(container1);
}
if (inst1.MatchBranch(out block1) && inst2.MatchLeave(out container2)
&& block1.IncomingEdgeCount > 1)
{
// if (..) goto x; leave c;
// Unless x can be inlined, it's better to swap the order if the 'leave'
// has a chance to turn into a 'break;' or 'return;'
if (container2.Parent is ILFunction) {
return true; // return
}
if (container2.EntryPoint.IncomingEdgeCount > 1) {
// break
return BlockContainer.FindClosestContainer(inst2) == container2;
}
}
return false;
}

10
ICSharpCode.Decompiler/IL/Instructions/BlockContainer.cs

@ -213,5 +213,15 @@ namespace ICSharpCode.Decompiler.IL @@ -213,5 +213,15 @@ namespace ICSharpCode.Decompiler.IL
Debug.Assert(postOrder[0] == Blocks[0]);
Blocks.ReplaceList(postOrder);
}
public static BlockContainer FindClosestContainer(ILInstruction inst)
{
while (inst != null) {
if (inst is BlockContainer bc)
return bc;
inst = inst.Parent;
}
return null;
}
}
}

8
ICSharpCode.Decompiler/IL/Transforms/DelegateConstruction.cs

@ -75,13 +75,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms @@ -75,13 +75,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms
private BlockContainer FindBlockContainer(Block block)
{
ILInstruction current = block;
while (current != null) {
current = current.Parent;
if (current is BlockContainer)
return (BlockContainer)current;
}
throw new NotSupportedException();
return BlockContainer.FindClosestContainer(block) ?? throw new NotSupportedException();
}
bool IsSimpleDisplayClass(IMethod method)

2
ICSharpCode.Decompiler/Tests/TestCases/Pretty/Loops.cs

@ -188,7 +188,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty @@ -188,7 +188,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
if (Condition("test")) {
if (Condition("continue"))
continue;
if (!Condition("break"))
if (!Condition("not-break"))
break;
}
Console.WriteLine("End of loop body");

Loading…
Cancel
Save