-
Notifications
You must be signed in to change notification settings - Fork 603
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[stevo]: shift register enable signal now enables ALL registers in th… #370
Conversation
shouldn't the enable be pipelined down, rather than simply fanning out to all of the stages? |
But then "disabling" the shift register just causes the value in the first register to shift down, filling all the registers, as is currently the case. When would this be the desired behavior? My intended use of the enable signal is to enable or disable the shifting for a cycle. |
I see your point. I guess what I'm describing is Pipe, not ShiftRegister. |
Why not also remove the n == 1 case now that this recurses more regularly? |
Agreed. |
This is an unacceptable change to the Chisel API. In the future please adhere to a deprecation schedule. |
In retrospect, I completely agree, @ccelio. Sorry about that. |
Bumps [chiseltest](https://github.com/ucb-bar/chisel-testers2) from `5ede0ae` to `5e81249`. - [Release notes](https://github.com/ucb-bar/chisel-testers2/releases) - [Commits](ucb-bar/chiseltest@5ede0ae...5e81249) --- updated-dependencies: - dependency-name: chiseltest dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
…e chain.
Previously, the enable signal only worked on the first register. Later ones still shifted every cycle. Now they are all controlled by en.
This also fixes the problem in PR #355, so I'll close that one in favor of this one.