[Libre-soc-dev] SimdSignal update

[lkcl]

https://git.libre-soc.org/?p=ieee754fpu.git;a=commitdiff;h=98b4968397c921fc1e104543240e4be5b29e1615

ok good, SimdHint and SimdMap are not used so no reversion is needed, that code can be ignored.

the VERY RUDIMENTARY operators add mul sub div need to be added to SimdShape

these are the ONLY operators that should be added.

if others turn out to be needed they can be added AS NEEDED.

a flag will be needed in SimdShape which records whether the constructor parameters had fixed_width set or whether elwidths were given.

(don't worry about both being set for now)

SimdShape.__add__ then checks that flag and performs an add on EITHER fixed_width OR elwidths as appropriate, and passes None for all parameters calculated by layout() to a new instance i.e. the return result.

this is dead simple and should IN NO WAY be over 20 lines of code to add the flag and an add operator.  the other operators sub mul div should likewise only be around 7-10 lines or 2 lines if calling a common function.

the entire diff should be absolutely no more than 40 lines, *please do not add more than that* without discussing it first, to explain why you believe it is necessary.

l.


On October 27, 2021 9:58:04 AM UTC, lkcl <luke.leighton at gmail.com> wrote:
>
>
>On October 27, 2021 9:32:05 AM UTC, Jacob Lifshay
><programmerjake at gmail.com> wrote:
>>I ended up somewhat distracted by other stuff, then I finished
>figuring
>>out
>>all of what you have done so far for SimdSignal and Cat, but by then
>it
>>was
>>rather late and Cat would likely take time, so I instead worked on
>>adding
>>fixed_width tracking to XLEN and adding the missing tests for util.py.
>>
>>Since XLEN-based arithmetic may end up with a
>>inconsistent/garbage/non-existant value for fixed_width, I decided to
>>name
>>it SimdWHintMap.width_hint instead.
>>
>>The idea is that SimdShape will check if width_hint looks sane and, if
>>not,
>>ignore it and use the width calculated from element widths like usual.
>
>unfortunately, that won't work.  you should have raised this before
>proceeding: now it needs reversion because it is a fundamental and
>unauthorised change to the API.
>
>when something is set to e.g. 64 bit fixed_width, it has to be actually
>and truly 64 bit.
>
>otherwise, as i said last week (twice): when trying to downcast and
>access the underlying Signal in order to get it out of the ALUs and
>into the regfile, the underlying signal could be completely wrong and
>contain arbitrary padding that of course, after downcasting, nmigen
>Signal has absolutely no clue about.
>
>additionally i also said, and am repeating once again, that if the
>fixed width is ignored when requested the resultant interaction between
>SimdSignals of different fixed_widths results in unnecessary Muxes
>being created even on things as basic as eq.
>
>this is why i did not engage in any discussions or write  any explicit
>words stating "this should be called a hint and completely ignored if
>we feel like it".
>
>if you recall, the discussions involved setting "priority" on whether
>fixed_width or elwidths were to be used.
>
>i.e. if a fixedwidth has been set, the fixedwidth is used in the
>calculations (multiply, divide, add etc).  Assertions can be raised if
>the corresponding elwidths do not match.
>
>this is complex which is *why i said keep it rudimentary*.
>
>if elwidths have been set then those take priority, and the overall
>width is dead easy to calculate, just pass the new elwifths to the
>layout() function, actually you don't even need to do that, because
>SimdShape when constructing a new return result will of course do that
>automatically.
>
>as i said in the meeting yesterday we are under extreme time pressure
>so please when i say "keep it rudimentary" please do that and do not
>add unnecessary operators, throw NotImplemented instead.
>
>so, please can you revert the unauthorised change to "hint", put it
>back to fixed_width, stick with the required API, and do *bare minimum*
>operators: add mul div sub rather than try to create operators that
>make no sense.
>
>l.