[Libre-soc-bugs] [Bug 755] add grev instruction (OP_GREV)

bugzilla-daemon at libre-soc.org bugzilla-daemon at libre-soc.org
Fri Dec 17 16:56:07 GMT 2021


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

--- Comment #8 from Jacob Lifshay <programmerjake at gmail.com> ---
(In reply to Luke Kenneth Casson Leighton from comment #7)
> https://git.libre-soc.org/?p=nmutil.git;a=commitdiff;
> h=c63d1f2b483573fae75af6688c07409b84efa7e4
> 
> i did a style rewrite, and updated the comments.  it's definitely a
> butterfly network - you can see by looking at the wikipedia page
> and comparing against the xbitmanip draft.
It's similar but not identical because wikipedia's example does grev from MSB
to LSB (swap 4, swap 2, swap 1) but the code I wrote goes the other way (swap
1, swap 2, swap 4).
>  it's definitely not
> an ror
I never said it's an ror, I said it's *similar* to an ror, which is true. it
uses a very similar sequence of 2:1 muxes.

> * i removed tee and pairwise: "adding yet more code" means "more things to
> read"

I documented exactly what pairwise does...the exact same as python's standard
library -- it returns successive pairs of the input items. imho it's waay
easier to understand how I had written it.

>   returned the code to the style that you replaced, which uses a list for
>   an accumulator, inside elaborate.
> 
>   this is perfectly fine to do because the dut is instantiated (elaborate
>   called) long before the unit test begins to refer to the actual contents

no it's not, you *obviously* didn't run the test (cuz you broke it). Also,
having new members that show up only after calling something other than the
constructor is poor style and quite confusing to use imho, hence why I replaced
it.
> 
>   this has the advantage of removing the intermediary signals from the
>   constructor, such that when someone reads the constructor to find out
>   what Signals they should be connecting up, they don't go "urrr what the
>   heck should i do with these"

then, simply, they just read the docstring for _steps and see that it says it's
an internal signal only exposed for unit tests, and they then can easily ignore
it. that should have been *also totally obvious* because its name starts with
an underline anyway.


> * the FSFE provides guidance on how to do proper copyright notices.
> 
>   Copyright (C) year, year, year, year fullname contactinfo

we had previously agreed just referring to Notices.txt was sufficient, cuz you
can put the copyright notice in there and only have to update the copyright
years in one place.
> 
>   it is NOT ok to put yearstart-yearend, and you MUST NOT claim
>   copyright in a year that you did not ACTUALLY make any additions.
> 
>   there are plenty of examples in the code to follow here.

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


More information about the libre-soc-bugs mailing list