booth cleanup
#3938
booth cleanup
#3938
Conversation
To represent intermediate signals use the `SigBit`/`SigSpec` classes as is customary in the Yosys codebase. Do not pass around `Wire` pointers unless we have special reason to.
For the basic single-bit operations, opt for gate cells (`$_AND_` etc.) instead of the coarse cells (`$and` etc.). For the emission of cells move to the conventional module methods (`module->addAndGate`) away from the local helpers. While at it, touch on the surrounding code.
Use the `addFa` helper, do not misuse `new_id` and make other changes to the transformation code.
povik
force-pushed
the
booth-cleanup
branch
from
059a854 to
91bcf81
Compare
|
@andyfox-rushc Take a look, feel free to ask and/or object. |
passes/techmap/booth.cc
Outdated
| log_assert(s_vec.size() == c_vec.size()); | ||
| log_assert(c_vec.size() == s_vec.size()); | ||
| // TODO: doesn't pass | ||
| //log_assert(result.size() == s_vec.size() + 2); |
There was a problem hiding this comment.
@andyfox-rushc Here I moved the result.size() == s_vec.size() + 2 condition down from the "axioms" comment to an actual assert, but it wasn't passing. Should this be upheld?
There was a problem hiding this comment.
Hi, Martin,
My comment is wrong and the axiom you refer to does not need to be upheld. The axiom for the unsigned case should be result.size() ==s_vec.size().
In this unsigned case an array of CSA (Wallace) trees is used: 1 per result bit. Each CSA tree produces a sum and carry. These are summed up by the CPA. The final carry out is ignored in the unsigned case. My error comes from the use of the same structure for signed multiplication, in which case the multiplier is not padded out and one partial product removed which leads to the different axiom for the size. But again my comment was wrong for the unsigned case.
There was a problem hiding this comment.
Thanks for the analysis! I wanted to make sure there's not an issue lurking there.
There was a problem hiding this comment.
PS Martin, sorry for being quiet. I did quickly go through all your changes which look good to me. I like the addition of the full adder helper creator to RTLIL. The simplifications to the cell selection and the extension of the arguments to fit the right size (I was adding weird buffers), all seem to make the code much clearer to me. I see these all got safely merged in today. All looks great to me. Thanks.
Fix assertion to what it should be per Andy's comments.
Cut the test down from taking ~25 s to ~3 s.
Following up to #3924, this PR will to some degree clean up the code of the Booth pass.