[Libre-soc-bugs] [Bug 731] potential design oversight in Partitioned SimdSignal Cat/Assign/etc lhs/rhs

[bugzilla-daemon at libre-soc.org]

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

--- Comment #8 from Jacob Lifshay <programmerjake at gmail.com> ---
(In reply to Luke Kenneth Casson Leighton from comment #0)
> Strategy / Incremental work plan:

reordering a bit for clarity:

> 2) incremental step of adding "mirror (trmporary) storage classes"

SwizzledSimdValue *is* that temporary storage class, it is just a little
different than what you envisioned here:
* use as rhs: its way of knowing when it's being used as the rhs is by trapping
other code's accesses of SwizzledSimdValue.sig, and lazily adding the submodule
that computes .sig's value. This way, no other code needs to operate
differently when using SwizzledSimdValue as a rhs, it looks like a completely
normal SimdSignal.
* use as lhs: it overrides __Assign__ so doesn't use PartitionedAssign, instead
returning a list of nmigen Assigns, therefore PartitionedAssign doesn't need to
be aware of SwizzledSimdValue at all.


> 
> 1) Unit test showing and confirming need for this work        TODO
>   a) first prototype
>   b) Cat     TODO
>   c) Part     TODO

iirc we weren't planning on supporting Part(...).eq(...), since, like Array,
it's very complex and not super necessary. SwizzledSimdValue doesn't implement
Part's functionality, because, if it did, it would basically devolve to a full
general turing-complete logic network, rather than a simple single-layer
bit-swizzle (different swizzle for each elwid).

If we need Part, we can have a rhs-only class (like PartitionedDynamicShift),
which should be good enough.

lhs Part is hard, as evidenced by nmigen's rtlil backend having to raise an
exception to convert each Part to a top-level rtlil switch statement:
raise here:
https://github.com/nmigen/nmigen/blob/177f1b2e40694da473fdb0d95f4f6b33f5ea12ab/nmigen/back/rtlil.py#L677
conversion to switch here:
https://github.com/nmigen/nmigen/blob/177f1b2e40694da473fdb0d95f4f6b33f5ea12ab/nmigen/back/rtlil.py#L785

>   d) Mux?     TODO

Mux(...).eq(...) doesn't work in nmigen, so we can remove that one from the
list:

Traceback (most recent call last):
  File "/data/data/com.termux/files/home/nmigen-eq-test.py", line 54, in
<module>
    m.d.comb += Mux(a, b, c).eq(d)
  File "/data/data/com.termux/files/home/nmigen/nmigen/hdl/dsl.py", line 38, in
__iadd__
......
    raise TypeError("Value {!r} cannot be used in assignments".format(self))
TypeError: Value (m (sig a) (sig b) (sig c)) cannot be used in assignments

>   e) Slice     TODO
> 3) Add recognition (isinstance) for mirrors in PartitionedAssign  TODO

unnecessary, as outlined above

> 4) swap over to mirror classes in partsig.py one at a time
>   a) Cat    TODO
>   b) Part   TODO
>   c) Mux?   TODO
>   d) Slice  TODO
> 5) create entirely new LHS Partitioned classes (or adapt existing ones)

We won't actually need any new classes, since SwizzledSimdValue is general
enough that Cat, Slice, and Repl *could* be implemented entirely (for both lhs
and rhs) by having __Cat__, __Slice__, etc. just construct a SwizzledSimdValue
instance:
kinda like (SimdSignal's method):
def __Cat__(self, *others):
    # convert to SwizzledSimdValue with same value
    retval = SwizzledSimdValue.from_simd_signal(self)

    # basically a wrapper around elwid, SwizzleKey can probably
    # be fully replaced with PartType once it grows more methods
    kinda_elwid = retval.swizzle_key

    # we construct the return value by Cat-ting SimdSignals one at a time,
    # this has no cost in complexity of the produced gates, since it
    # turns the whole sequence of Simd Cats into one layer of
    # non-simd muxes kinda like: Mux(elwid, Cat(bits...), Cat(...))
    for other in others:
        # convert from Value/SimdSignal to SwizzledSimdValue with
        # same value, splatting if needed
        other = SwizzledSimdValue.from_value(kinda_elwid, other)
        lhs_layout = get_layout(retval)
        rhs_layout = get_layout(other)
        layout = compute_cat_layout(retval, other)
        # dict of swizzles for each kinda_elwid value
        swizzles = {}
        for k in kinda_elwid.possible_values:
            # make `width` bit wide Swizzle initialized to const zeros
            swizzle = Swizzle.from_const(0, layout.width)
            # get lhs swizzle
            lhs = retval.swizzles[k]
            # get rhs swizzle
            rhs = other.swizzles[k]
            for lhs_el, rhs_el, el in zip(lhs_layout.elements(k),
                                          rhs_layout.elements(k),
                                          layout.elements(k)):
                # Cat the two lists of bits for this element, and
                # assign to slice of target Swizzle.bits.
                bits = lhs.bits[lhs_el.slice] + rhs.bits[rhs_el.slice]
                swizzle.bits[el.slice] = bits

                # above should probably be refactored into Swizzle.cat
                # and Swizzle.__setitem__ methods
            swizzles[k] = swizzle
        retval = SwizzledSimdValue(kinda_elwid, swizzles)
    return retval




> 6) update PartitionedAssign to cope with LHS   TODO

not necessary, as explained above

> 7) update partsig if necessary to new PartitionedAssign   TODO

not necessary, as explained above

> 8) run LHS unit tests, confirm functional     TODO

we retain step 8.

> note that the "mirror" classes for Cat, Part etc do not actually
> do anything, they act as simply temporary storage for the arguments
> passed to ast.Cat() etc pending encountering an Assign, at which
> point and ONLY at which point the missing information (LHS or
> RHS) is apparent.

That works great for LHS, however, for RHS use, you need the actual bits of a
Cat, Slice, etc. SimdSignal well before encountering a .eq() call.
SwizzledSimdValue handles that by trapping self.sig and lazily computing its
value if needed.

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