Merge branch 'copy-propagation'

ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj

@@ -61,6 +61,9 @@
     <None Include="TestCases\Correctness\StackTypes.il" />
     <None Include="TestCases\Correctness\Uninit.vb" />
     <None Include="TestCases\ILPretty\ConstantBlobs.il" />
+    <None Include="TestCases\ILPretty\CS1xSwitch_Debug.il" />
+    <None Include="TestCases\ILPretty\CS1xSwitch_Release.il" />
+    <None Include="TestCases\ILPretty\Issue1157.il" />
     <None Include="TestCases\ILPretty\Issue684.cs" />
     <None Include="TestCases\ILPretty\Issue684.il" />
     <None Include="TestCases\ILPretty\FSharpLoops.fs" />
@@ -78,6 +81,11 @@
     <Compile Include="DisassemblerPrettyTestRunner.cs" />
     <Compile Include="TestCases\ILPretty\ConstantBlobs.cs" />
     <Compile Include="TestCases\Pretty\OutVariables.cs" />
+    <None Include="TestCases\ILPretty\Issue1145.cs" />
+    <Compile Include="TestCases\ILPretty\Issue1157.cs" />
+    <None Include="TestCases\ILPretty\Unsafe.cs" />
+    <None Include="TestCases\ILPretty\CS1xSwitch_Debug.cs" />
+    <None Include="TestCases\ILPretty\CS1xSwitch_Release.cs" />
     <None Include="TestCases\ILPretty\DirectCallToExplicitInterfaceImpl.cs" />
     <Compile Include="TestCases\ILPretty\Issue1389.cs" />
     <Compile Include="TestCases\ILPretty\Issue1454.cs" />

ICSharpCode.Decompiler.Tests/TestCases/Pretty/InitializerTests.cs

@@ -1678,13 +1678,9 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty.InitializerTests
 			});
 		}
 
-		private void NestedListWithIndexInitializer(MyEnum myEnum)
+		private List<List<int>> NestedListWithIndexInitializer(MyEnum myEnum)
 		{
-#if !OPT
-			List<List<int>> list = new List<List<int>> {
-#else
-			List<List<int>> obj = new List<List<int>> {
-#endif
+			return new List<List<int>> {
 				[0] = {
 					1,
 					2,
@@ -1717,13 +1713,9 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty.InitializerTests
 			});
 		}
 
-		public static void Issue1390(IEnumerable<string> tokens, bool alwaysAllowAdministrators, char wireDelimiter)
+		public static List<KeyValuePair<string, string>> Issue1390(IEnumerable<string> tokens, bool alwaysAllowAdministrators, char wireDelimiter)
 		{
-#if OPT
-			List<KeyValuePair<string, string>> obj = new List<KeyValuePair<string, string>> {
-#else
-			List<KeyValuePair<string, string>> list = new List<KeyValuePair<string, string>> {
-#endif
+			return new List<KeyValuePair<string, string>> {
 			{
 				"tokens",
 					string.Join(wireDelimiter.ToString(), tokens),

ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs

@@ -131,7 +131,6 @@ namespace ICSharpCode.Decompiler.CSharp
 						// copy-propated (turned into two separate assignments of the constant).
 						// After is necessary because the assigned value might involve null coalescing/etc.
 						new StatementTransform(new ILInlining(), new TransformAssignment()),
-						new CopyPropagation(),
 						new StatementTransform(
 							// per-block transforms that depend on each other, and thus need to
 							// run interleaved (statement by statement).
@@ -156,6 +155,7 @@ namespace ICSharpCode.Decompiler.CSharp
 				},
 				new ProxyCallReplacer(),
 				new FixRemainingIncrements(),
+				new CopyPropagation(),
 				new DelegateConstruction(),
 				new LocalFunctionDecompiler(),
 				new TransformDisplayClassUsage(),

ICSharpCode.Decompiler/IL/ControlFlow/ControlFlowSimplification.cs

@@ -15,6 +15,7 @@
 // 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.Generic;
 using System.Diagnostics;
 using System.Linq;
@@ -45,6 +46,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 				context.CancellationToken.ThrowIfCancellationRequested();
 
 				RemoveNopInstructions(block);
+				RemoveDeadStackStores(block, aggressive: context.Settings.RemoveDeadCode);
 
 				InlineVariableInReturnBlock(block, context);
 				// 1st pass SimplifySwitchInstruction before SimplifyBranchChains()
@@ -68,6 +70,35 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 			block.Instructions.RemoveAll(inst => inst.OpCode == OpCode.Nop);
 		}
 
+		private void RemoveDeadStackStores(Block block, bool aggressive)
+		{
+			// Previously copy propagation did this;
+			// ideally the ILReader would already do this,
+			// for now do this here (even though it's not control-flow related).
+			for (int i = block.Instructions.Count - 1; i >= 0; i--) {
+				if (block.Instructions[i] is StLoc stloc && stloc.Variable.IsSingleDefinition && stloc.Variable.LoadCount == 0 && stloc.Variable.Kind == VariableKind.StackSlot) {
+					if (aggressive ? SemanticHelper.IsPure(stloc.Value.Flags) : IsSimple(stloc.Value)) {
+						Debug.Assert(SemanticHelper.IsPure(stloc.Value.Flags));
+						block.Instructions.RemoveAt(i++);
+					} else {
+						stloc.Value.AddILRange(stloc);
+						stloc.ReplaceWith(stloc.Value);
+					}
+				}
+			}
+
+			bool IsSimple(ILInstruction inst)
+			{
+				switch (inst.OpCode) {
+					case OpCode.LdLoc:
+					case OpCode.LdStr: // C# 1.0 compiler sometimes emits redundant ldstr in switch-on-string pattern
+						return true;
+					default:
+						return false;
+				}
+			}
+		}
+
 		void InlineVariableInReturnBlock(Block block, ILTransformContext context)
 		{
 			// In debug mode, the C#-compiler generates 'return blocks' that

ICSharpCode.Decompiler/IL/ControlFlow/DetectPinnedRegions.cs

@@ -504,26 +504,23 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 			// stloc nativeVar(conv o->i (ldloc pinnedVar))
 			// if (comp(ldloc nativeVar == conv i4->i <sign extend>(ldc.i4 0))) br targetBlock
 			// br adjustOffsetToStringData
-			if (!body.EntryPoint.Instructions[0].MatchStLoc(out ILVariable nativeVar, out ILInstruction initInst))
-				return;
 			ILVariable newVar;
-			if (body.EntryPoint.Instructions.Count != 3) {
+			if (!body.EntryPoint.Instructions[0].MatchStLoc(out ILVariable nativeVar, out ILInstruction initInst)) {
 				// potentially a special case with legacy csc and an unused pinned variable:
-				if (nativeVar.IsSingleDefinition && nativeVar.LoadCount == 0 && initInst.MatchLdLoc(pinnedRegion.Variable)
-					 && pinnedRegion.Variable.LoadCount == 1)
-				{
-					// initInst is dead store
-					body.EntryPoint.Instructions.RemoveAt(0);
+				if (pinnedRegion.Variable.AddressCount == 0 && pinnedRegion.Variable.LoadCount == 0) {
 					var charPtr = new PointerType(context.TypeSystem.FindType(KnownTypeCode.Char));
 					newVar = new ILVariable(VariableKind.PinnedLocal, charPtr, pinnedRegion.Variable.Index);
 					newVar.Name = pinnedRegion.Variable.Name;
 					newVar.HasGeneratedName = pinnedRegion.Variable.HasGeneratedName;
-					nativeVar.Function.Variables.Add(newVar);
+					pinnedRegion.Variable.Function.Variables.Add(newVar);
 					pinnedRegion.Variable = newVar;
 					pinnedRegion.Init = new ArrayToPointer(pinnedRegion.Init);
 				}
 				return;
 			}
+			if (body.EntryPoint.Instructions.Count != 3) {
+				return;
+			}
 
 			if (nativeVar.Type.GetStackType() != StackType.I)
 				return;

ICSharpCode.Decompiler/IL/Transforms/CopyPropagation.cs

@@ -31,7 +31,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 	///    then we can replace the variable with the argument.
 	/// 2) assignments of address-loading instructions to local variables
 	/// </summary>
-	public class CopyPropagation : IBlockTransform
+	public class CopyPropagation : IBlockTransform, IILTransform
 	{
 		public static void Propagate(StLoc store, ILTransformContext context)
 		{
@@ -42,6 +42,20 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 		}
 
 		public void Run(Block block, BlockTransformContext context)
+		{
+			RunOnBlock(block, context);
+		}
+
+		public void Run(ILFunction function, ILTransformContext context)
+		{
+			foreach (var block in function.Descendants.OfType<Block>()) {
+				if (block.Kind != BlockKind.ControlFlow)
+					continue;
+				RunOnBlock(block, context);
+			}
+		}
+
+		static void RunOnBlock(Block block, ILTransformContext context)
 		{
 			for (int i = 0; i < block.Instructions.Count; i++) {
 				ILVariable v;
@@ -52,7 +66,10 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 						if (SemanticHelper.IsPure(copiedExpr.Flags)) {
 							// no-op -> delete
 							context.Step("remove dead store to stack: no-op -> delete", block.Instructions[i]);
-							block.Instructions.RemoveAt(i--);
+							block.Instructions.RemoveAt(i);
+							// This can open up new inlining opportunities:
+							int c = ILInlining.InlineInto(block, i, InliningOptions.None, context: context);
+							i -= c + 1;
 						} else {
 							// evaluate the value for its side-effects
 							context.Step("remove dead store to stack: evaluate the value for its side-effects", block.Instructions[i]);

ICSharpCode.Decompiler/IL/Transforms/DynamicIsEventAssignmentTransform.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.Linq;
+
 namespace ICSharpCode.Decompiler.IL.Transforms
 {
 	public class DynamicIsEventAssignmentTransform : IStatementTransform
@@ -24,6 +26,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 		/// if (logic.not(ldloc V_1)) Block IL_004a {
 		/// 	stloc V_2(dynamic.getmember B(target))
 		/// }
+		/// [stloc copyOfValue(value)]
 		/// if (logic.not(ldloc V_1)) Block IL_0149 {
 		/// 	dynamic.setmember.compound B(target, dynamic.binary.operator AddAssign(ldloc V_2,  value))
 		/// } else Block IL_0151 {
@@ -41,11 +44,17 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 				return;
 			if (!(flagVar.IsSingleDefinition && flagVar.LoadCount == 2))
 				return;
-			if (!(MatchLhsCacheIfInstruction(block.Instructions[pos + 1], flagVar, out var dynamicGetMemberStore)))
+			if (!MatchLhsCacheIfInstruction(block.Instructions[pos + 1], flagVar, out var dynamicGetMemberStore))
 				return;
 			if (!(dynamicGetMemberStore.MatchStLoc(out var getMemberVar, out inst) && inst is DynamicGetMemberInstruction getMemberInst))
 				return;
-			foreach (var descendant in block.Instructions[pos + 2].Descendants) {
+			int offset = 2;
+			if (block.Instructions[pos + offset].MatchStLoc(out var valueVariable)
+				&& pos + 4 < block.Instructions.Count && valueVariable.IsSingleDefinition && valueVariable.LoadCount == 2
+				&& valueVariable.LoadInstructions.All(ld => ld.Parent is DynamicInstruction)) {
+				offset++;
+			}
+			foreach (var descendant in block.Instructions[pos + offset].Descendants) {
 				if (!MatchIsEventAssignmentIfInstruction(descendant, isEvent, flagVar, getMemberVar, out var setMemberInst, out var getMemberVarUse, out var isEventConditionUse))
 					continue;
 				context.Step("DynamicIsEventAssignmentTransform", block.Instructions[pos]);
@@ -87,7 +96,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 			} else
 				return false;
 			setMemberInst = Block.Unwrap(trueInst) as DynamicSetMemberInstruction;
-			if (!(setMemberInst != null))
+			if (setMemberInst == null)
 				return false;
 			if (!isEvent.Argument.Match(setMemberInst.Target).Success)
 				return false;

ICSharpCode.Decompiler/IL/Transforms/ExpressionTransforms.cs

@@ -582,7 +582,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 			for (int j = 0; j < dynamicGetIndex.Arguments.Count; j++) {
 				if (!SemanticHelper.IsPure(dynamicGetIndex.Arguments[j].Flags))
 					return;
-				if (!dynamicGetIndex.Arguments[j].Match(dynamicGetIndex.Arguments[j]).Success)
+				if (!dynamicGetIndex.Arguments[j].Match(inst.Arguments[j]).Success)
 					return;
 			}
 			if (!DynamicCompoundAssign.IsExpressionTypeSupported(binaryOp.Operation))

ICSharpCode.Decompiler/IL/Transforms/ILInlining.cs

@@ -283,6 +283,8 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 					return !method.IsStatic;
 				case OpCode.Await:
 					return true;
+				case OpCode.NullableUnwrap:
+					return ((NullableUnwrap)ldloca.Parent).RefInput;
 				default:
 					return false;
 			}

ICSharpCode.Decompiler/IL/Transforms/NullPropagationTransform.cs

@@ -17,22 +17,15 @@
 // DEALINGS IN THE SOFTWARE.
 
 using System;
-using System.Collections.Generic;
-using System.Collections.Immutable;
 using System.Diagnostics;
-using System.Linq;
-using System.Text;
-using ICSharpCode.Decompiler.CSharp.Syntax;
-using ICSharpCode.Decompiler.Semantics;
 using ICSharpCode.Decompiler.TypeSystem;
-using ICSharpCode.Decompiler.Util;
 
 namespace ICSharpCode.Decompiler.IL.Transforms
 {
 	/// <summary>
 	/// Transform that converts code patterns like "v != null ? v.M() : null" to "v?.M()"
 	/// </summary>
-	struct NullPropagationTransform
+	readonly struct NullPropagationTransform
 	{
 		internal static bool IsProtectedIfInst(IfInstruction ifInst)
 		{
@@ -214,6 +207,9 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 					inst = ldLen.Array;
 				} else if (inst is NullableUnwrap unwrap) {
 					inst = unwrap.Argument;
+					if (unwrap.RefInput && inst is AddressOf addressOf) {
+						inst = addressOf.Value;
+					}
 				} else if (inst is DynamicGetMemberInstruction dynGetMember) {
 					inst = dynGetMember.Target;
 				} else if (inst is DynamicInvokeMemberInstruction dynInvokeMember) {

ICSharpCode.Decompiler/IL/Transforms/TransformAssignment.cs

@@ -547,10 +547,14 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 		/// Some transforms effectively move a store around,
 		/// which is only valid if the variable stored to does not occur in the compound load/store.
 		/// </param>
+		/// <param name="previousInstruction">
+		/// Instruction preceding the load.
+		/// </param>
 		static bool IsMatchingCompoundLoad(ILInstruction load, ILInstruction store,
 			out ILInstruction target, out CompoundTargetKind targetKind,
 			out Action<ILTransformContext> finalizeMatch,
-			ILVariable forbiddenVariable = null)
+			ILVariable forbiddenVariable = null,
+			ILInstruction previousInstruction = null)
 		{
 			target = null;
 			targetKind = 0;
@@ -563,7 +567,16 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 					return false;
 				target = ldobj.Target;
 				targetKind = CompoundTargetKind.Address;
-				return ldobj.Target.Match(stobj.Target).Success;
+				if (ldobj.Target.Match(stobj.Target).Success) {
+					return true;
+				} else if (IsDuplicatedAddressComputation(stobj.Target, ldobj.Target)) {
+					// Use S_0 as target, so that S_0 can later be eliminated by inlining.
+					// (we can't eliminate previousInstruction right now, because it's before the transform's starting instruction)
+					target = stobj.Target;
+					return true;
+				} else {
+					return false;
+				}
 			} else if (MatchingGetterAndSetterCalls(load as CallInstruction, store as CallInstruction, out finalizeMatch)) {
 				if (forbiddenVariable != null && forbiddenVariable.IsUsedWithin(load))
 					return false;
@@ -580,6 +593,27 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 			} else {
 				return false;
 			}
+
+			bool IsDuplicatedAddressComputation(ILInstruction storeTarget, ILInstruction loadTarget)
+			{
+				// Sometimes roslyn duplicates the address calculation:
+				// stloc S_0(ldloc refParam)
+				// stloc V_0(ldobj System.Int32(ldloc refParam))
+				// stobj System.Int32(ldloc S_0, binary.add.i4(ldloc V_0, ldc.i4 1))
+				while (storeTarget is LdFlda storeLdFlda && loadTarget is LdFlda loadLdFlda) {
+					if (!storeLdFlda.Field.Equals(loadLdFlda.Field))
+						return false;
+					storeTarget = storeLdFlda.Target;
+					loadTarget = loadLdFlda.Target;
+				}
+				if (!storeTarget.MatchLdLoc(out var s))
+					return false;
+				if (!(s.Kind == VariableKind.StackSlot && s.IsSingleDefinition && s != forbiddenVariable))
+					return false;
+				if (s.StoreInstructions.SingleOrDefault() != previousInstruction)
+					return false;
+				return previousInstruction is StLoc addressStore && addressStore.Value.Match(loadTarget).Success;
+			}
 		}
 
 		/// <code>
@@ -677,8 +711,11 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 				// 'stloc tmp' is implicitly truncating the value
 				return false;
 			}
-			if (!IsMatchingCompoundLoad(inst.Value, store, out var target, out var targetKind, out var finalizeMatch, forbiddenVariable: inst.Variable))
+			if (!IsMatchingCompoundLoad(inst.Value, store, out var target, out var targetKind, out var finalizeMatch,
+				forbiddenVariable: inst.Variable,
+				previousInstruction: block.Instructions.ElementAtOrDefault(i - 1))) {
 				return false;
+			}
 			if (UnwrapSmallIntegerConv(value, out var conv) is BinaryNumericInstruction binary) {
 				if (!binary.Left.MatchLdLoc(tmpVar) || !binary.Right.MatchLdcI(1))
 					return false;