Correct shift operation and make ALU_OP_MOVZ and ALU_OP_MOVN encoding independent.

Signed-off-by: Pavel Pisa <pisa@cmp.felk.cvut.cz>

6 files changed, 44 insertions, 25 deletions

diff --git a/qtmips_machine/alu.cpp b/qtmips_machine/alu.cpp
index f83c4b2..83e73bb 100644
--- a/qtmips_machine/alu.cpp
+++ b/qtmips_machine/alu.cpp
@@ -39,9 +39,10 @@
 
 using namespace machine;
 
-std::uint32_t machine::alu_operate(enum AluOp operation, std::uint32_t s, std::uint32_t t, std::uint8_t sa, Registers *regs) {
+std::uint32_t machine::alu_operate(enum AluOp operation, std::uint32_t s, std::uint32_t t, std::uint8_t sa, Registers *regs, bool &discard) {
     std::int64_t s64_val;
     std::uint64_t u64_val;
+    discard = false;
 
     switch(operation) {
         case ALU_OP_SLL:
@@ -65,11 +66,13 @@ std::uint32_t machine::alu_operate(enum AluOp operation, std::uint32_t s, std::u
             // Pass return value in rt to save PC after isntruction, program counter is handled in handle_pc
             return t;
         case ALU_OP_MOVZ:
-            // We do this just to implement valid alu operation but we have to evaluate comparison outside of this function to disable register write
-            return t == 0 ? s : 0;
+            // Signal discard of result when condition is not true
+            discard = t != 0;
+            return discard ? 0: s;
         case ALU_OP_MOVN:
             // Same note as for MOVZ applies here
-            return t != 0 ? s : 0;
+            discard = t == 0;
+            return discard ? 0: s;
         case ALU_OP_BREAK:
              return 0;
         case ALU_OP_MFHI:
diff --git a/qtmips_machine/alu.h b/qtmips_machine/alu.h
index efadbe0..372e4e6 100644
--- a/qtmips_machine/alu.h
+++ b/qtmips_machine/alu.h
@@ -52,7 +52,7 @@ namespace machine {
 // sa: This is value directly from instruction it self (sa section) used for shift operations
 // regs: Registers used. We need direct access to lo and hi registers (those are not accessed from core it self but from alu directly
 // Returned value is commonly saved to rt/rd or any other way passed trough core
-std::uint32_t alu_operate(enum AluOp operation, std::uint32_t s, std::uint32_t t, std::uint8_t sa, Registers *regs);
+std::uint32_t alu_operate(enum AluOp operation, std::uint32_t s, std::uint32_t t, std::uint8_t sa, Registers *regs, bool &discard);
 
 }
 
diff --git a/qtmips_machine/core.cpp b/qtmips_machine/core.cpp
index c6093dd..7e529ae 100644
--- a/qtmips_machine/core.cpp
+++ b/qtmips_machine/core.cpp
@@ -149,17 +149,18 @@ struct Core::dtDecode Core::decode(const struct dtFetch &dt) {
 
 struct Core::dtExecute Core::execute(const struct dtDecode &dt) {
     emit instruction_executed(dt.inst);
+    bool discard;
 
     // Handle conditional move (we have to change regwrite signal if conditional is not met)
     bool regwrite = dt.regwrite;
-    if (dt.inst.opcode() == 0 && ((dt.inst.funct() == ALU_OP_MOVZ && dt.val_rt != 0) || (dt.inst.funct() == ALU_OP_MOVN && dt.val_rt == 0)))
-        regwrite = false;
 
     std::uint32_t alu_sec = dt.val_rt;
     if (dt.alusrc)
         alu_sec = dt.immediate_val; // Sign or zero extend immediate value
 
-    std::uint32_t alu_val = alu_operate(dt.aluop, dt.val_rs, alu_sec, dt.inst.shamt(), regs);
+    std::uint32_t alu_val = alu_operate(dt.aluop, dt.val_rs, alu_sec, dt.inst.shamt(), regs, discard);
+    if (discard)
+        regwrite = false;
 
     emit execute_alu_value(alu_val);
     emit execute_reg1_value(dt.val_rs);
diff --git a/qtmips_machine/instruction.cpp b/qtmips_machine/instruction.cpp
index 94c412d..e5e8be9 100644
--- a/qtmips_machine/instruction.cpp
+++ b/qtmips_machine/instruction.cpp
@@ -49,13 +49,14 @@ using namespace machine;
 #define FLAGS_ALU_I_STORE (IMF_SUPPORTED | IMF_ALUSRC | IMF_MEMWRITE | \
                           IMF_MEM | IMF_MEM_STORE | IMF_ALU_REQ_RS | IMF_ALU_REQ_RT)
 
-#define FLAGS_ALU_T_R (IMF_SUPPORTED | IMF_REGD | IMF_REGWRITE)
-#define FLAGS_ALU_T_R_STD (IMF_SUPPORTED | IMF_REGD | IMF_REGWRITE \
-                         | IMF_ALU_REQ_RS | IMF_ALU_REQ_RT)
-#define FLAGS_ALU_T_R_ST (IMF_SUPPORTED | IMF_ALU_REQ_RS | IMF_ALU_REQ_RT)
+#define FLAGS_ALU_T_R_D (IMF_SUPPORTED | IMF_REGD | IMF_REGWRITE)
+#define FLAGS_ALU_T_R_STD (FLAGS_ALU_T_R_D | IMF_ALU_REQ_RS | IMF_ALU_REQ_RT)
+#define FLAGS_ALU_T_R_STD_SHV (FLAGS_ALU_T_R_STD | IMF_ALU_SHIFT)
+#define FLAGS_ALU_T_R_TD (FLAGS_ALU_T_R_D | IMF_ALU_REQ_RT)
+#define FLAGS_ALU_T_R_TD_SHAMT (FLAGS_ALU_T_R_TD | IMF_ALU_SHIFT)
 #define FLAGS_ALU_T_R_S (IMF_SUPPORTED |  IMF_ALU_REQ_RS)
-#define FLAGS_ALU_T_R_D (IMF_SUPPORTED | IMF_REGD | IMF_REGWRITE )
-#define FLAGS_ALU_T_R_SD (IMF_SUPPORTED | IMF_REGD | IMF_REGWRITE | IMF_ALU_REQ_RS)
+#define FLAGS_ALU_T_R_SD (FLAGS_ALU_T_R_D | IMF_ALU_REQ_RS)
+#define FLAGS_ALU_T_R_ST (IMF_SUPPORTED | IMF_ALU_REQ_RS | IMF_ALU_REQ_RT)
 
 #define NOALU .alu = ALU_OP_SLL
 #define NOMEM .mem_ctl = AC_NONE
@@ -82,19 +83,19 @@ struct InstructionMap {
 // This table is indexed by funct
 static const struct InstructionMap  alu_instruction_map[] = {
     {"SLL",    IT_R, ALU_OP_SLL, NOMEM, nullptr,
-     .flags = FLAGS_ALU_T_R_STD},
+     .flags = FLAGS_ALU_T_R_TD_SHAMT},
     IM_UNKNOWN,
     {"SRL",    IT_R, ALU_OP_SRL, NOMEM, nullptr,
-     .flags = FLAGS_ALU_T_R_SD},
+     .flags = FLAGS_ALU_T_R_TD_SHAMT},
     {"SRA",    IT_R, ALU_OP_SRA, NOMEM, nullptr,
-     .flags = FLAGS_ALU_T_R_SD},
+     .flags = FLAGS_ALU_T_R_TD_SHAMT},
     {"SLLV",   IT_R, ALU_OP_SLLV, NOMEM, nullptr,
-     .flags = FLAGS_ALU_T_R_STD},
+     .flags = FLAGS_ALU_T_R_STD_SHV},
     IM_UNKNOWN,
     {"SRLV",   IT_R, ALU_OP_SRLV, NOMEM, nullptr,
-     .flags = FLAGS_ALU_T_R_STD},
+     .flags = FLAGS_ALU_T_R_STD_SHV},
     {"SRAV",   IT_R, ALU_OP_SRAV, NOMEM, nullptr,
-     .flags = FLAGS_ALU_T_R_STD},
+     .flags = FLAGS_ALU_T_R_STD_SHV},
     {"JR",     IT_R, ALU_OP_JR, NOMEM, nullptr,
      .flags = IMF_SUPPORTED | IMF_BJR_REQ_RS},
     {"JALR",   IT_R, ALU_OP_JALR, NOMEM, nullptr,
@@ -428,14 +429,25 @@ QString Instruction::to_str() const {
             res += next_delim + "$" + QString::number(rd());
             next_delim = ", ";
         }
-        if (im.flags & (IMF_BJR_REQ_RS | IMF_ALU_REQ_RS)) {
-            res += next_delim + "$" + QString::number(rs());
-            next_delim = ", ";
+        if (!(im.flags & IMF_ALU_SHIFT)) {
+            if (im.flags & (IMF_BJR_REQ_RS | IMF_ALU_REQ_RS)) {
+                res += next_delim + "$" + QString::number(rs());
+                next_delim = ", ";
+            }
         }
         if (im.flags & (IMF_BJR_REQ_RT | IMF_ALU_REQ_RT)) {
             res += next_delim + "$" + QString::number(rt());
             next_delim = ", ";
         }
+        if (im.flags & IMF_ALU_SHIFT) {
+            if (im.flags & (IMF_BJR_REQ_RS | IMF_ALU_REQ_RS)) {
+                res += next_delim + "$" + QString::number(rs());
+                next_delim = ", ";
+            } else {
+                res += next_delim + QString::number(shamt());
+                next_delim = ", ";
+            }
+        }
         break;
         }
 	case T_UNKNOWN:
diff --git a/qtmips_machine/instruction.h b/qtmips_machine/instruction.h
index b8c62b9..8396bc7 100644
--- a/qtmips_machine/instruction.h
+++ b/qtmips_machine/instruction.h
@@ -55,6 +55,7 @@ enum InstructionFlags {
     IMF_PC_TO_R31  = 1L<<7,
     IMF_BJR_REQ_RS = 1L<<8,
     IMF_BJR_REQ_RT = 1L<<9,
+    IMF_ALU_SHIFT = 1L<<10,
     IMF_MEM        = 1L<<11, // This instruction is memory access instruction,
     IMF_MEM_STORE  = 1L<<12,
     IMF_ALU_REQ_RS = 1L<<13,
diff --git a/qtmips_machine/tests/testalu.cpp b/qtmips_machine/tests/testalu.cpp
index 304c001..d3d27ec 100644
--- a/qtmips_machine/tests/testalu.cpp
+++ b/qtmips_machine/tests/testalu.cpp
@@ -215,6 +215,7 @@ void MachineTests::alu_data() {
 }
 
 void MachineTests::alu() {
+    bool discard;
     QFETCH(AluOp, op);
     QFETCH(std::uint32_t, s);
     QFETCH(std::uint32_t, t);
@@ -223,7 +224,7 @@ void MachineTests::alu() {
     QFETCH(Registers, regs_res);
     QFETCH(std::uint32_t, res);
 
-    QCOMPARE(alu_operate(op, s , t, sa, &regs_init), res);
+    QCOMPARE(alu_operate(op, s , t, sa, &regs_init, discard), res);
     QCOMPARE(regs_res, regs_init);
 }
 
@@ -246,11 +247,12 @@ void MachineTests::alu_except_data() {
 }
 
 void MachineTests::alu_except() {
+    bool discard;
     QFETCH(std::uint8_t, op);
     QFETCH(std::uint32_t, s);
     QFETCH(std::uint32_t, t);
     Registers regs;
 
     // Only runtime exception is expected as any other exception is a bug
-    QVERIFY_EXCEPTION_THROWN(alu_operate((enum AluOp)op, s , t, 0, &regs), QtMipsExceptionRuntime);
+    QVERIFY_EXCEPTION_THROWN(alu_operate((enum AluOp)op, s , t, 0, &regs, discard), QtMipsExceptionRuntime);
 }