[Libre-soc-bugs] [Bug 742] MultiCompUnit wrmask relies on sync ok

[bugzilla-daemon at libre-soc.org]

https://bugs.libre-soc.org/show_bug.cgi?id=742

--- Comment #4 from Luke Kenneth Casson Leighton <lkcl at lkcl.net> ---
removing these seems to be ok:

--- a/src/soc/experiment/compalu_multi.py
+++ b/src/soc/experiment/compalu_multi.py
@@ -215,11 +215,9 @@ class MultiCompUnit(RegSpecALUAPI, Elaboratable):
         # is enough, when combined with when read-phase is done (rst_l.q)
         wr_any = Signal(reset_less=True)
         req_done = Signal(reset_less=True)
-        m.d.comb += self.done_o.eq(self.busy_o &
-                                   ~((self.wr.rel_o & ~self.wrmask).bool()))
+        m.d.comb += self.done_o.eq(self.busy_o & ~(self.wr.rel_o).bool())
         m.d.comb += wr_any.eq(self.wr.go_i.bool() | prev_wr_go.bool())
-        m.d.comb += req_done.eq(wr_any & ~self.alu.n.i_ready &
-                                ((req_l.q & self.wrmask) == 0))
+        m.d.comb += req_done.eq(wr_any & ~self.alu.n.i_ready & (req_l.q == 0))
         # argh, complicated hack: if there are no regs to write,
         # instead of waiting for regs that are never going to happen,
         # we indicate "done" when the ALU is "done"
@@ -360,7 +358,7 @@ class MultiCompUnit(RegSpecALUAPI, Elaboratable):

         # write-release gated by busy and by shadow (and write-mask)
         brd = Repl(self.busy_o & self.shadown_i, self.n_dst)
-        m.d.comb += self.wr.rel_o.eq(req_l.q & brd & self.wrmask)
+        m.d.comb += self.wr.rel_o.eq(req_l.q & brd)

         # output the data from the latch on go_write
         for i in range(self.n_dst):

the reasons why it's ok is likely because:

* wrmask is the trigger (ANDed) into req_l setting)
* wr.rel_o ANDed with wrmask therefore should never be done
  (and is redundant)
* almost all of the places removed are (wr.rel_o & wrmask)

the ones that i am leaving in are the "complicated hack":

        with m.If((self.wrmask == 0) &
                  self.alu.n.i_ready & self.alu.n.o_valid & self.busy_o):
            m.d.comb += req_done.eq(1)

this is a special indicator which detects when the incoming ready/valid
pulse is valid, which is supposed to be the only time when wrmask (the
Cat() of all data ok signals) is valid.

and when all data ok signals are zero, at the *exact* moment of the
incoming ready/valid, that indicates that there are no registers to
write, so there is no waiting.

now, on the face of it, it looks like wr_any covers this:

        m.d.comb += wr_any.eq(self.wr.go_i.bool() | prev_wr_go.bool())
        m.d.comb += req_done.eq(wr_any & ~self.alu.n.i_ready & (req_l.q == 0))

why would you have to request req_done to cancel the request when, clearly,
wr.go_i should have no requests, right?

wrong.

the special "hack" covers the case where the *ALU* decides that it has
no need to write to registers.

this is for instances where XER or CR for example (XER.so in particular)
writing is determined *by the ALU* that it has no need to write to XER.so,
saving on a reg write.

but, it was not possible to determine that before giving the ALU the
*opportunity* to write to the XER.so (or other register)

commit 0b16b46191e24089916b902a1512902c87fd782

-- 
You are receiving this mail because:
You are on the CC list for the bug.