Skip to content
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

fix Chisel compile warnings in PseudoLRU.access(ways: Seq) #2378

Merged
merged 1 commit into from Apr 4, 2020

Conversation

ingallsj
Copy link
Contributor

Related issue: introduced by me in 9057b3c
Type of change: other enhancement (paying off technical debt)
Impact: no functional change
Development Phase: implementation

fix these Chisel compile warnings:

[warn] /rocket-chip/src/main/scala/util/Replacement.scala:46:39: match may not be exhaustive.
[warn] It would fail on the following inputs: (UInt(), _), (_, Valid()), (_, _)
[warn]     state_reg := ways.fold(state_reg) { case (prev: UInt, way: ValidIO[UInt]) =>
[warn]                                       ^
[warn] /rocket-chip/src/main/scala/util/Replacement.scala:46:64: non-variable type argument Chisel.UInt in type pattern chisel3.util.Valid[Chisel.UInt] (the underlying of Chisel.ValidIO[Chisel.UInt]) is unchecked since it is eliminated by erasure
[warn]     state_reg := ways.fold(state_reg) { case (prev: UInt, way: ValidIO[UInt]) =>
[warn]                                                                ^

@ingallsj ingallsj requested a review from ss2783 March 30, 2020 05:50
@aswaterman
Copy link
Member

The intent of the original code is clearer.

The match is only necessary because of the use of fold rather than foldLeft, but I think foldLeft is preferable anyway: we aren't trying to imply that different elaborations might produce different orderings. Try this instead:

def access(ways: Seq[ValidIO[UInt]]) {
  state_reg := ways.foldLeft(state_reg)((prev, way) => Mux(way.valid, get_next_state(prev, way.bits), prev))
}

@ingallsj
Copy link
Contributor Author

that seems to work, thank you for the magic incantation!

@ingallsj ingallsj merged commit 8e1f0c8 into master Apr 4, 2020
@ingallsj ingallsj deleted the warn-repl branch April 4, 2020 22:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants