[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 #13 from Luke Kenneth Casson Leighton <lkcl at lkcl.net> ---
i added a very simple unit test:

    m.d.comb += o.eq(Cat(a, b))

and some quick investigation "debug prints":

     def __Cat__(self, *args, src_loc_at=0):
+        print ("partsig cat", self, args)
     def __Assign__(self, val, *, src_loc_at=0):
+        print ("partsig assign", self, val)

@@ -75,6 +75,7 @@ class PartitionedAssign(Elaboratable):
     def elaborate(self, platform):
+        print ("PartitionedAssign start")
...
...
+        print ("PartitionedAssign end")

etc. which reveals:

partsig cat <ieee754.part.partsig.SimdSignal object at 0x7f0e88f28400>
(<ieee754.part.partsig.SimdSignal object at 0x7f0e88e697b8>,)
partsig assign <ieee754.part.partsig.SimdSignal object at 0x7f0e88e76128>
<ieee754.part.partsig.SimdSignal object at 0x7f0e88e76470>
PartitionedCat start
PartitionedCat end
PartitionedAssign start
PartitionedAssign end

which is kinda as expected (but a relief to confirm) that elaborate()
is called **AFTER** submodule instantiation.

therefore, the opportunity to call a function "set_lhs()" exists
inside SimdSignal.__Assign__(), performing intrusive "tree-walking"
or basic one-level inspection of the contents of its inputs, and
adding the necessary parameter which alters the behaviour from
(default RHS) to LHS.

i have absolutely no problem at all with a truly dreadful hack of
adding in __something variables by PCat (and PSlice) which tell
the output that it came from a submodule.

def PCat(m, arglist, ctx):
    from ieee754.part_cat.cat import PartitionedCat # avoid recursive import
    global modcount
    modcount += 1
    pc = PartitionedCat(arglist, ctx)
    setattr(m.submodules, "pcat%d" % modcount, pc)
    pc.output.__trulyterriblehack = pc             <----- hack here
    return pc.output

thus, in SimdSignal.__Assign__ it may go, "oh, does lhs have a member
variable __trulyterriblehack? so let's call lhs.__trulyterriblehack.set_lhs()"

the behaviour of PartitionedCat may therefore SWITCH to one of LHS
rather than RHS.

also, *after* the set_lhs() function is called, lengths of signals at that
time may be adapted / adjusted, such that PartitionedAssign can ascertain
the lengths of the sub-signals, solving what i *think* you might have been 
referring to in comment #9

this should not cause any "damage" and is a minimal incremental change
satisfying the Project Development Practices.

that it is absolutely truly dreadful and will make anyone who sees it
scream if their eyes do not melt first is completely irrelevant :)

hack first [and for goodness sake document it], clean up later.


commit 994d0c3f3d549b1aaa71f64277ed589098c15405 (HEAD -> master, origin/master)
Author: Luke Kenneth Casson Leighton <lkcl at lkcl.net>
Date:   Thu Oct 21 13:40:29 2021 +0100

    add quick print statements to show that elaborate() gets called as a
    second phase after the creation of the AST tree
    this gives a window of opportunity to tree-walk and set whether SimdSignals
    are LHS or RHS as determined by encountering SimdSignal.__Assign__

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