[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 #21 from Luke Kenneth Casson Leighton <lkcl at lkcl.net> ---
(In reply to Jacob Lifshay from comment #18)
> Does it pass:
> 
> a = SimdSignal(...)
> b = SimdSignal(...)
> c = SimdSignal(...)
> d = SimdSignal(...)
> 
> cat = Cat(a, b)
> 
> m.d.comb += cat.eq(c)
> m.d.comb += d.eq(cat)

no, because there aren't any locations where that's used
(because the practice of multiple-use of expensive AST
expressions as python variables i learned very early on
results in duplicated HDL)

> I expect not, since you fell into the trap that each PartitionedCat would
> only ever be used on the lhs or the rhs, not both.

... not a trap per se: i entirely avoid the practice throughout
the entirety of the code and will, if i see it being used, request
that it be fixed as a severe bug (or be commented in exceptional
circumstances)

if it turns out to be necessary, there is a simple fix: identify
whether set_lhs_mode has been called or not, if it's an incompatible
{LR}HS, and if so, duplicate the expression prior to using it.

this is about 6-8 lines of code, not 400.

> Also, does it handle:
> a = SimdSignal(...)
> b = SimdSignal(...)
> c = SimdSignal(...)
> d = SimdSignal(...)
> 
> cat = Cat(a, b)
> 
> m.d.comb += Cat(cat, c).eq(d) # two stages of Cat

no, and i explained already that this is, again, something that
is not used (double-cat), because, again, i learned very quickly
(2 years ago) that using python assignments is not a good idea.

therefore i know that there's no code that does this,
therefore it is not needed right now,
therefore there is no point worrying about it and certainly not
getting into discussions that delay progress more than progress
has already been delayed by trying to work in version 2 code and
concepts into version 1.

> I also expect not, SwizzledSimdValue also correctly handles this case,
> because it is designed to handle assignments through any number of Cat and
> Slice calls.

that's great - if it were needed for version 1.0 which is the "pragmatic
get-it-done-as-quickly-as-possible and learn from the exercise" version.

i know what you're doing. you're solving problems ahead-of-time.  this
is what i used to do.  you feel, "this person isn't listening to what
i'm saying therefore i'm just going to go ahead and do it".

the problem with that approach is that things emerge retrospectively.
i didn't know that you'd designed something that solved issues i hadn't
even thought about or understood, yet.

an incremental approach on the other hand allows me to understand what
you are trying to communicate, in terms of what i already understand.

the two cases you've noted - if in the extremely unlikely event that they
are needed - can be fixed by adding the capability to the existing code.

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