fix: adding remark to SHR and SHL

Kelvinyu1117 · Jun 9, 2024 · e7bc36c · e7bc36c
1 parent d637061
commit e7bc36c
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 10 deletions.
diff --git a/arabica/cpu/cpu.cpp b/arabica/cpu/cpu.cpp
@@ -113,6 +113,10 @@ void CPU::run(const Memory& memory) {
       advance_pc(pc);
     } break;
     case OP_CODE::SHR_Vx: {
+      // Remark: historically, the semantics is "right shift V[x] by V[y] amount
+      // and store the result to V[x]" in the original chip8 implementation, however, most of the game
+      // after 90s follows the buggy implementation of HP which ignoring V[y], so we just follow the same for now.
+
       uint8_t x      = (instruction & 0x0F00) >> 8;
       registers[0xF] = registers[x] & 1;
       registers[x]   = registers[x] >> 1;
@@ -129,6 +133,9 @@ void CPU::run(const Memory& memory) {
       advance_pc(pc);
     } break;
     case OP_CODE::SHL_Vx: {
+      // Remark: historically, the semantics is "left shift V[x] by V[y] amount
+      // and store the result to V[x]" in the original chip8 implementation, however, most of the game
+      // after 90s follows the buggy implementation of HP which ignoring V[y], so we just follow the same for now.
       uint8_t x      = (instruction & 0x0F00) >> 8;
       registers[0xF] = (registers[x] >> 7) & 1;
       registers[x]   = registers[x] << 1;

diff --git a/test/cpu/cpu_test_suite.hpp b/test/cpu/cpu_test_suite.hpp
@@ -276,39 +276,39 @@ arabica_cpu_test(test_shr_vx,
 
   // SHR V[0], expected V[0] = 0x7
   memory.write(0x202, 0x80);
-  memory.write(0x203, 0x16);
+  memory.write(0x203, 0x06);
   cpu.run(memory);
   ASSERT_EQ(cpu.pc, 0x204);
   ASSERT_EQ(cpu.registers[0], 0x7);
   ASSERT_EQ(cpu.registers[0xF], 1);
 
   // SHR V[0], expected V[0] = 0x3
   memory.write(0x204, 0x80);
-  memory.write(0x205, 0x16);
+  memory.write(0x205, 0x06);
   cpu.run(memory);
   ASSERT_EQ(cpu.pc, 0x206);
   ASSERT_EQ(cpu.registers[0], 0x3);
   ASSERT_EQ(cpu.registers[0xF], 1);
 
   // SHR V[0], expected V[0] = 0x1
   memory.write(0x206, 0x80);
-  memory.write(0x207, 0x16);
+  memory.write(0x207, 0x06);
   cpu.run(memory);
   ASSERT_EQ(cpu.pc, 0x208);
   ASSERT_EQ(cpu.registers[0], 0x1);
   ASSERT_EQ(cpu.registers[0xF], 1);
 
   // SHR V[0], expected V[0] = 0x0
   memory.write(0x208, 0x80);
-  memory.write(0x209, 0x16);
+  memory.write(0x209, 0x06);
   cpu.run(memory);
   ASSERT_EQ(cpu.pc, 0x20A);
   ASSERT_EQ(cpu.registers[0], 0x0);
   ASSERT_EQ(cpu.registers[0xF], 1);
 
   // SHR V[0], expected V[0] = 0x0
   memory.write(0x20A, 0x80);
-  memory.write(0x20B, 0x16);
+  memory.write(0x20B, 0x06);
   cpu.run(memory);
   ASSERT_EQ(cpu.pc, 0x20C);
   ASSERT_EQ(cpu.registers[0], 0x0);
@@ -372,39 +372,39 @@ arabica_cpu_test(test_shl_vx,
 
   // SHL V[0], expected V[0] = 0x1E
   memory.write(0x202, 0x80);
-  memory.write(0x203, 0x1E);
+  memory.write(0x203, 0x0E);
   cpu.run(memory);
   ASSERT_EQ(cpu.pc, 0x204);
   ASSERT_EQ(cpu.registers[0], 0x1E);
   ASSERT_EQ(cpu.registers[0xF], 0);
 
   // SHL V[0], expected V[0] = 0x3C
   memory.write(0x204, 0x80);
-  memory.write(0x205, 0x1E);
+  memory.write(0x205, 0x0E);
   cpu.run(memory);
   ASSERT_EQ(cpu.pc, 0x206);
   ASSERT_EQ(cpu.registers[0], 0x3C);
   ASSERT_EQ(cpu.registers[0xF], 0);
 
   // SHL V[0], expected V[0] = 0x78
   memory.write(0x206, 0x80);
-  memory.write(0x207, 0x1E);
+  memory.write(0x207, 0x0E);
   cpu.run(memory);
   ASSERT_EQ(cpu.pc, 0x208);
   ASSERT_EQ(cpu.registers[0], 0x78);
   ASSERT_EQ(cpu.registers[0xF], 0);
 
   // SHL V[0], expected V[0] = 0xF0
   memory.write(0x208, 0x80);
-  memory.write(0x209, 0x1E);
+  memory.write(0x209, 0x0E);
   cpu.run(memory);
   ASSERT_EQ(cpu.pc, 0x20A);
   ASSERT_EQ(cpu.registers[0], 0xF0);
   ASSERT_EQ(cpu.registers[0xF], 0);
 
   // SHL V[0], expected V[0] = 0xE0
   memory.write(0x20A, 0x80);
-  memory.write(0x20B, 0x1E);
+  memory.write(0x20B, 0x0E);
   cpu.run(memory);
   ASSERT_EQ(cpu.pc, 0x20C);
   ASSERT_EQ(cpu.registers[0], 0xE0);