[libre-riscv-dev] [Bug 132] SIMD-like nmigen signal for partitioning

[bugzilla-daemon at libre-riscv.org]

http://bugs.libre-riscv.org/show_bug.cgi?id=132

--- Comment #31 from Luke Kenneth Casson Leighton <lkcl at lkcl.net> ---
(In reply to Jacob Lifshay from comment #30)

> I actually think it may be better to refactor the code to use Mux instead of
> m.If and m.Else:

interesting. why? because, ultimately, yosys "proc" and "opt" end up replacing
m.If/Else with Muxes anyway.

> 1. we won't have to modify nmigen's Module

i was thinking along the lines of a derived class (or a wrapper that takes
Module as an argument).

> 2. it appears as though we have been subconsciously thinking that m.If
> somehow makes the logic inside the m.If less expensive when the condition is
> dynamically disabled. Having an explicit Mux may fix that.

i wasn't thinking :)

here's a simple example:

-        with m.If(self.i.product[-1]):
-            comb += p.eq(self.i.product)
-        with m.Else():
-            # get 1 bit of extra accuracy if the mantissa top bit is zero
-            comb += p.eq(self.i.product<<1)
-            comb += self.o.z.e.eq(self.i.z.e-1)
+        msb = Signal(reset_less=True)
+        e = self.o.z.e
+        comb += msb.eq(self.i.product[-1])
+        comb += p.eq(Mux(msb, self.i.product, self.i.product<<1))
+        comb += e.eq(Mux(msb, self.i.z.e, self.i.z.e-1))

that would do alright, wouldn't it? 

my only concern is: where logically, the actions used to be "grouped"
together (the "If" things are grouped together), now they're separated.

> 3. we will need to review and, if needed, rewrite code anyway when
> converting to partitioned form. rewriting to Mux may help us avoid missing
> any code.

yes, as "grep Mux" would provide the identification-points.

for example, the msb above now clearly needs to be a "multi-partitioned"
Signal.

> for part 3, we will need PartitionedValue to not be derived from nmigen's
> Value to allow nmigen to catch mixups.

hmmm hm hm hm... maaybe.

> It may also be a good idea to use a slightly different name for Mux since
> they are different operations, how about MuxP for Mux-partitioned? that
> would allow using a free function/class instead of a member.

yehyeh, that's a good idea.

sigh this is a big change, which can't be tested (live) without actually
committing entirely to it.

can you think of a way to gain some confidence that this is going to work,
*before* going ahead with the whole lot?

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