search

ucb-bar / dsptools

A Library of Chisel3 Tools for Digital Signal Processing
Apache License 2.0
224 stars 38 forks source link

Make DspComplex multiplies use growing addition. #157

Closed grebe closed 4 years ago

grebe commented 5 years ago

DspComplex relies on using the underlying Ring's multiplication. For UInt, SInt, FixedPoint, etc. these additions wrap instead of grow. For our implementation of complex multiplication, growing is more desirable than wrapping.

This commit adds specialized versions of the DspComplex for UInt, SInt, FixedPoint, etc. that use growing addition.

This is in response to an issue @mkosunen and @yuedai14 were running into that they were working around by increasing input bitwidth.

@shunshou, what do you think?

shunshou commented 5 years ago

Fine by me. It would be nice if Firrtl could throw a warning when the output of some chain of multiplications grew to have too many bits (sanity check in case people generated waste fully large intermediate terms)

On Wednesday, April 24, 2019, Paul Rigge notifications@github.com wrote:

DspComplex relies on using the underlying Ring's multiplication. For UInt, SInt, FixedPoint, etc. these additions wrap instead of grow. For our implementation of complex multiplication, growing is more desirable than wrapping.

This commit adds specialized versions of the DspComplex for UInt, SInt, FixedPoint, etc. that use growing addition.

This is in response to an issue @mkosunen https://github.com/mkosunen and @Yuedai14 https://github.com/Yuedai14 were running into that they were working around by increasing input bitwidth.

@shunshou https://github.com/shunshou, what do you think?

You can view, comment on, or merge this pull request online at:

https://github.com/ucb-bar/dsptools/pull/157 Commit Summary

  • Make DspComplex multiplies use growing addition.

File Changes

Patch Links:

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ucb-bar/dsptools/pull/157, or mute the thread https://github.com/notifications/unsubscribe-auth/ABSNGFQ35CDBCXMNEFY2AILPSD6IZANCNFSM4HII5NQA .

shunshou commented 5 years ago

Hm actually my recommendation was always to use context_(op) and have people explicitly define behavior.

On Wed, Apr 24, 2019 at 11:22 PM Angie shunshou@gmail.com wrote:

Fine by me. It would be nice if Firrtl could throw a warning when the output of some chain of multiplications grew to have too many bits (sanity check in case people generated waste fully large intermediate terms)

On Wednesday, April 24, 2019, Paul Rigge notifications@github.com wrote:

DspComplex relies on using the underlying Ring's multiplication. For UInt, SInt, FixedPoint, etc. these additions wrap instead of grow. For our implementation of complex multiplication, growing is more desirable than wrapping.

This commit adds specialized versions of the DspComplex for UInt, SInt, FixedPoint, etc. that use growing addition.

This is in response to an issue @mkosunen https://github.com/mkosunen and @Yuedai14 https://github.com/Yuedai14 were running into that they were working around by increasing input bitwidth.

@shunshou https://github.com/shunshou, what do you think?

You can view, comment on, or merge this pull request online at:

https://github.com/ucb-bar/dsptools/pull/157 Commit Summary

  • Make DspComplex multiplies use growing addition.

File Changes

Patch Links:

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ucb-bar/dsptools/pull/157, or mute the thread https://github.com/notifications/unsubscribe-auth/ABSNGFQ35CDBCXMNEFY2AILPSD6IZANCNFSM4HII5NQA .