From 51a978624370649dbded2d47e7ac0d60282ace43 Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Fri, 25 Nov 2016 13:44:56 +0100 Subject: [PATCH] Change some transforms to block transforms. --- .../CSharp/CSharpDecompiler.cs | 26 +++--- ICSharpCode.Decompiler/DecompilerSettings.cs | 2 +- .../ICSharpCode.Decompiler.csproj | 3 + .../ICSharpCode.Decompiler.ruleset | 81 +++++++++++++++++++ .../IL/Transforms/BlockTransform.cs | 12 ++- .../IL/Transforms/CopyPropagation.cs | 58 +++++++------ .../IL/Transforms/ExpressionTransforms.cs | 16 ++-- .../IL/Transforms/ILInlining.cs | 10 ++- .../Transforms/TransformArrayInitializers.cs | 22 +++-- .../IL/Transforms/TransformAssignment.cs | 44 +++++----- .../Properties/AssemblyInfo.template.cs | 3 - .../Tests/RoundtripAssembly.cs | 6 +- ILSpy/ILSpy.csproj | 1 + ILSpy/Properties/AssemblyInfo.template.cs | 2 - 14 files changed, 187 insertions(+), 99 deletions(-) create mode 100644 ICSharpCode.Decompiler/ICSharpCode.Decompiler.ruleset diff --git a/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs b/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs index a03fe41c8..8e79b3efd 100644 --- a/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs +++ b/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs @@ -63,21 +63,23 @@ namespace ICSharpCode.Decompiler.CSharp new IntroduceExitPoints(), new BlockILTransform( // per-block transforms new ConditionDetection(), - new LoopingBlockTransform( // per-block transforms that depend on each other, and thus need to loop + new ILInlining(), + new TransformAssignment(), + new CopyPropagation(), + new LoopingBlockTransform( + // per-block transforms that depend on each other, and thus need to loop. + // Pretty much all transforms that open up new expression inlining + // opportunities belong in this category. + new ExpressionTransforms(), + new TransformArrayInitializers(), + new ILInlining() ) ), - new ILInlining(), - new TransformAssignment(), - new CopyPropagation(), - new InlineCompilerGeneratedVariables(), - new ExpressionTransforms(), // must run once before "the loop" to allow RemoveDeadVariablesInit + //new InlineCompilerGeneratedVariables(), + // -- isn't InlineCompilerGeneratedVariables redundant now that we have variable splitting? new RemoveDeadVariableInit(), // must run after ExpressionTransforms because it does not handle stobj(ldloca V, ...) - new DelegateConstruction(), - new LoopingTransform( // the loop: transforms that cyclicly depend on each other - new ExpressionTransforms(), - new TransformArrayInitializers(), - new ILInlining() - ) + //new DelegateConstruction(), + // DelegateConstruction disabled because its broken since the BlockILTransform change }; } diff --git a/ICSharpCode.Decompiler/DecompilerSettings.cs b/ICSharpCode.Decompiler/DecompilerSettings.cs index 74bdc9433..133d297aa 100644 --- a/ICSharpCode.Decompiler/DecompilerSettings.cs +++ b/ICSharpCode.Decompiler/DecompilerSettings.cs @@ -27,7 +27,7 @@ namespace ICSharpCode.Decompiler /// public class DecompilerSettings : INotifyPropertyChanged { - bool anonymousMethods = true; + bool anonymousMethods = false; /// /// Decompile anonymous methods/lambdas. diff --git a/ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj b/ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj index eca936778..dd4e8adbf 100644 --- a/ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj +++ b/ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj @@ -45,9 +45,11 @@ false TRACE;DEBUG;STEP + ICSharpCode.Decompiler.ruleset false + ICSharpCode.Decompiler.ruleset @@ -179,6 +181,7 @@ + TextTemplatingFileGenerator ILOpCodes.cs diff --git a/ICSharpCode.Decompiler/ICSharpCode.Decompiler.ruleset b/ICSharpCode.Decompiler/ICSharpCode.Decompiler.ruleset new file mode 100644 index 000000000..a875fb71f --- /dev/null +++ b/ICSharpCode.Decompiler/ICSharpCode.Decompiler.ruleset @@ -0,0 +1,81 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/ICSharpCode.Decompiler/IL/Transforms/BlockTransform.cs b/ICSharpCode.Decompiler/IL/Transforms/BlockTransform.cs index 19438f11d..8b4338ac6 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/BlockTransform.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/BlockTransform.cs @@ -22,7 +22,12 @@ namespace ICSharpCode.Decompiler.IL.Transforms public class BlockTransformContext : ILTransformContext { /// - /// The container currently being processed. + /// The function containing the block currently being processed. + /// + public ILFunction Function { get; set; } + + /// + /// The container containing the block currently being processed. /// public BlockContainer Container { get; set; } @@ -72,6 +77,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms public void Run(ILFunction function, ILTransformContext context) { var blockContext = new BlockTransformContext(context); + blockContext.Function = function; foreach (var container in function.Descendants.OfType()) { context.CancellationToken.ThrowIfCancellationRequested(); var cfg = LoopDetection.BuildCFG(container); @@ -100,11 +106,11 @@ namespace ICSharpCode.Decompiler.IL.Transforms context.ControlFlowNode = cfgNode; context.Block = block; - context.Block.CheckInvariant(ILPhase.Normal); + block.CheckInvariant(ILPhase.Normal); foreach (var transform in blockTransforms) { context.CancellationToken.ThrowIfCancellationRequested(); transform.Run(context.Block, context); - context.Block.CheckInvariant(ILPhase.Normal); + block.CheckInvariant(ILPhase.Normal); } context.Stepper.EndGroup(); } diff --git a/ICSharpCode.Decompiler/IL/Transforms/CopyPropagation.cs b/ICSharpCode.Decompiler/IL/Transforms/CopyPropagation.cs index f9349dc70..337da8838 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/CopyPropagation.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/CopyPropagation.cs @@ -29,41 +29,39 @@ namespace ICSharpCode.Decompiler.IL.Transforms /// then we can replace the variable with the argument. /// 2) assignments of address-loading instructions to local variables /// - public class CopyPropagation : IILTransform + public class CopyPropagation : IBlockTransform { - public void Run(ILFunction function, ILTransformContext context) + public void Run(Block block, BlockTransformContext context) { - foreach (var block in function.Descendants.OfType()) { - for (int i = 0; i < block.Instructions.Count; i++) { - ILVariable v; - ILInstruction copiedExpr; - if (block.Instructions[i].MatchStLoc(out v, out copiedExpr)) { - if (v.IsSingleDefinition && CanPerformCopyPropagation(v, copiedExpr)) { - // un-inline the arguments of the ldArg instruction - ILVariable[] uninlinedArgs = new ILVariable[copiedExpr.Children.Count]; - for (int j = 0; j < uninlinedArgs.Length; j++) { - var arg = copiedExpr.Children[j]; - var type = context.TypeSystem.Compilation.FindType(arg.ResultType.ToKnownTypeCode()); - uninlinedArgs[j] = new ILVariable(VariableKind.StackSlot, type, arg.ResultType, arg.ILRange.Start) { - Name = "C_" + arg.ILRange.Start - }; - block.Instructions.Insert(i++, new StLoc(uninlinedArgs[j], arg)); - } - v.Scope.Variables.AddRange(uninlinedArgs); - // perform copy propagation: - foreach (var expr in v.Scope.Descendants) { - if (expr.MatchLdLoc(v)) { - var clone = copiedExpr.Clone(); - for (int j = 0; j < uninlinedArgs.Length; j++) { - clone.Children[j].ReplaceWith(new LdLoc(uninlinedArgs[j])); - } - expr.ReplaceWith(clone); + for (int i = 0; i < block.Instructions.Count; i++) { + ILVariable v; + ILInstruction copiedExpr; + if (block.Instructions[i].MatchStLoc(out v, out copiedExpr)) { + if (v.IsSingleDefinition && CanPerformCopyPropagation(v, copiedExpr)) { + // un-inline the arguments of the ldArg instruction + ILVariable[] uninlinedArgs = new ILVariable[copiedExpr.Children.Count]; + for (int j = 0; j < uninlinedArgs.Length; j++) { + var arg = copiedExpr.Children[j]; + var type = context.TypeSystem.Compilation.FindType(arg.ResultType.ToKnownTypeCode()); + uninlinedArgs[j] = new ILVariable(VariableKind.StackSlot, type, arg.ResultType, arg.ILRange.Start) { + Name = "C_" + arg.ILRange.Start + }; + block.Instructions.Insert(i++, new StLoc(uninlinedArgs[j], arg)); + } + v.Scope.Variables.AddRange(uninlinedArgs); + // perform copy propagation: + foreach (var expr in v.Scope.Descendants) { + if (expr.MatchLdLoc(v)) { + var clone = copiedExpr.Clone(); + for (int j = 0; j < uninlinedArgs.Length; j++) { + clone.Children[j].ReplaceWith(new LdLoc(uninlinedArgs[j])); } + expr.ReplaceWith(clone); } - block.Instructions.RemoveAt(i); - int c = new ILInlining().InlineInto(block, i, aggressive: false); - i -= c + 1; } + block.Instructions.RemoveAt(i); + int c = new ILInlining().InlineInto(block, i, aggressive: false); + i -= c + 1; } } } diff --git a/ICSharpCode.Decompiler/IL/Transforms/ExpressionTransforms.cs b/ICSharpCode.Decompiler/IL/Transforms/ExpressionTransforms.cs index 7b26ebf3d..3d4c7bacf 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/ExpressionTransforms.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/ExpressionTransforms.cs @@ -30,14 +30,14 @@ namespace ICSharpCode.Decompiler.IL.Transforms /// /// Should run after inlining so that the expression patterns can be detected. /// - public class ExpressionTransforms : ILVisitor, IILTransform + public class ExpressionTransforms : ILVisitor, IBlockTransform { ILTransformContext context; - - void IILTransform.Run(ILFunction function, ILTransformContext context) + + public void Run(Block block, BlockTransformContext context) { this.context = context; - function.AcceptVisitor(this); + Default(block); } protected override void Default(ILInstruction inst) @@ -46,7 +46,13 @@ namespace ICSharpCode.Decompiler.IL.Transforms child.AcceptVisitor(this); } } - + + protected internal override void VisitBlock(Block block) + { + // Don't visit child blocks; since this is a block transform + // we know those were already handled previously. + } + protected internal override void VisitComp(Comp inst) { base.VisitComp(inst); diff --git a/ICSharpCode.Decompiler/IL/Transforms/ILInlining.cs b/ICSharpCode.Decompiler/IL/Transforms/ILInlining.cs index 4dd37ab42..6cd951a8e 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/ILInlining.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/ILInlining.cs @@ -29,7 +29,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms /// /// Performs inlining transformations. /// - public class ILInlining : IILTransform + public class ILInlining : IILTransform, IBlockTransform { public void Run(ILFunction function, ILTransformContext context) { @@ -39,6 +39,11 @@ namespace ICSharpCode.Decompiler.IL.Transforms function.Variables.RemoveDead(); } + public void Run(Block block, BlockTransformContext context) + { + InlineAllInBlock(block); + } + public bool InlineAllInBlock(Block block) { bool modified = false; @@ -312,13 +317,14 @@ namespace ICSharpCode.Decompiler.IL.Transforms else return false; // abort, inlining not possible } - + /// /// Determines whether it is safe to move 'expressionBeingMoved' past 'expr' /// static bool IsSafeForInlineOver(ILInstruction expr, ILInstruction expressionBeingMoved) { return SemanticHelper.MayReorder(expressionBeingMoved, expr); + } } } diff --git a/ICSharpCode.Decompiler/IL/Transforms/TransformArrayInitializers.cs b/ICSharpCode.Decompiler/IL/Transforms/TransformArrayInitializers.cs index d6915a5fc..8be2d65af 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/TransformArrayInitializers.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/TransformArrayInitializers.cs @@ -28,20 +28,16 @@ namespace ICSharpCode.Decompiler.IL.Transforms /// Transforms array initialization pattern of System.Runtime.CompilerServices.RuntimeHelpers.InitializeArray. /// For collection and object initializers see /// - public class TransformArrayInitializers : IILTransform + public class TransformArrayInitializers : IBlockTransform { - ILTransformContext context; - ILFunction function; + BlockTransformContext context; - void IILTransform.Run(ILFunction function, ILTransformContext context) + void IBlockTransform.Run(Block block, BlockTransformContext context) { this.context = context; - this.function = function; - foreach (var block in function.Descendants.OfType()) { - for (int i = block.Instructions.Count - 1; i >= 0; i--) { - if (!DoTransform(block, i)) - DoTransformMultiDim(block, i); - } + for (int i = block.Instructions.Count - 1; i >= 0; i--) { + if (!DoTransform(block, i)) + DoTransformMultiDim(block, i); } } @@ -58,7 +54,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms ILInstruction[] values; int initArrayPos; if (ForwardScanInitializeArrayRuntimeHelper(body, pos + 1, v, elementType, arrayLength, out values, out initArrayPos)) { - var tempStore = function.RegisterVariable(VariableKind.StackSlot, v.Type); + var tempStore = context.Function.RegisterVariable(VariableKind.StackSlot, v.Type); var block = BlockFromInitializer(tempStore, elementType, arrayLength, values); body.Instructions[pos].ReplaceWith(new StLoc(v, block)); body.Instructions.RemoveAt(initArrayPos); @@ -70,7 +66,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms int instructionsToRemove; if (HandleSimpleArrayInitializer(body, pos + 1, v, arrayLength[0], out finalStore, out values, out instructionsToRemove)) { var block = new Block(BlockType.ArrayInitializer); - var tempStore = function.RegisterVariable(VariableKind.StackSlot, v.Type); + var tempStore = context.Function.RegisterVariable(VariableKind.StackSlot, v.Type); block.Instructions.Add(new StLoc(tempStore, new NewArr(elementType, arrayLength.Select(l => new LdcI4(l)).ToArray()))); block.Instructions.AddRange(values.SelectWithIndex( (i, value) => { @@ -88,7 +84,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms } if (HandleJaggedArrayInitializer(body, pos + 1, v, arrayLength[0], out finalStore, out values, out instructionsToRemove)) { var block = new Block(BlockType.ArrayInitializer); - var tempStore = function.RegisterVariable(VariableKind.StackSlot, v.Type); + var tempStore = context.Function.RegisterVariable(VariableKind.StackSlot, v.Type); block.Instructions.Add(new StLoc(tempStore, new NewArr(elementType, arrayLength.Select(l => new LdcI4(l)).ToArray()))); block.Instructions.AddRange(values.SelectWithIndex((i, value) => StElem(new LdLoc(tempStore), new[] { new LdcI4(i) }, value, elementType))); block.FinalInstruction = new LdLoc(tempStore); diff --git a/ICSharpCode.Decompiler/IL/Transforms/TransformAssignment.cs b/ICSharpCode.Decompiler/IL/Transforms/TransformAssignment.cs index 53261e270..f602bc4c7 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/TransformAssignment.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/TransformAssignment.cs @@ -27,32 +27,30 @@ namespace ICSharpCode.Decompiler.IL.Transforms /// /// Description of TransformAssignment. /// - public class TransformAssignment : IILTransform + public class TransformAssignment : IBlockTransform { - ILTransformContext context; + BlockTransformContext context; - void IILTransform.Run(ILFunction function, ILTransformContext context) + void IBlockTransform.Run(Block block, BlockTransformContext context) { this.context = context; - foreach (var block in function.Descendants.OfType()) { - for (int i = block.Instructions.Count - 1; i >= 0; i--) { - if (TransformPostIncDecOperatorOnAddress(block, i) || TransformPostIncDecOnStaticField(block, i) || TransformCSharp4PostIncDecOperatorOnAddress(block, i)) { - block.Instructions.RemoveAt(i); - continue; - } - if (InlineLdAddressUsages(block, i)) { - block.Instructions.RemoveAt(i); - continue; - } - if (TransformPostIncDecOperator(block, i, function)) { - block.Instructions.RemoveAt(i); - continue; - } - if (TransformInlineAssignmentStObj(block, i)) - continue; - if (TransformInlineAssignmentCall(block, i)) - continue; + for (int i = block.Instructions.Count - 1; i >= 0; i--) { + if (TransformPostIncDecOperatorOnAddress(block, i) || TransformPostIncDecOnStaticField(block, i) || TransformCSharp4PostIncDecOperatorOnAddress(block, i)) { + block.Instructions.RemoveAt(i); + continue; } + if (InlineLdAddressUsages(block, i)) { + block.Instructions.RemoveAt(i); + continue; + } + if (TransformPostIncDecOperator(block, i)) { + block.Instructions.RemoveAt(i); + continue; + } + if (TransformInlineAssignmentStObj(block, i)) + continue; + if (TransformInlineAssignmentCall(block, i)) + continue; } } @@ -219,7 +217,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms /// final: ldloc s2 /// }) /// - static bool TransformPostIncDecOperator(Block block, int i, ILFunction function) + bool TransformPostIncDecOperator(Block block, int i) { var inst = block.Instructions[i] as StLoc; var nextInst = block.Instructions.ElementAtOrDefault(i + 1) as StLoc; @@ -230,7 +228,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms return false; if ((binary.Operator != BinaryNumericOperator.Add && binary.Operator != BinaryNumericOperator.Sub) || !binary.Left.MatchLdLoc(inst.Variable) || !binary.Right.MatchLdcI4(1)) return false; - var tempStore = function.RegisterVariable(VariableKind.StackSlot, inst.Variable.Type); + var tempStore = context.Function.RegisterVariable(VariableKind.StackSlot, inst.Variable.Type); var assignment = new Block(BlockType.CompoundOperator); assignment.Instructions.Add(new StLoc(tempStore, new LdLoc(nextInst.Variable))); assignment.Instructions.Add(new StLoc(nextInst.Variable, new BinaryNumericInstruction(binary.Operator, new LdLoc(tempStore), new LdcI4(1), binary.CheckForOverflow, binary.Sign))); diff --git a/ICSharpCode.Decompiler/Properties/AssemblyInfo.template.cs b/ICSharpCode.Decompiler/Properties/AssemblyInfo.template.cs index b89ce1b86..5a7a67424 100644 --- a/ICSharpCode.Decompiler/Properties/AssemblyInfo.template.cs +++ b/ICSharpCode.Decompiler/Properties/AssemblyInfo.template.cs @@ -30,6 +30,3 @@ using System.Diagnostics.CodeAnalysis; [assembly: SuppressMessage("Microsoft.Usage", "CA2243:AttributeStringLiteralsShouldParseCorrectly", Justification = "AssemblyInformationalVersion does not need to be a parsable version")] -[assembly: SuppressMessage("Language Usage Opportunities", "RECS0091:Use 'var' keyword when possible")] -[assembly: SuppressMessage("Redundancies in Code", "RECS0113:Redundant comma in array initializer")] - diff --git a/ICSharpCode.Decompiler/Tests/RoundtripAssembly.cs b/ICSharpCode.Decompiler/Tests/RoundtripAssembly.cs index c5a5c4715..9bcbceef1 100644 --- a/ICSharpCode.Decompiler/Tests/RoundtripAssembly.cs +++ b/ICSharpCode.Decompiler/Tests/RoundtripAssembly.cs @@ -44,11 +44,7 @@ namespace ICSharpCode.Decompiler.Tests [Test] public void NewtonsoftJson_net40() { - try { - RunWithTest("Newtonsoft.Json-net40", "Newtonsoft.Json.dll", "Newtonsoft.Json.Tests.dll"); - } catch (CompilationFailedException) { - Assert.Ignore("Known bug in lambda decompilation"); - } + RunWithTest("Newtonsoft.Json-net40", "Newtonsoft.Json.dll", "Newtonsoft.Json.Tests.dll"); } [Test] diff --git a/ILSpy/ILSpy.csproj b/ILSpy/ILSpy.csproj index f79466f5a..a084a5d3c 100644 --- a/ILSpy/ILSpy.csproj +++ b/ILSpy/ILSpy.csproj @@ -58,6 +58,7 @@ false false + ..\ICSharpCode.Decompiler\ICSharpCode.Decompiler.ruleset false diff --git a/ILSpy/Properties/AssemblyInfo.template.cs b/ILSpy/Properties/AssemblyInfo.template.cs index 4e6843825..b40fac15c 100644 --- a/ILSpy/Properties/AssemblyInfo.template.cs +++ b/ILSpy/Properties/AssemblyInfo.template.cs @@ -33,8 +33,6 @@ using System.Diagnostics.CodeAnalysis; [assembly: SuppressMessage("Microsoft.Usage", "CA2243:AttributeStringLiteralsShouldParseCorrectly", Justification = "AssemblyInformationalVersion does not need to be a parsable version")] -[assembly: SuppressMessage("Language Usage Opportunities", "RECS0091:Use 'var' keyword when possible")] -[assembly: SuppressMessage("Redundancies in Code", "RECS0113:Redundant comma in array initializer")] [assembly: SuppressMessage("Redundancies in Symbol Declarations", "RECS0122:Initializing field with default value is redundant", Justification = "Explicit default initialization is necessary to avoid compiler warning with MEF")]