Refactor: remove redundant SPARC IndexedMemoryOperand

Keeping a single memory operand class simplifies the SPARC rewriter.
uxmal · Sep 18, 2024 · ebc4dc1 · ebc4dc1
1 parent 570ff17
commit ebc4dc1
Show file tree

Hide file tree

Showing 11 changed files with 70 additions and 66 deletions.
diff --git a/src/Arch/Sparc/MemoryOperand.cs b/src/Arch/Sparc/MemoryOperand.cs
@@ -24,48 +24,53 @@
 using Reko.Core.Types;
 using System;
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.Linq;
 using System.Text;
 
 namespace Reko.Arch.Sparc
 {
     public class MemoryOperand : AbstractMachineOperand
     {
-        public readonly RegisterStorage Base;
-        public readonly Constant Offset;
-
-        public MemoryOperand(RegisterStorage b, Constant offset, PrimitiveType width) : base(width)
+        private MemoryOperand(RegisterStorage b, Constant? offset, RegisterStorage? index, PrimitiveType width) : base(width)
         {
             this.Base = b;
             this.Offset = offset;
+            this.Index = index;
         }
 
-        protected override void DoRender(MachineInstructionRenderer renderer, MachineInstructionRendererOptions options)
+        public static MemoryOperand Indirect(RegisterStorage baseRg, Constant offset, PrimitiveType dt)
         {
-            renderer.WriteFormat("[%{0}", Base.Name);
-            if (!Offset.IsNegative)
-            {
-                renderer.WriteString("+");
-            }
-            renderer.WriteString(Offset.ToInt16().ToString());
-            renderer.WriteString("]");
+            return new MemoryOperand(baseRg, offset, null, dt);
         }
-    }
-
-    public class IndexedMemoryOperand : AbstractMachineOperand
-    {
-        public readonly RegisterStorage Base;
-        public readonly RegisterStorage Index;
 
-        public IndexedMemoryOperand(RegisterStorage r1, RegisterStorage r2, PrimitiveType width) : base(width)
+        public static MemoryOperand Indexed(RegisterStorage baseReg, RegisterStorage index, PrimitiveType dt)
         {
-            this.Base = r1;
-            this.Index = r2;
+            return new MemoryOperand(baseReg, null, index, dt);
         }
 
+        public RegisterStorage Base { get; }
+        public Constant? Offset { get; }
+        public RegisterStorage? Index { get; }
+
+
         protected override void DoRender(MachineInstructionRenderer renderer, MachineInstructionRendererOptions options)
         {
-            renderer.WriteFormat("[%{0}+%{1}]", Base, Index);
+            renderer.WriteFormat("[%{0}", Base.Name);
+            if (Offset is not null)
+            {
+                if (!Offset.IsNegative)
+                {
+                    renderer.WriteString("+");
+                }
+                renderer.WriteString(Offset.ToInt16().ToString());
+            }
+            else
+            {
+                Debug.Assert(Index is not null);
+                renderer.WriteFormat("+%{0}", Index);
+            }
+            renderer.WriteString("]");
         }
     }
 }
diff --git a/src/Arch/Sparc/SparcDisassembler.cs b/src/Arch/Sparc/SparcDisassembler.cs
@@ -366,20 +366,20 @@ private static MachineOperand GetAlternateSpaceOperand(Registers registers, uint
             RegisterStorage b = registers.GetRegister(wInstr >> 14);
             RegisterStorage idx = registers.GetRegister(wInstr);
             var asi = (wInstr >> 4) & 0xFF;
-            return new MemoryOperand(b, Constant.Int32((int) asi), type);
+            return MemoryOperand.Indirect(b, Constant.Int32((int) asi), type);
         }
 
         private static MachineOperand GetMemoryOperand(Registers registers, uint wInstr, PrimitiveType type)
         {
             RegisterStorage b = registers.GetRegister(wInstr >> 14);
             if ((wInstr & (1 << 13)) != 0)
             {
-                return new MemoryOperand(b, Constant.Int32(SignExtend(wInstr, 13)), type);
+                return MemoryOperand.Indirect(b, Constant.Int32(SignExtend(wInstr, 13)), type);
             }
             else
             {
                 RegisterStorage idx = registers.GetRegister(wInstr);
-                return new IndexedMemoryOperand(b, idx, type);
+                return MemoryOperand.Indexed(b, idx, type);
             }
         }
 

diff --git a/src/Arch/Sparc/SparcInstructionComparer.cs b/src/Arch/Sparc/SparcInstructionComparer.cs
@@ -61,12 +61,14 @@ private bool CompareOperands(MachineOperand a, MachineOperand b)
                 var mB = (MemoryOperand) b;
                 if (!CompareRegisters(mA.Base, mB.Base))
                     return false;
-                return CompareValues(mA.Offset, mB.Offset);
-            case IndexedMemoryOperand xA:
-                var xB = (IndexedMemoryOperand) b;
-                if (!CompareRegisters(xA.Base, xB.Base))
-                    return false;
-                return CompareRegisters(xA.Index, xB.Index);
+                if (mA.Offset is not null)
+                {
+                    return CompareValues(mA.Offset, mB.Offset);
+                }
+                else
+                {
+                    return CompareRegisters(mA.Index, mB.Index);
+                }
             }
             throw new NotImplementedException();
         }
@@ -95,10 +97,7 @@ private int GetOperandHash(MachineOperand op)
             case MemoryOperand m:
                 var h = GetRegisterHash(m.Base);
                 h = h ^ 29 * GetConstantHash(m.Offset);
-                return h;
-            case IndexedMemoryOperand x:
-                h = GetRegisterHash(x.Base);
-                h = h ^ 59 * GetRegisterHash(x.Index);
+                h = h ^ 59 * GetRegisterHash(m.Index);
                 return h;
             }
             throw new NotImplementedException();

diff --git a/src/Arch/Sparc/SparcRewriter.cs b/src/Arch/Sparc/SparcRewriter.cs
@@ -30,6 +30,7 @@
 using System;
 using System.Collections;
 using System.Collections.Generic;
+using System.Diagnostics;
 
 namespace Reko.Arch.Sparc
 {
@@ -358,31 +359,28 @@ private Expression RewriteMemOp(MachineOperand op, DataType size)
         {
             Expression? baseReg;
             Expression? offset;
-            if (op is MemoryOperand mem)
+            var mem = (MemoryOperand) op;
+            if (mem.Offset is not null)
             {
                 baseReg = mem.Base == arch.Registers.g0 ? null : binder.EnsureRegister(mem.Base);
                 offset = mem.Offset.IsIntegerZero ? null : mem.Offset;
             }
             else
             {
-                if (op is IndexedMemoryOperand i)
-                {
-                    baseReg = i.Base == arch.Registers.g0 ? null : binder.EnsureRegister(i.Base);
-                    offset = i.Index == arch.Registers.g0 ? null : binder.EnsureRegister(i.Index);
-                }
-                else
-                    throw new NotImplementedException(string.Format("Unknown memory operand {0} ({1})", op, op.GetType().Name));
+                Debug.Assert(mem.Index is not null);
+                baseReg = mem.Base == arch.Registers.g0 ? null : binder.EnsureRegister(mem.Base);
+                offset = mem.Index == arch.Registers.g0 ? null : binder.EnsureRegister(mem.Index);
             }
             return m.Mem(size, SimplifySum(baseReg, offset));
         }
 
         private Expression SimplifySum(Expression? srcLeft, Expression? srcRight)
         {
-            if (srcLeft == null && srcRight == null)
+            if (srcLeft is null && srcRight is null)
                 return Constant.Zero(PrimitiveType.Ptr32);
-            else if (srcLeft == null)
+            else if (srcLeft is null)
                 return srcRight!;
-            else if (srcRight == null)
+            else if (srcRight is null)
                 return srcLeft;
             else
                 return m.IAdd(srcLeft, srcRight);

diff --git a/src/Arch/Telink/TC32Architecture.cs b/src/Arch/Telink/TC32Architecture.cs
@@ -43,7 +43,7 @@ public TC32Architecture(IServiceProvider services, string archId, Dictionary<str
             this.InstructionBitSize = 16;
             this.MemoryGranularity = 8;
             this.PointerType = PrimitiveType.Ptr32;
-            this.StackRegister = null; //$TODO
+            this.StackRegister = null!; //$TODO
             this.WordWidth = PrimitiveType.Word32;
         }
 

diff --git a/src/Arch/X86/X86ProcessorArchitecture.cs b/src/Arch/X86/X86ProcessorArchitecture.cs
@@ -65,7 +65,7 @@ public enum FlagM : byte
     [Designer("Reko.Arch.X86.Design.X86ArchitectureDesigner,Reko.Arch.X86.Design")]
 	public class IntelArchitecture : ProcessorArchitecture
 	{
-        private ProcessorMode mode;
+        private readonly ProcessorMode mode;
         private Decoder[]? rootDecoders;
 
         public IntelArchitecture(IServiceProvider services, string archId, ProcessorMode mode, Dictionary<string, object> options)

diff --git a/src/Core/Machine/InstructionComparer.cs b/src/Core/Machine/InstructionComparer.cs
@@ -74,16 +74,16 @@ public virtual bool Equals(MachineInstruction? x, MachineInstruction? y)
             return CompareOperands(x, y);
         }
 
-        public int GetConstantHash(Constant c)
+        public int GetConstantHash(Constant? c)
         {
-            if ((norm & Normalize.Constants) != 0)
+            if ((norm & Normalize.Constants) != 0 || c is null)
                 return 0;
             return c.GetValue().GetHashCode();
         }
 
-        public int GetRegisterHash(RegisterStorage r)
+        public int GetRegisterHash(RegisterStorage? r)
         {
-            if ((norm & Normalize.Registers) != 0)
+            if ((norm & Normalize.Registers) != 0 || r is null)
                 return 0;
             return r.Number.GetHashCode();
         }

diff --git a/src/Gui/TextViewing/DisassemblyFormatter.cs b/src/Gui/TextViewing/DisassemblyFormatter.cs
@@ -53,6 +53,7 @@ public DisassemblyFormatter(
             this.arch = arch;
             this.instr = instr;
             this.line = line;
+            this.addrInstr = default!;
             this.annotations = new List<string>();
             this.mnemonicStyle = Gui.Services.UiStyles.DisassemblerOpcode;
         }
@@ -130,7 +131,7 @@ public void WriteUInt32(uint n)
             sb.Append(n);
         }
 
-        public void WriteString(string s)
+        public void WriteString(string? s)
         {
             sb.Append(s);
         }
@@ -140,9 +141,12 @@ public void WriteFormat(string fmt, params object[] parms)
             sb.AppendFormat(fmt, parms);
         }
 
-        public void AddAnnotation(string annotation)
+        public void AddAnnotation(string? annotation)
         {
-            this.annotations.Add(annotation);
+            if (annotation is not null)
+            {
+                this.annotations.Add(annotation);
+            }
         }
 
         public void NewLine()

diff --git a/src/UserInterfaces/AvaloniaUI/Controls/TextSpan.cs b/src/UserInterfaces/AvaloniaUI/Controls/TextSpan.cs
@@ -38,7 +38,7 @@ protected TextSpan()
         }
 
         public abstract string GetText();
-        public string Style { get; set; }
+        public string? Style { get; set; }
 
         public object? tag;
         public object? Tag {

diff --git a/src/UserInterfaces/AvaloniaUI/ViewModels/AvaloniaTextSpanFactory.cs b/src/UserInterfaces/AvaloniaUI/ViewModels/AvaloniaTextSpanFactory.cs
@@ -114,7 +114,7 @@ public class InertTextSpan : TextSpan
         {
             private readonly string text;
 
-            public InertTextSpan(string text, string style)
+            public InertTextSpan(string text, string? style)
             {
                 this.text = text;
                 base.Style = style;
@@ -169,7 +169,7 @@ public class MemoryTextSpan : TextSpan
 
             public Address Address { get; private set; }
 
-            public MemoryTextSpan(string text, string style)
+            public MemoryTextSpan(string text, string? style)
             {
                 this.text = text;
                 base.Style = style;

diff --git a/subjects/regression.log b/subjects/regression.log