cannon: Remove memory.GetUint32 (#12730)

* cannon: Remove memory.GetUint32

* review comments

cannon/cmd/run.go

@@ -13,6 +13,7 @@ import (
 
 	"github.com/ethereum-optimism/optimism/cannon/mipsevm"
 	"github.com/ethereum-optimism/optimism/cannon/mipsevm/arch"
+	mipsexec "github.com/ethereum-optimism/optimism/cannon/mipsevm/exec"
 	"github.com/ethereum-optimism/optimism/cannon/mipsevm/program"
 	"github.com/ethereum-optimism/optimism/cannon/mipsevm/versions"
 	preimage "github.com/ethereum-optimism/optimism/op-preimage"
@@ -410,14 +411,16 @@ func Run(ctx *cli.Context) error {
 
 		if infoAt(state) {
 			delta := time.Since(start)
+			pc := state.GetPC()
+			insn := mipsexec.LoadSubWord(state.GetMemory(), pc, 4, false, new(mipsexec.NoopMemoryTracker))
 			l.Info("processing",
 				"step", step,
 				"pc", mipsevm.HexU32(state.GetPC()),
-				"insn", mipsevm.HexU32(state.GetMemory().GetUint32(state.GetPC())),
+				"insn", mipsevm.HexU32(insn),
 				"ips", float64(step-startStep)/(float64(delta)/float64(time.Second)),
 				"pages", state.GetMemory().PageCount(),
 				"mem", state.GetMemory().Usage(),
-				"name", meta.LookupSymbol(state.GetPC()),
+				"name", meta.LookupSymbol(pc),
 			)
 		}
 

cannon/mipsevm/exec/memory.go

@@ -56,3 +56,7 @@ func (m *MemoryTrackerImpl) MemProof() [memory.MemProofSize]byte {
 func (m *MemoryTrackerImpl) MemProof2() [memory.MemProofSize]byte {
 	return m.memProof2
 }
+
+type NoopMemoryTracker struct{}
+
+func (n *NoopMemoryTracker) TrackMemAccess(Word) {}

cannon/mipsevm/exec/mips_instructions.go

@@ -22,7 +22,11 @@ const (
 )
 
 func GetInstructionDetails(pc Word, memory *memory.Memory) (insn, opcode, fun uint32) {
-	insn = memory.GetUint32(pc)
+	if pc&0x3 != 0 {
+		panic(fmt.Errorf("invalid pc: %x", pc))
+	}
+	word := memory.GetWord(pc & arch.AddressMask)
+	insn = uint32(SelectSubWord(pc, word, 4, false))
 	opcode = insn >> 26 // First 6-bits
 	fun = insn & 0x3f   // Last 6-bits
 

cannon/mipsevm/memory/memory.go

@@ -212,20 +212,6 @@ func (m *Memory) SetWord(addr Word, v Word) {
 	arch.ByteOrderWord.PutWord(p.Data[pageAddr:pageAddr+arch.WordSizeBytes], v)
 }
 
-// GetUint32 returns the first 32 bits located at the specified location.
-func (m *Memory) GetUint32(addr Word) uint32 {
-	// addr must be aligned to 4 bytes
-	if addr&3 != 0 {
-		panic(fmt.Errorf("unaligned memory access: %x", addr))
-	}
-	p, ok := m.pageLookup(addr >> PageAddrSize)
-	if !ok {
-		return 0
-	}
-	pageAddr := addr & PageAddrMask
-	return binary.BigEndian.Uint32(p.Data[pageAddr : pageAddr+4])
-}
-
 // GetWord reads the maximum sized value, [arch.Word], located at the specified address.
 // Note: Also referred to by the MIPS64 specification as a "double-word" memory access.
 func (m *Memory) GetWord(addr Word) Word {

cannon/mipsevm/memory/memory64_test.go

@@ -144,41 +144,24 @@ func TestMemory64ReadWrite(t *testing.T) {
 		m := NewMemory()
 		m.SetWord(16, 0xAABBCCDD_EEFF1122)
 		require.Equal(t, Word(0xAABBCCDD_EEFF1122), m.GetWord(16))
-		require.Equal(t, uint32(0xAABBCCDD), m.GetUint32(16))
-		require.Equal(t, uint32(0xEEFF1122), m.GetUint32(20))
 		m.SetWord(16, 0xAABB1CDD_EEFF1122)
 		require.Equal(t, Word(0xAABB1CDD_EEFF1122), m.GetWord(16))
-		require.Equal(t, uint32(0xAABB1CDD), m.GetUint32(16))
-		require.Equal(t, uint32(0xEEFF1122), m.GetUint32(20))
 		m.SetWord(16, 0xAABB1CDD_EEFF1123)
 		require.Equal(t, Word(0xAABB1CDD_EEFF1123), m.GetWord(16))
-		require.Equal(t, uint32(0xAABB1CDD), m.GetUint32(16))
-		require.Equal(t, uint32(0xEEFF1123), m.GetUint32(20))
 	})
 
 	t.Run("unaligned read", func(t *testing.T) {
 		m := NewMemory()
-		m.SetWord(16, 0xAABBCCDD_EEFF1122)
+		m.SetWord(16, Word(0xAABBCCDD_EEFF1122))
 		m.SetWord(24, 0x11223344_55667788)
 		for i := Word(17); i < 24; i++ {
 			require.Panics(t, func() {
 				m.GetWord(i)
 			})
-			if i != 20 {
-				require.Panics(t, func() {
-					m.GetUint32(i)
-				})
-			}
 		}
 		require.Equal(t, Word(0x11223344_55667788), m.GetWord(24))
-		require.Equal(t, uint32(0x11223344), m.GetUint32(24))
 		require.Equal(t, Word(0), m.GetWord(32))
-		require.Equal(t, uint32(0), m.GetUint32(32))
 		require.Equal(t, Word(0xAABBCCDD_EEFF1122), m.GetWord(16))
-		require.Equal(t, uint32(0xAABBCCDD), m.GetUint32(16))
-
-		require.Equal(t, uint32(0xEEFF1122), m.GetUint32(20))
-		require.Equal(t, uint32(0x55667788), m.GetUint32(28))
 	})
 
 	t.Run("unaligned write", func(t *testing.T) {
@@ -206,7 +189,6 @@ func TestMemory64ReadWrite(t *testing.T) {
 			m.SetWord(23, 0x11223344)
 		})
 		require.Equal(t, Word(0xAABBCCDD_EEFF1122), m.GetWord(16))
-		require.Equal(t, uint32(0xAABBCCDD), m.GetUint32(16))
 	})
 }
 
@@ -218,7 +200,6 @@ func TestMemory64JSON(t *testing.T) {
 	var res Memory
 	require.NoError(t, json.Unmarshal(dat, &res))
 	require.Equal(t, Word(0xAABBCCDD_EEFF1122), res.GetWord(8))
-	require.Equal(t, uint32(0xAABBCCDD), res.GetUint32(8))
 }
 
 func TestMemory64Copy(t *testing.T) {
@@ -226,6 +207,5 @@ func TestMemory64Copy(t *testing.T) {
 	m.SetWord(0xAABBCCDD_8000, 0x000000_AABB)
 	mcpy := m.Copy()
 	require.Equal(t, Word(0xAABB), mcpy.GetWord(0xAABBCCDD_8000))
-	require.Equal(t, uint32(0), mcpy.GetUint32(0xAABBCCDD_8000))
 	require.Equal(t, m.MerkleRoot(), mcpy.MerkleRoot())
 }

cannon/mipsevm/memory/memory_test.go

@@ -125,7 +125,6 @@ func TestMemoryReadWrite(t *testing.T) {
 			v := m.GetWord(i)
 			expected := binary.BigEndian.Uint32(data[i : i+4])
 			require.Equalf(t, expected, v, "read at %d", i)
-			require.Equalf(t, expected, m.GetUint32(i), "read at %d", i)
 		}
 	})
 
@@ -144,10 +143,8 @@ func TestMemoryReadWrite(t *testing.T) {
 		m := NewMemory()
 		m.SetWord(12, 0xAABBCCDD)
 		require.Equal(t, uint32(0xAABBCCDD), m.GetWord(12))
-		require.Equal(t, uint32(0xAABBCCDD), m.GetUint32(12))
 		m.SetWord(12, 0xAABB1CDD)
 		require.Equal(t, uint32(0xAABB1CDD), m.GetWord(12))
-		require.Equal(t, uint32(0xAABB1CDD), m.GetUint32(12))
 	})
 
 	t.Run("unaligned read", func(t *testing.T) {
@@ -156,22 +153,16 @@ func TestMemoryReadWrite(t *testing.T) {
 		m.SetWord(16, 0x11223344)
 		require.Panics(t, func() {
 			m.GetWord(13)
-			m.GetUint32(13)
 		})
 		require.Panics(t, func() {
 			m.GetWord(14)
-			m.GetUint32(14)
 		})
 		require.Panics(t, func() {
 			m.GetWord(15)
-			m.GetUint32(15)
 		})
 		require.Equal(t, uint32(0x11223344), m.GetWord(16))
-		require.Equal(t, uint32(0x11223344), m.GetUint32(16))
 		require.Equal(t, uint32(0), m.GetWord(20))
-		require.Equal(t, uint32(0), m.GetUint32(20))
 		require.Equal(t, uint32(0xAABBCCDD), m.GetWord(12))
-		require.Equal(t, uint32(0xAABBCCDD), m.GetUint32(12))
 	})
 
 	t.Run("unaligned write", func(t *testing.T) {
@@ -187,7 +178,6 @@ func TestMemoryReadWrite(t *testing.T) {
 			m.SetWord(15, 0x11223344)
 		})
 		require.Equal(t, uint32(0xAABBCCDD), m.GetWord(12))
-		require.Equal(t, uint32(0xAABBCCDD), m.GetUint32(12))
 	})
 }
 
@@ -199,7 +189,6 @@ func TestMemoryJSON(t *testing.T) {
 	var res Memory
 	require.NoError(t, json.Unmarshal(dat, &res))
 	require.Equal(t, uint32(0xAABBCCDD), res.GetWord(8))
-	require.Equal(t, uint32(0xAABBCCDD), res.GetUint32(8))
 }
 
 func TestMemoryCopy(t *testing.T) {
@@ -207,6 +196,5 @@ func TestMemoryCopy(t *testing.T) {
 	m.SetWord(0x8000, 123)
 	mcpy := m.Copy()
 	require.Equal(t, Word(123), mcpy.GetWord(0x8000))
-	require.Equal(t, Word(123), mcpy.GetUint32(0x8000))
 	require.Equal(t, m.MerkleRoot(), mcpy.MerkleRoot())
 }

cannon/mipsevm/tests/evm_common_test.go

@@ -86,7 +86,7 @@ func TestEVM(t *testing.T) {
 					if exitGroup && goVm.GetState().GetExited() {
 						break
 					}
-					insn := state.GetMemory().GetUint32(state.GetPC())
+					insn := testutil.GetInstruction(state.GetMemory(), state.GetPC())
 					t.Logf("step: %4d pc: 0x%08x insn: 0x%08x", state.GetStep(), state.GetPC(), insn)
 
 					stepWitness, err := goVm.Step(true)
@@ -868,7 +868,7 @@ func TestHelloEVM(t *testing.T) {
 				if goVm.GetState().GetExited() {
 					break
 				}
-				insn := state.GetMemory().GetUint32(state.GetPC())
+				insn := testutil.GetInstruction(state.GetMemory(), state.GetPC())
 				if i%1000 == 0 { // avoid spamming test logs, we are executing many steps
 					t.Logf("step: %4d pc: 0x%08x insn: 0x%08x", state.GetStep(), state.GetPC(), insn)
 				}
@@ -921,7 +921,7 @@ func TestClaimEVM(t *testing.T) {
 					break
 				}
 
-				insn := state.GetMemory().GetUint32(state.GetPC())
+				insn := testutil.GetInstruction(state.GetMemory(), state.GetPC())
 				if i%1000 == 0 { // avoid spamming test logs, we are executing many steps
 					t.Logf("step: %4d pc: 0x%08x insn: 0x%08x", state.GetStep(), state.GetPC(), insn)
 				}
@@ -969,7 +969,7 @@ func TestEntryEVM(t *testing.T) {
 				if goVm.GetState().GetExited() {
 					break
 				}
-				insn := state.GetMemory().GetUint32(state.GetPC())
+				insn := testutil.GetInstruction(state.GetMemory(), state.GetPC())
 				if i%10_000 == 0 { // avoid spamming test logs, we are executing many steps
 					t.Logf("step: %4d pc: 0x%08x insn: 0x%08x", state.GetStep(), state.GetPC(), insn)
 				}

cannon/mipsevm/testutil/memory.go

@@ -27,9 +27,12 @@ func StoreInstruction(mem *memory.Memory, pc Word, insn uint32) {
 	if pc&0x3 != 0 {
 		panic(fmt.Errorf("unaligned memory access: %x", pc))
 	}
-	exec.StoreSubWord(mem, pc, 4, Word(insn), new(noopMemTracker))
+	exec.StoreSubWord(mem, pc, 4, Word(insn), new(exec.NoopMemoryTracker))
 }
 
-type noopMemTracker struct{}
-
-func (n *noopMemTracker) TrackMemAccess(Word) {}
+func GetInstruction(mem *memory.Memory, pc Word) uint32 {
+	if pc&0x3 != 0 {
+		panic(fmt.Errorf("unaligned memory access: %x", pc))
+	}
+	return exec.LoadSubWord(mem, pc, 4, false, new(exec.NoopMemoryTracker))
+}