From 809f17813341f4bbf71fb9bab5f01746ef9eb857 Mon Sep 17 00:00:00 2001
From: Luke Kenneth Casson Leighton <lkcl@lkcl.net>
Date: Mon, 22 Feb 2021 15:59:33 +0000
Subject: [PATCH] moving PC-setting (NIA) out of execute_fsm in TestIssuer

---
 src/soc/simple/issuer.py           | 30 ++++++++++++++++++------------
 src/soc/simple/test/test_issuer.py | 12 ++++++------
 2 files changed, 24 insertions(+), 18 deletions(-)

diff --git a/src/soc/simple/issuer.py b/src/soc/simple/issuer.py
index e0cf39a7..f6daa834 100644
--- a/src/soc/simple/issuer.py
+++ b/src/soc/simple/issuer.py
@@ -244,7 +244,7 @@ class TestIssuerInternal(Elaboratable):
                 with m.If(exec_insn_ready_i):
                     m.next = "IDLE"
 
-    def execute_fsm(self, m, core, nia,
+    def execute_fsm(self, m, core, insn_done, pc_changed,
                         cur_state, fetch_insn_o,
                         fetch_pc_ready_o, fetch_pc_valid_i,
                         exec_insn_valid_o, exec_insn_ready_i):
@@ -268,8 +268,6 @@ class TestIssuerInternal(Elaboratable):
         core_issue_i = core.issue_i               # instruction is issued
         insn_type = core.e.do.insn_type           # instruction MicroOp type
 
-        pc_changed = Signal() # note write to PC
-
         with m.FSM():
 
             # go fetch the instruction at the current PC
@@ -297,7 +295,6 @@ class TestIssuerInternal(Elaboratable):
             with m.State("INSN_START"):
                 comb += core_ivalid_i.eq(1) # instruction is valid
                 comb += core_issue_i.eq(1)  # and issued
-                sync += pc_changed.eq(0)
 
                 m.next = "INSN_ACTIVE" # move to "wait completion"
 
@@ -308,15 +305,11 @@ class TestIssuerInternal(Elaboratable):
                 with m.If(self.state_nia.wen & (1<<StateRegs.PC)):
                     sync += pc_changed.eq(1)
                 with m.If(~core_busy_o): # instruction done!
-                    # ok here we are not reading the branch unit.  TODO
-                    # this just blithely overwrites whatever pipeline
-                    # updated the PC
-                    with m.If(~pc_changed):
-                        comb += self.state_w_pc.wen.eq(1<<StateRegs.PC)
-                        comb += self.state_w_pc.data_i.eq(nia)
+                    comb += insn_done.eq(1)
                     sync += core.e.eq(0)
                     sync += core.raw_insn_i.eq(0)
                     sync += core.bigendian_i.eq(0)
+                    sync += pc_changed.eq(0)
                     m.next = "INSN_FETCH"  # back to fetch
 
     def elaborate(self, platform):
@@ -437,7 +430,7 @@ class TestIssuerInternal(Elaboratable):
         fetch_pc_valid_i = Signal() # Execute tells Fetch "start next read"
         fetch_pc_ready_o = Signal() # Fetch Tells SVSTATE "proceed"
 
-        # SVSTATE FSM TODO.
+        # SVSTATE FSM TODO. actually, an "Issue" FSM that happens to do SVSTATE
         fetch_to_sv_ready_i = Signal()
         fetch_to_sv_valid_o = Signal()
 
@@ -467,7 +460,20 @@ class TestIssuerInternal(Elaboratable):
         # fetch pc/insn ready/valid and advances SVSTATE.srcstep
         # until it reaches VL-1 or PowerDecoder2.no_out_vec is True.
 
-        self.execute_fsm(m, core, nia,
+        # code-morph: moving the actual PC-setting out of "execute"
+        # so that it's easier to move this into an "issue" FSM.
+
+        # ok here we are not reading the branch unit.  TODO
+        # this just blithely overwrites whatever pipeline
+        # updated the PC
+        pc_changed = Signal() # note write to PC
+        insn_done = Signal()  # fires just once
+        core_busy_o = core.busy_o                 # core is busy
+        with m.If(insn_done & (~pc_changed) & (~core_busy_o)):
+            comb += self.state_w_pc.wen.eq(1<<StateRegs.PC)
+            comb += self.state_w_pc.data_i.eq(nia)
+
+        self.execute_fsm(m, core, insn_done, pc_changed,
                         cur_state, fetch_insn_o,
                         fetch_pc_ready_o, fetch_pc_valid_i,
                         exec_insn_valid_o, exec_insn_ready_i)
diff --git a/src/soc/simple/test/test_issuer.py b/src/soc/simple/test/test_issuer.py
index ebe93b4b..79c9619e 100644
--- a/src/soc/simple/test/test_issuer.py
+++ b/src/soc/simple/test/test_issuer.py
@@ -28,14 +28,14 @@ if __name__ == "__main__":
     unittest.main(exit=False)
     suite = unittest.TestSuite()
     # suite.addTest(TestRunner(HelloTestCases.test_data))
-    suite.addTest(TestRunner(DivTestCases().test_data))
+    #suite.addTest(TestRunner(DivTestCases().test_data))
     # suite.addTest(TestRunner(AttnTestCase.test_data))
-    suite.addTest(TestRunner(GeneralTestCases.test_data))
-    suite.addTest(TestRunner(LDSTTestCase().test_data))
-    suite.addTest(TestRunner(CRTestCase().test_data))
-    suite.addTest(TestRunner(ShiftRotTestCase().test_data))
+    #suite.addTest(TestRunner(GeneralTestCases.test_data))
+    #suite.addTest(TestRunner(LDSTTestCase().test_data))
+    #suite.addTest(TestRunner(CRTestCase().test_data))
+    #suite.addTest(TestRunner(ShiftRotTestCase().test_data))
     suite.addTest(TestRunner(LogicalTestCase().test_data))
-    suite.addTest(TestRunner(ALUTestCase().test_data))
+    #suite.addTest(TestRunner(ALUTestCase().test_data))
     # suite.addTest(TestRunner(BranchTestCase.test_data))
     # suite.addTest(TestRunner(SPRTestCase.test_data))
 
-- 
2.30.2