[libre-riscv-dev] [Bug 120] implement RISC-V FSGNJ instruction

[bugzilla-daemon at libre-riscv.org]

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

--- Comment #22 from Luke Kenneth Casson Leighton <lkcl at lkcl.net> ---
(In reply to Michael Nolan from comment #20)
> > correct.  the partition mask will at some point be passed in through a
> > Mix-In class such that there should be very little in the way of changes.
> 
> That's pretty neat!

yeah :) testing's going to be a pig.  far more cases... *sigh* :)

> > the block "proc group_0" is where the switch/case statement is, which
> > needs replacing with some Mux()es.  those will basically be:
> 
> >   Mux(op[1],
> >       Mux(op[0], 0b11 calculation, 0b10 calculation),
> >       Mux(op[0], 0b01 calculation, 0b00 calculation)
> >      )
> 
> I added something like this on my latest commit. However I defined the
> behavior of opcode 0b11 to be the same as 0b10 (Saving 1 mux in the
> process). Since the RISCV spec doesn't define the behavior of opcode 0b11,
> this seems safe to do (right? Rocket does it)

yehyeh makes perfect sense to me.

okaaay, so this gets interesting.  take a look at the second
screenshot (after doing a git pull and re-run the unit tests
you can generate it yourself if you prefer).

http://bugs.libre-riscv.org/attachment.cgi?id=20

can you see that there's some spurious inexplicable maths
going on, there, compared to the first graphviz?

http://bugs.libre-riscv.org/attachment.cgi?id=19

that extra "stuff" is because inside the FPNumDecode, for
convenience i got it to subtract the "offset" that's part
of an exponent, and the "create()" function adds it back on.

so if you do yosys "show sc_decode_a" you'll find a whole stack
of extra stuff, none of which is actually necessary.

we know it's not necessary because the previous version - without
FPNumDecode - worked perfectly well and pass all unit tests.

this is why i emphasise in https://libre-riscv.org/HDL_workflow/
that it's essential to run the yosys "show" command as part of the
workflow.  you get to see the actual graph - the actual gates
created - and go "oink?? what's that doing in there??"

:)


second (minor) thing, you'd created a Signal "sign" and then had
immediately overwritten it with a python variable *also* named
"sign" which was the result of the Mux().

this seems to be a common source of confusion in nmigen, i've seen
this done before: a python variable is created and then overwritten.
remember that nmigen creates ASTs (abstract syntax trees) and so
anything added to m.d.comb (or m.d.sync) is completely unused.


third thing: again, which i spotted only after looking at the
graphviz: Mux(opcode[0], ~b.s, b.s) was extremely puzzling
in the graph however is quite logical in the code.

i thought about it and realised that the Mux could be replaced with
an XOR gate.  sign = opcode[0] ^ ~b.s

however that's really not very obvious so i provided a (terse)
explanatory comment.


i'd suggest getting rid of FPNumDecode, and returning to b[-1] etc.
can i leave that with you?

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