[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 #22 from Luke Kenneth Casson Leighton <lkcl at lkcl.net> ---
(In reply to Jacob Lifshay from comment #20)

a quick readthrough:

> def layout(elwid, part_counts, lane_shapes):

using the FP example dict from comment #2, i think you might have
misunderstood:
the idea is not to create a set of *separate* PartitionPoints
one per elwidth, the idea is to create a *merged* set of PartitionPoints
(and associated Signal mask)

normally PartitionPoints has to take a mask as input,
thus, we need a function that calculates the length of
that mask (see make_partition2 as an example)


>     if not isinstance(lane_shapes, Mapping):
>         lane_shapes = {i: lane_shapes for i in part_counts}
>     lane_shapes = {i: Shape.cast(lane_shapes[i]) for i in part_counts}
>     signed = lane_shapes[0].signed
>     assert all(i.signed == signed for i in lane_shapes.values())

by removing signed all of these lines can go.  this reduces the function
by 25%.   lane_shapes can become a straight dict containing
numbers, Shape can dral with the signed argument.

>     part_wid = -min(-lane_shapes[i].width // c for i, c in
> part_counts.items())
>     part_count = max(part_counts.values())

unfortunately this lwts the function determine the lwngth
of the underlying Signal, which is where i think you are getting
the mistaken impression that it is impossible to use PartitionedSignal
from.

the overall width of the *full* signal should be passed in as
an argument.

>     width = part_wid * part_count

here.

width needs to an input parameter *to* layout(), not a
computed quantity, where part_wid is instead calculated
by division.

*then* Signals will end up at uniform predetermined lengths
and *then* straight assignment (wire-only  no muxes) can occur

if the underlying signal ends up varying by arbitrary lengths
determined by the pieces used, of *course* you can't wire them
up safely because tbey're all different total sizes.

fixing the length makes the power 2 alignment sizes all the same,
problem solved.

>     points = {}
>     for i, c in part_counts.items():
>         def add_p(p):
>             points[p] = points.get(p, False) | (elwid == i)

this needs to allocate the actual mask Signal, i'm not totally
sure how, with the length not yet being determined, hm.

>         for start in range(0, part_count, c):
>             add_p(start * part_wid) # start of lane
>             add_p(start * part_wid + lane_shapes[i].width) # start of padding
>     points.pop(0, None)
>     points.pop(width, None)

ok now the length is known, now a Signal(len(points))
can be created, then a bit each dropped into the values
in sequence order of its keys, end result just like
make_partition2

>     return (PartitionPoints(points), Shape(width, signed), lane_shapes,
>         part_wid, part_count)

Shape() is not needed because width is a given and signed is
not layout()'s responsibility.

here also the extra dict needs to be returned which gives
the options, "if you want elwidth=0b00, this is the
value you have to set in the PartitionPoints mask
to get that to happen"

> 
> part_counts = {
>     0: 1,
>     1: 1,
>     2: 2,
>     3: 4,
> }

ok yeah, so you went with elwidth => num_of_partitions, cool.

> for i in range(4):
>     l = {0: signed(5), 1: signed(6), 2: signed(12), 3: signed(24)}
>     pprint((i, layout(i, part_counts, l)))

and a straight dict for length of subsignals (excl padding).
minus the signed() it is real dead simple.

the only thing being as i said, it produces different
overall widths depending on the input lane_shape which
is a showstopper.

if overall width (64 bit) is given as the input it fixes that.

does mean a safety check is needed, elwidth=0b11 means
4 partitions, 4 partitions of 24 bit obviously won't fit
in 64 bit!

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