Improve ILInlining.IsSafeForInlineOver()

ICSharpCode.Decompiler/FlowAnalysis/ReachingDefinitionsVisitor.cs

@@ -310,7 +310,7 @@ namespace ICSharpCode.Decompiler.FlowAnalysis
 					storesByVar[vi] = new List<ILInstruction> { null };
 			}
 			foreach (var inst in scope.Descendants) {
-				if ((inst.DirectFlags & InstructionFlags.MayWriteLocals) != 0) {
+				if (inst.HasDirectFlag(InstructionFlags.MayWriteLocals)) {
 					ILVariable v = ((IInstructionWithVariableOperand)inst).Variable;
 					if (v.Scope == scope && activeVariables[v.IndexInScope]) {
 						storesByVar[v.IndexInScope].Add(inst);

ICSharpCode.Decompiler/IL/ILReader.cs

@@ -253,7 +253,7 @@ namespace ICSharpCode.Decompiler.IL
 				decodedInstruction.ILRange = new Interval(start, reader.Position);
 				UnpackPush(decodedInstruction).ILRange = decodedInstruction.ILRange;
 				instructionBuilder.Add(decodedInstruction);
-				if ((decodedInstruction.DirectFlags & InstructionFlags.EndPointUnreachable) != 0) {
+				if (decodedInstruction.HasDirectFlag(InstructionFlags.EndPointUnreachable)) {
 					if (!stackByOffset.TryGetValue(reader.Position, out currentStack)) {
 						currentStack = ImmutableStack<ILVariable>.Empty;
 					}

ICSharpCode.Decompiler/IL/InstructionFlags.cs

@@ -37,7 +37,7 @@ namespace ICSharpCode.Decompiler.IL
 		/// </summary>
 		/// <remarks>
 		/// This flag is not set for indirect writes to local variables through pointers.
-		/// Ensure you also check the SideEffect flag.
+		/// Ensure you also check the SideEffect flag when checking for instructions that might write to locals.
 		/// </remarks>
 		MayWriteLocals = 0x20,
 		/// <summary>

ICSharpCode.Decompiler/IL/Instructions/ILInstruction.cs

@@ -155,13 +155,21 @@ namespace ICSharpCode.Decompiler.IL
 		}
 
 		/// <summary>
-		/// Returns whether the instruction has at least one of the specified flags.
+		/// Returns whether the instruction (or one of its child instructions) has at least one of the specified flags.
 		/// </summary>
 		public bool HasFlag(InstructionFlags flags)
 		{
 			return (this.Flags & flags) != 0;
 		}
 		
+		/// <summary>
+		/// Returns whether the instruction (without considering child instructions) has at least one of the specified flags.
+		/// </summary>
+		public bool HasDirectFlag(InstructionFlags flags)
+		{
+			return (this.DirectFlags & flags) != 0;
+		}
+		
 		protected void InvalidateFlags()
 		{
 			for (ILInstruction inst = this; inst != null && inst.flags != invalidFlags; inst = inst.parent)

ICSharpCode.Decompiler/IL/SemanticHelper.cs

@@ -17,6 +17,7 @@
 // DEALINGS IN THE SOFTWARE.
 
 using System;
+using System.Linq;
 
 namespace ICSharpCode.Decompiler.IL
 {
@@ -63,5 +64,44 @@ namespace ICSharpCode.Decompiler.IL
 			return (inst1 & readFlag) != 0 && (inst2 & writeFlag) != 0
 				|| (inst2 & readFlag) != 0 && (inst1 & writeFlag) != 0;
 		}
+		
+		/// <summary>
+		/// Gets whether the instruction sequence 'inst1; inst2;' may be ordered to 'inst2; inst1;'
+		/// </summary>
+		internal static bool MayReorder(ILInstruction inst1, ILInstruction inst2)
+		{
+			// If both instructions perform an impure action, we cannot reorder them
+			if (!IsPure(inst1.Flags) && !IsPure(inst2.Flags))
+				return false;
+			// We cannot reorder if inst2 might write what inst1 looks at
+			if (Inst2MightWriteToVariableReadByInst1(inst1, inst2))
+				return false;
+			// and the same in reverse:
+			if (Inst2MightWriteToVariableReadByInst1(inst2, inst1))
+				return false;
+			return true;
+		}
+
+		static bool Inst2MightWriteToVariableReadByInst1(ILInstruction inst1, ILInstruction inst2)
+		{
+			if (!inst1.HasFlag(InstructionFlags.MayReadLocals)) {
+				// quick exit if inst1 doesn't read any variables
+				return false;
+			}
+			var variables = inst1.Descendants.OfType<LdLoc>().Select(load => load.Variable).ToHashSet();
+			if (inst2.HasFlag(InstructionFlags.SideEffect) && variables.Any(v => v.AddressCount > 0)) {
+				// If inst2 might have indirect writes, we cannot reorder with any loads of variables that have their address taken.
+				return true;
+			}
+			foreach (var inst in inst2.Descendants) {
+				if (inst.HasDirectFlag(InstructionFlags.MayWriteLocals)) {
+					ILVariable v = ((IInstructionWithVariableOperand)inst).Variable;
+					if (variables.Contains(v)) {
+						return true;
+					}
+				}
+			}
+			return false;
+		}
 	}
 }

ICSharpCode.Decompiler/IL/Transforms/ILInlining.cs

@@ -318,20 +318,6 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 		/// </summary>
 		static bool IsSafeForInlineOver(ILInstruction expr, ILInstruction expressionBeingMoved)
 		{
-			ILVariable v;
-			if (expr.MatchLdLoc(out v) && v.AddressCount == 0) {
-				// MayReorder() only looks at flags, so it doesn't
-				// allow reordering 'stloc x y; ldloc v' to 'ldloc v; stloc x y'
-				
-				// We'll allow the reordering unless x==v
-				if (expressionBeingMoved.HasFlag(InstructionFlags.MayWriteLocals)) {
-					foreach (var stloc in expressionBeingMoved.Descendants.OfType<StLoc>()) {
-						if (stloc.Variable == v)
-							return false;
-					}
-				}
-				return true;
-			}
 			return SemanticHelper.MayReorder(expressionBeingMoved.Flags, expr.Flags);
 		}
 	}