[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 03:27:08 BST 2019


http://bugs.libre-riscv.org/show_bug.cgi?id=64

--- Comment #38 from Jacob Lifshay <programmerjake at gmail.com> ---
(In reply to Luke Kenneth Casson Leighton from comment #21)
> (In reply to Jacob Lifshay from comment #8)
> 
> > > * the name "EntryPort" begins with the letter E, and so does "ExitPort".
> > >   this is problematic from a clarity perspective in later usage
> > >   (see below)
> > I wanted to pick a name pair other than input/output to avoid confusion with
> > the signal directions.
> > What about ingress/egress, from/to, upstream/downstream, or next/previous?
> 
>  from/to - from is a python keyword. ingress/egress too pretentious .. :)
>  upstream/downstream ok, next/previous is ok.
> 
> > Or do you think input/output will be fine?
>  ...
>  total utter confusion.
I actually think we should use something more like input/output, but not
literally input/output, because I wanted to emphasize that the pipeline
building blocks API should support arbitrary directed graphs (multiple
entries/exits per block), not just linear graphs, hence why I picked
entry/exit.
If I have to choose from upstream/downstream and next/previous, I have a mild
preference for upstream/downstream, though I think entry/exit could also work
if abbreviated to e/x.

>  similarly with connect_to_entry and connect_to_exit.  both beginning with
> "e"
>  and both having "to" and "from", i can stare at the piece of code for ten
>  to twenty minutes and not understand it.
> 
>  the use of a totally different word ("connect_in") and the inclusion of
>  an underscore (on the one that's *not* supposed to be used to connect
>  stage-to-stage) is sufficient to indicate "this is a private function"
>  and the logic/letter-based dyslexia is broken.
The connect_from_* functions are not supposed to be private functions. I don't
currently have any better suggestions, except for .eq (where the direction is
how data is assigned) or something like that.

> > > * use of Data.assign for ready_in/out and valid_in/out, they should just
> > >   be an eq.
> > They can't be a member function because Record.eq doesn't work (the
> > simulator doesn't handle it),
> 
>  i was referring only to ready_in/out and valid_in/out, not Data in/out.
>  ready_in/out are pure signals (not Data), and therefore eq works fine.
>  (i.e. it's just Signal.eq, not Data.eq and not Record.eq)
I was using Data.assign for consistency's sake, am perfectly fine switching to
Signal.eq.

> > though I guess we could reassign Record.eq to
> > a custom function that does work. 
> 
>  whilst Record has been "fixed", i will be absolutely bluntly honest: i am
>  not happy to be beholden to the way that nmigen's development is being
>  handled.
> 
>  ...
> 
>  the global eq(), shape() and cat() functions (now moved to nmobject.py
>  pending a decision) are under *our* control.
> 
>  Record.eq is not.
> 
>  i have already had to add workarounds into both eq() and cat() that
>  take into consideration both bugs in nmigen and intransigence in its
>  key developer.
> 
>  we cannot have our entire project at the mercy of a team that is overloaded
>  and does not understand or respect python conventions, and who have
>  demonstrated an irrational and pathological unwillingness to listen.
> 
>  this said despite the fact that nmigen is otherwise extremely good,
>  and, i believe, worth sticking with.

In my opinion, Record should not be a Value, since a Value behaves like a
Signal, and the only shared operations are assignment (though that technically
is different because Record members have a direction and .connect is named
differently). This is another instance of abusing inheritance to share
implementations, rather than because Record is a Value.

I guess this is (since there is no documentation to state otherwise) because I
see a Record as a tree of named Signals rather than a Signal that has parts
that happen to be named.

I guess this is more of Rust rubbing off on me, since in Rust most things are
done through traits (interfaces) and not through specific types. This is less
restrictive than class inheritance in that a type implementing some trait is
independent of implementing some other trait unless the traits are related.

This is similar to how types work in python except that, in python, the traits
required are stated in the documentation (we hope) rather than in the actual
code.

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


More information about the libre-riscv-dev mailing list