[libre-riscv-dev] [Bug 64] data handling / io control / data routing API needed
bugzilla-daemon at libre-riscv.org
bugzilla-daemon at libre-riscv.org
Tue Apr 30 14:43:47 BST 2019
--- Comment #45 from Luke Kenneth Casson Leighton <lkcl at lkcl.net> ---
(In reply to Jacob Lifshay from comment #39)
> > shape() is used to find out, from the *recursively-iterable*
> > thing-of-Value-derivatives, what size of Signal() would be needed to store
> > it
> > (as a sequence of bits)
> I think we should call it bitlen or something like that, to me shape implies
> returning the type of the Data rather than the number of bits inside it.
yes. agreed. however...
> In C++ it would be sizeof vs. decltype.
...that needs to be a recommendation made to the nmigen developers, to change
the nmigen API from "shape" to "bitlen".
the nmoperator.shape function simply follows the nmigen convention: not
doing so is... well... inadviseable: it will result in "surprises":
so... i agree that yes, shape is a bad name... unfortunately, the name's
already been chosen, and it would be required to be documented that the
functionality is identical despite the name being different.
the better choice, i feel, is to stick with the [poor] name-choice, "shape()"
> > cat() is used to flatten() and *de*-flatten() the incoming and outgoing
> > hierarchy-of-Value-derivatives so that ONLY ONE Queue object is needed.
> I think we should do that inside the Queue implementation (or better yet,
> inside Memory, which is what the original Chisel code does).
mmmm this is an idea which, initially, i like, however it has knock-on
the first is: Memory2 (or whatever it is called) now has a critical
dependence on the nmoperator API, which would be the first "base"
utility class outside of the Stage/Pipe API to do so.
i'm... reluctant to set that precedent, particularly given that the API
would change to no longer specify a width, it would need to specify
the "spec". i.e. it would need to be passed an "iospecfn".
that's a really drastic change in the API, going beyond my comfort level
for a long-term API
second: the use of the flattening actually results in some truly
dreadful graphviz representations, as the data gets jammed into a
box of bits, then extracted on the other side in a truly awful messy
third: the same logic that applies to Memory also applies to Queue.
Queue's API would need to be changed, and i'm really not comfortable
right now, it's pretty damn obvious. read the chisel3 code, read
the comment about the name-changes, set the two pieces of code
side-by-side and it's damn obvious (if you can read chisel3 that is)
that the two are directly equivalent.
that equivalence is DESTROYED by embedding a hard critical dependence
on the iospecfn (data format) API.
use what i called FIFOControl... because FIFOControl *is* Queue [with
the flattening and processing added].
in an earlier iteration of FIFOControl, this direct one-for-one equivalence
was much more obvious, because i was able to make the input port DIRECTLY
equal to a PrevControl, and the output port DIRECTLY equal to a NextControl.
unfortunately due to the need to process the in/out data, that equivalence
was broken, and i had to extract the contents of NextControl._connect_in
and use those contents *in* FIFOControl.
* if you don't want the nmoperator API to be involved, don't want
the Shape API involved, don't want the pipeline API involved...
... use Queue
* if you *do* want the data to be flattened...
... use FIFOControl. they are EXACTLY and PRECISELY equivalent.
ABSOLUTELY NO CHANGE IN THE ARGUMENTS OR THE LOGIC HAS BEEN MADE.
if this is not understood, see the comments in queue.py:
din = enq_data, writable = enq_ready, we = enq_valid
dout = deq_data, re = deq_ready, readable = deq_valid
Memory has problems of its own (can't do length-1 arrays), which
makes a Memory2 worthwhile considering doing (for different reasons)
SyncFIFOBuffered should not exist, basically.
> We need at least one more function, create a new Data that shares the type
> of an existing one. I did that using Shape.get(data).create_data(), similar
> to python type(value)(). This is used to create Data instances for internal
> registers and what-not.
you're thinking of Value.like. Signal.like. etc.
yes, and the nice thing is: whilst Record.like does not exist (and it should),
nmoperator.like() can substitute for that lack by auto-detecting if it is
ever passed a Record.
the only thing is: we'll need to actually write a recursive-Record-walker
"like" function (as a non-member function).
basically Record.like is really really simple: grab the fields member,
and um create a new Record instance using that Record's fields.
the only thing we have to watch out for is: if we want to support something
that *derives* from Record, it should have its own "like" function so
that we can test for it (if hasattr(x, "like")) and call it.
You are receiving this mail because:
You are on the CC list for the bug.
More information about the libre-riscv-dev