[libre-riscv-dev] [Bug 64] data handling / io control / data routing API needed

[bugzilla-daemon at libre-riscv.org]

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

--- Comment #42 from Luke Kenneth Casson Leighton <lkcl at lkcl.net> ---
(In reply to Jacob Lifshay from comment #38)
> (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), 

 see multipipe.py.  the choice of names works without confusion for me.
 p becomes an array of PrevControl instances in the fan-in case.
 n becomes an array of NextControl instances in the fan-out case.

 no confusion in my mind arises over naming.

 p and n become plural, yet the PrevControl and NextControl objects
 within their (respective) arrays remain singular.


> 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.

 see below.  it's not just about the objects, it's about the function
 names that result during their use.


> >  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. 


 which one gets connected to which, again?

 what does connect_from_entry connect to again?

 and which way round is connect_from_exit?

 which entry is connected to what?

 what's the difference between connecting to an entry and connecting to
 an exit?

 which one does what again?

 ... no need to answer those questions: i'm illustrating that from the
 name *i can't tell*.

 that's enough to highlight that the choices - both entry/exit *and*
 connect_from_e/e - are not good choices.


> I
> don't currently have any better suggestions, except for .eq (where the
> direction is how data is assigned) or something like that.

 eq is actually different _again_... because those are *definitely*
 straight (non-direction-respecting) assignments.  this is PrevControl's
 eq function (which, actually, can now be removed, given that it
 has an __iter__)

    def eq(self, i):
        return [nmoperator.eq(self.data_i, i.data_i),
                self.ready_o.eq(i.ready_o),
                self.valid_i.eq(i.valid_i)]

 that is *NOT*, repeat *NOT* the same thing as _connect_in, which does this:

 def _connect_in(self, prev):
        return [self.valid_i.eq(valid_i),
                prev.ready_o.eq(self.ready_o),
                nmoperator.eq(self.data_i, data_i)]

 you see how _connect_in, the ready_o assignment is inverted in its
 direction?  this distinction is absolutely critically important
 enough to have a different function.

 it's basically the distinction between Record.eq and Record.connect.
 *both are needed*, and (see below) it's something that really, really
 needs to be extended down to the whole of the nmigen API.

 if the nmigen API (all of it) supported Direction, eq would *be* connect,
 and we would not be having this discussion.

 *sigh* :)


> 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.

 Signal also derives from Value, so it's not quite true, however i do
 know what you mean.

 take a deep breath... once things have stabilised and become a bit
 more established, i'm going to recommend a *massive* fundamental
 design paradigm shift to the nmigen developers: that the *entire*
 API consider "Direction" to be associated with *EVERY* bit on EVERY
 derivative from Value.

 and that connect be propagated down to Value as part of the API,
 and that a similar "direction-flipping" function (like in Chisel3)
 be added.

 this is the only way that the concept introduced in Record() can
 be properly supported.

 Const obviously would always be an OUT.

 the default would obviously always be INOUT (for backwards compatibility)

 extreme care will have to be taken in recommending it, as whitequark
 is not coping well with python, or the responsibility of being the
 maintainer of nmigen.

 anyway i digress.


> 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.

 [*clenched teeth and trying to smile at the same time*] iii kknoooow.

 it would help enormously if you could be a second person to have that
 conversation with whitequark, independently of me.


> I guess this is more of Rust rubbing off on me, 

 nooo, it's not just you, it's how whitequark is not accepting the
 reality of how people view Record, and wants to forcibly impose
 a particular mental perspective on how it must be used.

 the more people that can independently provide an alternative perspective,
 the easier things will become.

 or, we just fork nmigen.


> since in Rust most things
> are done through traits (interfaces) and not through specific types.

 yes - you'll need to be careful about that, as the technique results
 in an above-average increase in code size.


> 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.

 now you know why i insisted on the software engineering practice of
  well-documented code [and why writing unit ests are so utterly,
 utterly critical].

 you _can_ write code that doesn't have docstrings or unit tests... it'll
 just be a hopeless waste of time as not only other users won't understand
 it, neither will you in about... mmm.... 4-5 months time.

 i'm serious! :)

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