From dbbcbb87fe8604e2741ee9e23987de59c37229db Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Sun, 5 Sep 2021 18:33:30 +0200 Subject: [PATCH] Make pattern matching transform a simple ILTransform for both reference and value types. Check that the true branch dominates all uses of the pattern variable. --- .../TestCases/Pretty/PatternMatching.cs | 13 + .../CSharp/CSharpDecompiler.cs | 1 - .../ICSharpCode.Decompiler.csproj | 1 - .../Transforms/EarlyExpressionTransforms.cs | 54 ++++ .../IL/Transforms/ExpressionTransforms.cs | 41 +-- .../PatternMatchingRefTypesTransform.cs | 128 -------- .../IL/Transforms/PatternMatchingTransform.cs | 306 ++++++++++++++---- 7 files changed, 319 insertions(+), 225 deletions(-) delete mode 100644 ICSharpCode.Decompiler/IL/Transforms/PatternMatchingRefTypesTransform.cs diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/PatternMatching.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/PatternMatching.cs index efeadf557..173e038bd 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/PatternMatching.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/PatternMatching.cs @@ -194,6 +194,19 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty Use(F() && GetObject() is int num && num.GetHashCode() > 0 && num % 2 == 0); } + public static void NotTypePatternVariableUsedOutsideTrueBranch(object x) + { + var text = x as string; + if (text != null && text.Length > 5) + { + Console.WriteLine("pattern matches"); + } + if (text != null && text.Length > 10) + { + Console.WriteLine("other use!"); + } + } + private bool F() { return true; diff --git a/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs b/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs index cfa783999..f43be94ff 100644 --- a/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs +++ b/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs @@ -138,7 +138,6 @@ namespace ICSharpCode.Decompiler.CSharp // Inlining must be first, because it doesn't trigger re-runs. // Any other transform that opens up new inlining opportunities should call RequestRerun(). new ExpressionTransforms(), - new PatternMatchingRefTypesTransform(), new DynamicIsEventAssignmentTransform(), new TransformAssignment(), // inline and compound assignments new NullCoalescingTransform(), diff --git a/ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj b/ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj index 638020a9a..962f6795a 100644 --- a/ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj +++ b/ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj @@ -93,7 +93,6 @@ - diff --git a/ICSharpCode.Decompiler/IL/Transforms/EarlyExpressionTransforms.cs b/ICSharpCode.Decompiler/IL/Transforms/EarlyExpressionTransforms.cs index a4fb74a3b..b0b4dfd02 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/EarlyExpressionTransforms.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/EarlyExpressionTransforms.cs @@ -40,6 +40,60 @@ namespace ICSharpCode.Decompiler.IL.Transforms } } + protected internal override void VisitComp(Comp inst) + { + base.VisitComp(inst); + FixComparisonKindLdNull(inst, context); + } + + internal static void FixComparisonKindLdNull(Comp inst, ILTransformContext context) + { + if (inst.IsLifted) + { + return; + } + if (inst.Right.MatchLdNull()) + { + if (inst.Kind == ComparisonKind.GreaterThan) + { + context.Step("comp(left > ldnull) => comp(left != ldnull)", inst); + inst.Kind = ComparisonKind.Inequality; + } + else if (inst.Kind == ComparisonKind.LessThanOrEqual) + { + context.Step("comp(left <= ldnull) => comp(left == ldnull)", inst); + inst.Kind = ComparisonKind.Equality; + } + } + else if (inst.Left.MatchLdNull()) + { + if (inst.Kind == ComparisonKind.LessThan) + { + context.Step("comp(ldnull < right) => comp(ldnull != right)", inst); + inst.Kind = ComparisonKind.Inequality; + } + else if (inst.Kind == ComparisonKind.GreaterThanOrEqual) + { + context.Step("comp(ldnull >= right) => comp(ldnull == right)", inst); + inst.Kind = ComparisonKind.Equality; + } + } + + if (inst.Right.MatchLdNull() && inst.Left.MatchBox(out var arg, out var type) && type.Kind == TypeKind.TypeParameter) + { + if (inst.Kind == ComparisonKind.Equality) + { + context.Step("comp(box T(..) == ldnull) -> comp(.. == ldnull)", inst); + inst.Left = arg; + } + if (inst.Kind == ComparisonKind.Inequality) + { + context.Step("comp(box T(..) != ldnull) -> comp(.. != ldnull)", inst); + inst.Left = arg; + } + } + } + protected internal override void VisitStObj(StObj inst) { base.VisitStObj(inst); diff --git a/ICSharpCode.Decompiler/IL/Transforms/ExpressionTransforms.cs b/ICSharpCode.Decompiler/IL/Transforms/ExpressionTransforms.cs index 56106209b..f9fa9a7b8 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/ExpressionTransforms.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/ExpressionTransforms.cs @@ -113,32 +113,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms { return; } - if (inst.Right.MatchLdNull()) - { - if (inst.Kind == ComparisonKind.GreaterThan) - { - context.Step("comp(left > ldnull) => comp(left != ldnull)", inst); - inst.Kind = ComparisonKind.Inequality; - } - else if (inst.Kind == ComparisonKind.LessThanOrEqual) - { - context.Step("comp(left <= ldnull) => comp(left == ldnull)", inst); - inst.Kind = ComparisonKind.Equality; - } - } - else if (inst.Left.MatchLdNull()) - { - if (inst.Kind == ComparisonKind.LessThan) - { - context.Step("comp(ldnull < right) => comp(ldnull != right)", inst); - inst.Kind = ComparisonKind.Inequality; - } - else if (inst.Kind == ComparisonKind.GreaterThanOrEqual) - { - context.Step("comp(ldnull >= right) => comp(ldnull == right)", inst); - inst.Kind = ComparisonKind.Equality; - } - } + EarlyExpressionTransforms.FixComparisonKindLdNull(inst, context); var rightWithoutConv = inst.Right.UnwrapConv(ConversionKind.SignExtend).UnwrapConv(ConversionKind.ZeroExtend); if (rightWithoutConv.MatchLdcI4(0) @@ -183,20 +158,6 @@ namespace ICSharpCode.Decompiler.IL.Transforms inst.Right.AddILRange(rightWithoutConv); } } - - if (inst.Right.MatchLdNull() && inst.Left.MatchBox(out arg, out var type) && type.Kind == TypeKind.TypeParameter) - { - if (inst.Kind == ComparisonKind.Equality) - { - context.Step("comp(box T(..) == ldnull) -> comp(.. == ldnull)", inst); - inst.Left = arg; - } - if (inst.Kind == ComparisonKind.Inequality) - { - context.Step("comp(box T(..) != ldnull) -> comp(.. != ldnull)", inst); - inst.Left = arg; - } - } } protected internal override void VisitConv(Conv inst) diff --git a/ICSharpCode.Decompiler/IL/Transforms/PatternMatchingRefTypesTransform.cs b/ICSharpCode.Decompiler/IL/Transforms/PatternMatchingRefTypesTransform.cs deleted file mode 100644 index d83f62bf4..000000000 --- a/ICSharpCode.Decompiler/IL/Transforms/PatternMatchingRefTypesTransform.cs +++ /dev/null @@ -1,128 +0,0 @@ -// Copyright (c) 2021 Daniel Grunwald, Siegfried Pammer -// -// 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. - -#nullable enable - -using ICSharpCode.Decompiler.TypeSystem; - -namespace ICSharpCode.Decompiler.IL.Transforms -{ - class PatternMatchingRefTypesTransform : IStatementTransform - { - /// - /// stloc V(isinst T(testedOperand)) - /// call Use(..., comp.o(ldloc V != ldnull)) - /// => - /// call Use(..., match.type[T].notnull(V = testedOperand)) - /// - /// - or - - /// - /// stloc S(isinst T(testedOperand)) - /// stloc V(ldloc S) - /// call Use(..., comp.o(ldloc S != ldnull)) - /// => - /// call Use(..., match.type[T].notnull(V = testedOperand)) - /// - void IStatementTransform.Run(Block block, int pos, StatementTransformContext context) - { - if (!context.Settings.PatternMatching) - return; - int startPos = pos; - if (!block.Instructions[pos].MatchStLoc(out var s, out var value)) - return; - IType? unboxType; - if (value is UnboxAny unboxAny) - { - // stloc S(unbox.any T(isinst T(testedOperand))) - unboxType = unboxAny.Type; - value = unboxAny.Argument; - } - else - { - unboxType = null; - } - if (value is not IsInst { Argument: var testedOperand, Type: var type }) - return; - if (type.IsReferenceType != true) - return; - if (!(unboxType == null || type.Equals(unboxType))) - return; - if (!s.IsSingleDefinition) - return; - if (s.Kind is not (VariableKind.Local or VariableKind.StackSlot)) - return; - pos++; - ILVariable v; - if (block.Instructions.ElementAtOrDefault(pos) is StLoc stloc && stloc.Value.MatchLdLoc(s)) - { - v = stloc.Variable; - pos++; - if (!v.IsSingleDefinition) - return; - if (v.Kind is not (VariableKind.Local or VariableKind.StackSlot)) - return; - if (s.LoadCount != 2) - return; - } - else - { - v = s; - } - - if (!v.Type.Equals(type)) - return; - if (pos >= block.Instructions.Count) - return; - var result = ILInlining.FindLoadInNext(block.Instructions[pos], s, testedOperand, InliningOptions.None); - if (result.Type != ILInlining.FindResultType.Found) - return; - if (result.LoadInst is not LdLoc) - return; - bool invertCondition; - if (result.LoadInst.Parent!.MatchCompNotEqualsNull(out _)) - { - invertCondition = false; - - } - else if (result.LoadInst.Parent!.MatchCompEqualsNull(out _)) - { - invertCondition = true; - } - else - { - return; - } - - context.Step($"Type pattern matching {v.Name}", block.Instructions[pos]); - // call Use(..., match.type[T](V = testedOperand)) - - var target = result.LoadInst.Parent; - ILInstruction matchInstruction = new MatchInstruction(v, testedOperand) { - CheckNotNull = true, - CheckType = true - }; - if (invertCondition) - { - matchInstruction = Comp.LogicNot(matchInstruction); - } - target.ReplaceWith(matchInstruction.WithILRange(target)); - block.Instructions.RemoveRange(startPos, pos - startPos); - v.Kind = VariableKind.PatternLocal; - } - } -} diff --git a/ICSharpCode.Decompiler/IL/Transforms/PatternMatchingTransform.cs b/ICSharpCode.Decompiler/IL/Transforms/PatternMatchingTransform.cs index 8f0c99fa4..ddaa25e80 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/PatternMatchingTransform.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/PatternMatchingTransform.cs @@ -18,16 +18,216 @@ #nullable enable +using System; +using System.ComponentModel; +using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Linq; +using System.Threading; +using System.Xml.Linq; +using ICSharpCode.Decompiler.CSharp.Resolver; +using ICSharpCode.Decompiler.IL.ControlFlow; using ICSharpCode.Decompiler.TypeSystem; using ICSharpCode.Decompiler.Util; +using static ICSharpCode.Decompiler.TypeSystem.ReflectionHelper; + namespace ICSharpCode.Decompiler.IL.Transforms { class PatternMatchingTransform : IILTransform { + + void IILTransform.Run(ILFunction function, ILTransformContext context) + { + if (!context.Settings.PatternMatching) + return; + foreach (var container in function.Descendants.OfType()) + { + ControlFlowGraph? cfg = null; + foreach (var block in container.Blocks) + { + if (PatternMatchValueTypes(block, container, context, ref cfg)) + { + continue; + } + if (PatternMatchRefTypes(block, container, context, ref cfg)) + { + continue; + } + } + } + } + + /// Block { + /// ... + /// stloc V(isinst T(testedOperand)) + /// if (comp.o(ldloc V == ldnull)) br falseBlock + /// br trueBlock + /// } + /// + /// All other uses of V are in blocks dominated by trueBlock. + /// => + /// Block { + /// ... + /// if (match.type[T].notnull(V = testedOperand)) br trueBlock + /// br falseBlock + /// } + /// + /// - or - + /// + /// Block { + /// stloc s(isinst T(testedOperand)) + /// stloc v(ldloc s) + /// if (logic.not(comp.o(ldloc s != ldnull))) br falseBlock + /// br trueBlock + /// } + /// => + /// Block { + /// ... + /// if (match.type[T].notnull(V = testedOperand)) br trueBlock + /// br falseBlock + /// } + /// + /// All other uses of V are in blocks dominated by trueBlock. + private bool PatternMatchRefTypes(Block block, BlockContainer container, ILTransformContext context, ref ControlFlowGraph? cfg) + { + if (!block.MatchIfAtEndOfBlock(out var condition, out var trueInst, out var falseInst)) + return false; + int pos = block.Instructions.Count - 3; + if (condition.MatchLdLoc(out var conditionVar)) + { + // stloc conditionVar(comp.o(ldloc s == ldnull)) + // if (logic.not(ldloc conditionVar)) br trueBlock + if (pos < 0) + return false; + if (!(conditionVar.IsSingleDefinition && conditionVar.LoadCount == 1 + && conditionVar.Kind == VariableKind.StackSlot)) + { + return false; + } + if (!block.Instructions[pos].MatchStLoc(conditionVar, out condition)) + return false; + pos--; + } + if (condition.MatchCompEqualsNull(out var arg)) + { + ExtensionMethods.Swap(ref trueInst, ref falseInst); + } + else if (condition.MatchCompNotEqualsNull(out arg)) + { + // do nothing + } + else + { + return false; + } + if (!arg.MatchLdLoc(out var s)) + return false; + if (!s.IsSingleDefinition) + return false; + if (s.Kind is not (VariableKind.Local or VariableKind.StackSlot)) + return false; + if (pos < 0) + return false; + // stloc V(isinst T(testedOperand)) + ILInstruction storeToV = block.Instructions[pos]; + if (!storeToV.MatchStLoc(out var v, out var value)) + return false; + if (value.MatchLdLoc(s)) + { + // stloc v(ldloc s) + pos--; + if (!block.Instructions[pos].MatchStLoc(s, out value)) + return false; + if (!v.IsSingleDefinition) + return false; + if (v.Kind is not (VariableKind.Local or VariableKind.StackSlot)) + return false; + if (s.LoadCount != 2) + return false; + } + else + { + if (v != s) + return false; + } + IType? unboxType; + if (value is UnboxAny unboxAny) + { + // stloc S(unbox.any T(isinst T(testedOperand))) + unboxType = unboxAny.Type; + value = unboxAny.Argument; + } + else + { + unboxType = null; + } + if (value is not IsInst { Argument: var testedOperand, Type: var type }) + return false; + if (type.IsReferenceType != true) + return false; + if (!(unboxType == null || type.Equals(unboxType))) + return false; + + if (!v.Type.Equals(type)) + return false; + if (!CheckAllUsesDominatedBy(v, container, trueInst, storeToV, context, ref cfg)) + return false; + context.Step($"Type pattern matching {v.Name}", block); + // if (match.type[T].notnull(V = testedOperand)) br trueBlock + + var ifInst = (IfInstruction)block.Instructions.SecondToLastOrDefault()!; + + ifInst.Condition = new MatchInstruction(v, testedOperand) { + CheckNotNull = true, + CheckType = true + }.WithILRange(ifInst.Condition); + ifInst.TrueInst = trueInst; + block.Instructions[block.Instructions.Count - 1] = falseInst; + block.Instructions.RemoveRange(pos, ifInst.ChildIndex - pos); + v.Kind = VariableKind.PatternLocal; + return true; + } + + private bool CheckAllUsesDominatedBy(ILVariable v, BlockContainer container, ILInstruction trueInst, + ILInstruction storeToV, ILTransformContext context, ref ControlFlowGraph? cfg) + { + var targetBlock = trueInst as Block; + if (targetBlock == null && !trueInst.MatchBranch(out targetBlock)) + { + return false; + } + + if (targetBlock.Parent != container) + return false; + cfg ??= new ControlFlowGraph(container, context.CancellationToken); + var targetBlockNode = cfg.GetNode(targetBlock); + Debug.Assert(v.StoreInstructions.Count == 1); + var uses = v.LoadInstructions.Concat(v.AddressInstructions) + .Concat(v.StoreInstructions.Cast()); + foreach (var use in uses) + { + if (use == storeToV) + continue; + Block? found = null; + for (ILInstruction? current = use; current != null; current = current.Parent) + { + if (current.Parent == container) + { + found = (Block)current; + break; + } + } + if (found == null) + return false; + var node = cfg.GetNode(found); + if (!targetBlockNode.Dominates(node)) + return false; + } + return true; + } + /// Block { /// ... /// [stloc temp(ldloc testedOperand)] @@ -45,61 +245,56 @@ namespace ICSharpCode.Decompiler.IL.Transforms /// if (match.type[T].notnull(V = testedOperand)) br unboxBlock /// br falseBlock /// } - void IILTransform.Run(ILFunction function, ILTransformContext context) + private bool PatternMatchValueTypes(Block block, BlockContainer container, ILTransformContext context, ref ControlFlowGraph? cfg) { - if (!context.Settings.PatternMatching) - return; - foreach (var container in function.Descendants.OfType()) + if (!MatchIsInstBlock(block, out var type, out var testedOperand, + out var unboxBlock, out var falseBlock)) { - foreach (var block in container.Blocks) - { - if (!MatchIsInstBlock(block, out var type, out var testedOperand, - out var unboxBlock, out var falseBlock)) - { - continue; - } - StLoc? tempStore = block.Instructions.ElementAtOrDefault(block.Instructions.Count - 3) as StLoc; - if (tempStore == null || !tempStore.Value.MatchLdLoc(testedOperand.Variable)) - { - tempStore = null; - } - if (!MatchUnboxBlock(unboxBlock, type, out var unboxOperand, out var v)) - { - continue; - } - if (unboxOperand == testedOperand.Variable) - { - // do nothing - } - else if (unboxOperand == tempStore?.Variable) - { - if (!(tempStore.Variable.IsSingleDefinition && tempStore.Variable.LoadCount == 1)) - continue; - } - else - { - continue; - } - context.Step($"PatternMatching with {v.Name}", block); - var ifInst = (IfInstruction)block.Instructions.SecondToLastOrDefault()!; - ifInst.Condition = new MatchInstruction(v, testedOperand) { - CheckNotNull = true, - CheckType = true - }; - ((Branch)ifInst.TrueInst).TargetBlock = unboxBlock; - ((Branch)block.Instructions.Last()).TargetBlock = falseBlock; - unboxBlock.Instructions.RemoveAt(0); - if (unboxOperand == tempStore?.Variable) - { - block.Instructions.Remove(tempStore); - } - // HACK: condition detection uses StartILOffset of blocks to decide which branch of if-else - // should become the then-branch. Change the unboxBlock StartILOffset from an offset inside - // the pattern matching machinery to an offset belonging to an instruction in the then-block. - unboxBlock.SetILRange(unboxBlock.Instructions[0]); - v.Kind = VariableKind.PatternLocal; - } + return false; + } + StLoc? tempStore = block.Instructions.ElementAtOrDefault(block.Instructions.Count - 3) as StLoc; + if (tempStore == null || !tempStore.Value.MatchLdLoc(testedOperand.Variable)) + { + tempStore = null; } + if (!MatchUnboxBlock(unboxBlock, type, out var unboxOperand, out var v, out var storeToV)) + { + return false; + } + if (unboxOperand == testedOperand.Variable) + { + // do nothing + } + else if (unboxOperand == tempStore?.Variable) + { + if (!(tempStore.Variable.IsSingleDefinition && tempStore.Variable.LoadCount == 1)) + return false; + } + else + { + return false; + } + if (!CheckAllUsesDominatedBy(v, container, unboxBlock, storeToV, context, ref cfg)) + return false; + context.Step($"PatternMatching with {v.Name}", block); + var ifInst = (IfInstruction)block.Instructions.SecondToLastOrDefault()!; + ifInst.Condition = new MatchInstruction(v, testedOperand) { + CheckNotNull = true, + CheckType = true + }; + ((Branch)ifInst.TrueInst).TargetBlock = unboxBlock; + ((Branch)block.Instructions.Last()).TargetBlock = falseBlock; + unboxBlock.Instructions.RemoveAt(0); + if (unboxOperand == tempStore?.Variable) + { + block.Instructions.Remove(tempStore); + } + // HACK: condition detection uses StartILOffset of blocks to decide which branch of if-else + // should become the then-branch. Change the unboxBlock StartILOffset from an offset inside + // the pattern matching machinery to an offset belonging to an instruction in the then-block. + unboxBlock.SetILRange(unboxBlock.Instructions[0]); + v.Kind = VariableKind.PatternLocal; + return true; } /// ... @@ -143,14 +338,15 @@ namespace ICSharpCode.Decompiler.IL.Transforms /// ... /// } private bool MatchUnboxBlock(Block unboxBlock, IType type, [NotNullWhen(true)] out ILVariable? testedOperand, - [NotNullWhen(true)] out ILVariable? v) + [NotNullWhen(true)] out ILVariable? v, [NotNullWhen(true)] out ILInstruction? storeToV) { v = null; + storeToV = null; testedOperand = null; if (unboxBlock.IncomingEdgeCount != 1) return false; - - if (!unboxBlock.Instructions[0].MatchStLoc(out v, out var value)) + storeToV = unboxBlock.Instructions[0]; + if (!storeToV.MatchStLoc(out v, out var value)) return false; if (!(value.MatchUnboxAny(out var arg, out var t) && t.Equals(type) && arg.MatchLdLoc(out testedOperand))) return false;