[libre-riscv-dev] pipeline sync issues

[Luke Kenneth Casson Leighton]

On Wed, Apr 17, 2019 at 8:09 AM Luke Kenneth Casson Leighton
<lkcl at lkcl.net> wrote:
>
> On Wed, Apr 17, 2019 at 5:31 AM Jacob Lifshay <programmerjake at gmail.com> wrote:
> >
> > On Tue, Apr 16, 2019 at 5:45 AM Luke Kenneth Casson Leighton
> > <lkcl at lkcl.net> wrote:
> > >
> > > that's it: pipe mode is where the problems occur.
> > >
> > >                      # set readable and writable (NOTE: see pipe mode below)
> > >                      self.readable.eq(~empty), # cannot read if empty!
> > >                      self.writable.eq(~full),  # cannot write if full!
> > >
> > > so that's all sensible, so far.  if there's no room, don't allow
> > > writing!  unfortunately... "pipe mode" does this:
> > >
> > >             with m.If(self.re):
> > >                 m.d.comb += self.writable.eq(1)
> > >
> > > i.e. the "writeable" conditions are over-ridden (against plain
> > > commonsense, i feel obliged to say).
> > >
> > > thus, if read is enabled, writable is *FORCED* to be set.  therefore,
> > > if the output does NOT read the data, the input will DESTROY one value
> > > in the FIFO by overwriting it.
> > The value has not been destroyed, it's been output and the space in
> > memory is simultaneously reused for the next input value. This works
> > because the memory read port is asynchronous and therefore reads the
> > old value and the write port is synchronous, so it acts similarly to a
> > D flip-flop in that the old value can be read in the same clock cycle
> > that a new value is being written.
>
>  i understand that... it wasn't quite what i meant, having missed out
> a word that allowed an ambiguous meaning.
>
>  i meant, destroyed by virtue not of the conditions *inside* the stage
> that contains the memory cell, instead by conditions (the contract)
> being misunderstood *outside* of the stage that contains the memory
> cell, i.e. by the *previous* stage
>
>  i.e. by virtue of the former stage having set e.g. flow=True,
> pipe=False, and the following stage having set... i don't know..
> say... flow=False,pipe=True.
>
>  my hunch is that there may be some conditions that, given the
> existence of the new parameters flow (fwft) and pipe, we *might* be
> able to *programmatically* check that, if a connection between two
> pipes is made, and we *know* the conditions under which data will be
> destroyed, it will be possible to throw an exception.
>
> > > would you concur, jacob?
> > I think there's no problem with that. If deq_ready (renamed to re) is
> > set by external logic, that tells Queue that the external logic wants
> > to transfer a value out, so if the external logic doesn't actually use
> > the value it asked for, that's not Queue's fault.
>
>  it's not the Queue's fault... yet what is to stop a programmer from
> *not realising* that a problem can occur, and connecting two
> pipe-stages together that are *known* to destroy data?
>
>  we *need* to track this down, properly.

i replace (over-rode) PassThroughHandshake with this:

 class PassThroughHandshake(FIFOControl):
    def __init__(self, stage, in_multi=None, stage_ctl=False):
        FIFOControl.__init__(self, 1, stage, in_multi, stage_ctl,
                                   fwft=False, pipe=True)

and it failed (did not pass unit test 18).  can you please investigate?

if the code that you wrote at the end of queue.py is correct, the
above is intended to be reg_stage:
reg_stage = Queue(1, 1, pipe=True) # fwft=True as the default argument to Queue

however it doesn't work, whereas this does:
        FIFOControl.__init__(self, 1, stage, in_multi, stage_ctl,
                                   fwft=True, pipe=True)


a second discrepancy:
break_ready_chain_stage = Queue(1, 1, pipe=True, fwft=True)

and with the default argument fwft=True... and from what you wrote
earlier, you must mean:
break_ready_chain_stage = Queue(1, 1, pipe=False, fwft=True)


basically i really like the Queue class, it has the built-in
workaround which makes it much better than SyncFIFO and
SyncFIFOBuffered, and it seems to also cover all potential use cases.

therefore i'd like to identify all of the use cases, replace them with
overrides of FIFOControl, and *delete* all of the other classes.

can you help do that?

l.