pushed dynamic configuration through all multiplier componentry#222
pushed dynamic configuration through all multiplier componentry#222desmonddak wants to merge 12 commits intointel:mainfrom
Conversation
|
It would be nice to generalize this to integer or boolean configurations. A simple test example I can use is a shift register (or the rotater) which would take either a Logic or an int for a shift input. |
| /// The actual shift in each row. This value will be modified by the | ||
| /// sign extension routine used when folding in a sign bit from another | ||
| /// row. | ||
| /// row. |
There was a problem hiding this comment.
typo: accidentally added duplicate "row." here?
| throw RohdHclException('multiplicand sign reconfiguration requires ' | ||
| 'signedMultiplicand=false'); | ||
| } | ||
| // if (signedMultiplicand && (selectSignedMultiplicand != null)) { |
| throw RohdHclException('multiplicand sign reconfiguration requires ' | ||
| 'signedMultiplicand=false'); | ||
| } | ||
| // if (signedMultiplicand && (selectSignedMultiplicand != null)) { |
| final Logic sign; | ||
| if (selectSignedMultiplicand != null) { | ||
| sign = mux(selectSignedMultiplicand!, addend.last, signs[row]); | ||
| if (signedMultiplicandParameter.dynamicConfig != null) { |
There was a problem hiding this comment.
I notice a lot of checking for != null, would be nice to have a bool get isDynamic helper?
|
|
||
| /// Factory constructor to create a [StaticOrDynamicParameter] instance from a | ||
| /// dynamic. | ||
| factory StaticOrDynamicParameter.ofDynamic(dynamic config) { |
There was a problem hiding this comment.
It seems like this the most commonly used constructor, yet the name is the longest and also a little weird (though I dont have a better suggestion, it seems appropriate) since "dynamic" is used in two meanings in the same call: a changing Logic and a dynamically typed variable.
What if we made this constructor just StaticOrDynamicParameter and the other one something like StaticOrDynamicParameter.withConfigs or something (maybe a better name)?
| if (selectSignedMultiplicand != null) { | ||
| sign = mux(selectSignedMultiplicand!, addend.last, signs[row]); | ||
| if (signedMultiplicandParameter.dynamicConfig != null) { | ||
| sign = mux(signedMultiplicandParameter.dynamicConfig!, addend.last, |
There was a problem hiding this comment.
what if we added a mux or select function into this StaticOrDynamicParameter that automatically handles the switch between using a mux or a ternary operator? this would only work when it's a 1-bit value.
also, mux will be updated (eventually) so that if the select is a Const, it will resolve the result statically. but it still might be worth adding it as an option on this infrastructure
| } | ||
| this.subtractIn = | ||
| (subtractIn != null) ? addInput('subtractIn', subtractIn) : null; | ||
| subtractIn = StaticOrDynamicParameter.ofDynamic(subtract).clone(this); |
2 participants
Description & Motivation
Completed the use of dynamic configuration of signed operands through the multiplier componentry.
Related Issue(s)
None.
Testing
Ran existing multiplier tests.
Backwards-compatibility
Yes. Various components now have simpler APIs for sign configuration of multiplicand and multiplier (and addend for MAC).
Documentation
API documentation fully updated. Markdown documentation TBD.