[Libre-soc-bugs] [Bug 713] PartitionedSignal enhancement to add partition-context-aware lengths

[bugzilla-daemon at libre-soc.org]

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

--- Comment #98 from Luke Kenneth Casson Leighton <lkcl at lkcl.net> ---
(In reply to Jacob Lifshay from comment #97)
> I added XLEN, and refactored layout into a SimdLayout class.

as a 957-line diff.  remember i said that the Project Development Policy
is to do small incremental commits?

  git diff 4fca5d | wc
    957    4486   36684


> (In reply to Luke Kenneth Casson Leighton from comment #95)
> > dont go overboard.  add, mul, div, sub, is probably about it.
> > operands int more than enough.
> 
> umm...well lets just say you won't have to worry about missing some
> methods...

this will take several days for me to review and understand.
far from moving the project forward when we are under time pressure,
that unfortunately delays it even more.

> maybe I should have put it in an alternate branch...

it's not the branch, it's that it's such a massive diff that it
stops me from being able to understand it.

ok.

what to do.

what have i seen other project lead developers do in the past...

i know: yes, it needs reversion, and moving to a branch, and to
create a series of "planned" (incremental) patches.

1) add SimdMap and util.py but do *not* modify layout_experiment.py
2) fix the bugs in layout_experiment.py (and leave in the accidental
   example where the layout was {0:5 1:6 2:6 3:6})
3) update layout_experiment.py to a class but do *not* get it to use
   SimdMap (because it hasn't been reviewed / authorised)
4) then we join the two together

also, leave in the dots:

+    # elwid=F64 1x 24-bit    |<--------i24-------->|
+    # elwid=F32 2x 12-bit    |<---i12--->|<---i12--->|
+    # elwid=F16 4x 6-bit     |<-i6>|<-i6>|<-i6>|<-i6>|
+    # elwid=BF16 4x 5-bit    |.<i5>|.<i5>|.<i5>|.<i5>|

what you'd done in the (957-line diff) was remove the examples which
i'd spent some time analysing and understanding.

i _was_ going to add Asserts which would (eventually form the basis
of a proper unit test) show that layout is producing the wrong answer

because you have made *two* changes combined into one, it is now
very difficult to illustrate to you what i mean.

you changed this:

    widths_at_elwidth = {
        0: 5,
        1: 6,
        2: 12,
        3: 24
    }

to this:

    widths_at_elwidth = {
        FpElWid.F64: 24,
        FpElWid.F32: 12,
        FpElWid.F16: 6,
        FpElWid.BF16: 5,
    }

(and likewise part_count)

     part_counts = {
-        0: 1,
-        1: 1,
-        2: 2,
-        3: 4,
+        FpElWid.F64: 4,
+        FpElWid.F32: 2,
+        FpElWid.F16: 1,
+        FpElWid.BF16: 1,
     }

which is *two* changes.

1) changing of the actual vector element widths
2) changing of the specification of elwidth from int type to FpElwid type

both of these compounded changes now make it extra work to explain.

in the existing layout_experiment.py (as of yesterday), the following
was done:

    widths_at_elwidth = {
        0: 5,    # not divided --> vec element = 5
        1: 6,    # not divided --> vec element = 6
        2: 12,   # gets divided by 2 --> vec element = 6
        3: 24    # gets divided by 4 --> vec element = 6
    }


oh s*** i think i now know what you were talking about, finally.

+        FpElWid.F64: 4,
+        FpElWid.F32: 2,
+        FpElWid.F16: 1,
+        FpElWid.BF16: 1,

you've still specified F64 as comprising *four* partitions.

the requirement is to *NOT* specify the number of *BYTES*.

the requirement is to specify the NUMBER OF ELEMENTS.

we *require* part_count to be:

     part_counts = {
        0: 1, # at elwidth=0b00, the number of vector elements = 1
        1: 1, # at elwidth=0b01, the number of vector elements = 1
        2: 2, # at elwidth=0b10, the number of vector elements = 2
        3: 4, # at elwidth=0b11, the number of vector elements = 4
     }

**NOT** "the total number of bytes at elwdith=0b0 shall be one (1)

much as i love the documentation in layout_experiment.py, unfortunately
because it is an "all-or-nothing" commit, the entire lot needs
reversion (i'll handle it).

util.py can stay as it is.

i really do love the documentation in layout_experiment.py btw.
once reverted i'll see if i can extract it.

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