[libre-riscv-dev] [Bug 318] fix LDSTCompUnit

[bugzilla-daemon at libre-soc.org]

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

--- Comment #25 from Luke Kenneth Casson Leighton <lkcl at lkcl.net> ---
(In reply to Cesar Strauss from comment #24)
> (In reply to Luke Kenneth Casson Leighton from comment #22)
> > the issue that needs looking at is to confirm that compldst_multi.py's
> > unit tests are properly functional: i'm just not confident that they
> > are.
> 
> Have yet to see the traces, but found this by inspection:
> 
> in load():
> 
> 591        yield dut.rd.go.eq(rd)
> 592        yield from wait_for(dut.rd.rel)
> 593        yield dut.rd.go.eq(0)
> 
> This is backwards, I think.

it's simply broken, violating the protocol by potentially leaving the rd.go HI
for more than one cycle.  it needs to be

yield from waitfor(dut.rd.rel)
yield rd.go.eq(rd)
yield
yield rd.go.eq(0)

now it *may* be the case that by coincidence there is one clock in waitfor. 
that would give the correct ptotocol behaviour by accident.

> We wait for the CompUnit to assert rel, then we
> send a one-clock pulse on the corresponding go.

yes.

> 
> I guess a solution is to simply wait on one rel signal at a time.


except this would be a different type of test.


> 
> Eventually I'll come back to write the parallel test for this.

that would be fantastic.

> Meanwhile I'll take a look at the traces, see if I can catch anything.

you can see i caught an error where go signals are wired directly to rel
signals.

this trick will force an immediate response, in parallel, and there were a
number of corner cases in LDST and ALU CompUnits.

these turned out to be that the latch covering that particular go/req was
receiving a set *and* reset on the *same cycle*, and, with set taking priority,
the latch was left in the "set" position where it should have been dropped to
low.

even L0CacheBuffer had this exact same bug.  have a look at the last few git
diffs.

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