[datapath] Signed Partial Product Generation#9592
Merged
Conversation
Member
|
This is super cool!! Will try reviewing details tomorrow :) |
uenoku
approved these changes
Feb 5, 2026
Member
There was a problem hiding this comment.
Super cool!! LGTM! I believe this is so-called Baugh-Wooley Algorithm? In that case could you mention in the code? (it took some time to reach to this word :) I agree it's too hard to write test for this, I'll try to consider some way to test this kind of code :)
| // Note constant correction will depend on lhs and rhs widths - so general | ||
| // case is not twice the correction for one side. | ||
| auto ones = APInt::getAllOnes(inputWidth); | ||
| auto lowerLhs = APInt(inputWidth, (1 << lhsBaseWidth)); |
Member
There was a problem hiding this comment.
1 << lhsBaseWidth would overflow when > 32. Would be good to use getOneBitSet().
Suggested change
| auto lowerLhs = APInt(inputWidth, (1 << lhsBaseWidth)); | |
| auto lowerLhs = APInt::getOneBitSet(inputWidth, lhsBaseWidth); |
Contributor
Author
There was a problem hiding this comment.
Neat - did not know this existed :)
Contributor
Author
|
Thanks for the review @uenoku - have expanded the documentation (with pointers to the original Baugh-Wooley) and addressed other comments! |
Member
|
Thank you for adding comments! This is awesome! |
Arya-Golkari
pushed a commit
to Arya-Golkari/circt
that referenced
this pull request
Feb 7, 2026
* Signed multiplier optimizations + tests * Update comments * Simplify * Update comments * final updates * Address warnings * Address comments
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
We can greatly reduce the gate count when lowering signed multipliers (sext(a) * sext(b)) by simplifying the partial product generation.
Given a p-bit input a, and a q-bit input b, if we extend both to p+q bits, then we can simplify using the following:
sext(a) * sext(b) == a[p-2:0] * b[q-2:0] - 2^(p-1)*a[p-1]*b[q-2:0] - 2^(q-1)*a[p-2:0]*b[q-1] + 2^(p+q-2) a[p-1]*b[q-1]We can then use the identity: -x = ~x + 1, and perform a bunch of constant folding to really reduce the number of variable bits going into the compressor.
Currently implemented for equal width multipliers - need to add support for operands of different base widths.
The signed multiplier optimization is tested via LEC tests, within the integration tests - the code is overly verbose for a FileCheck test IMHO.