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

Potential Bug at DCache WriteArb #667

Open
Jerry-Tianchen opened this issue Nov 18, 2023 · 3 comments
Open

Potential Bug at DCache WriteArb #667

Jerry-Tianchen opened this issue Nov 18, 2023 · 3 comments

Comments

@Jerry-Tianchen
Copy link

Jerry-Tianchen commented Nov 18, 2023

Type of issue: bug report

Impact: new rtl

Development Phase: proposal

Other information

  def onReset = L1Metadata(0.U, ClientMetadata.onReset)
  val meta = Seq.fill(memWidth) { Module(new L1MetadataArray(onReset _)) }
  val metaWriteArb = Module(new Arbiter(new L1MetaWriteReq, 2))
  // 0 goes to MSHR refills, 1 goes to prober
  val metaReadArb = Module(new Arbiter(new BoomL1MetaReadReq, 6))
  // 0 goes to MSHR replays, 1 goes to prober, 2 goes to wb, 3 goes to MSHR meta read,
  // 4 goes to pipeline, 5 goes to prefetcher

  metaReadArb.io.in := DontCare
  for (w <- 0 until memWidth) {
    meta(w).io.write.valid := metaWriteArb.io.out.fire
    meta(w).io.write.bits  := metaWriteArb.io.out.bits
    meta(w).io.read.valid  := metaReadArb.io.out.valid
    meta(w).io.read.bits   := metaReadArb.io.out.bits.req(w)
  }
  metaReadArb.io.out.ready  := meta.map(_.io.read.ready).reduce(_||_)
  metaWriteArb.io.out.ready := meta.map(_.io.write.ready).reduce(_||_)

  // data
  val data = Module(if (boomParams.numDCacheBanks == 1) new BoomDuplicatedDataArray else new BoomBankedDataArray)
  val dataWriteArb = Module(new Arbiter(new L1DataWriteReq, 2))
  // 0 goes to pipeline, 1 goes to MSHR refills
  val dataReadArb = Module(new Arbiter(new BoomL1DataReadReq, 3))
  // 0 goes to MSHR replays, 1 goes to wb, 2 goes to pipeline
  dataReadArb.io.in := DontCare

For the metaWriteArb.io.out.ready := meta.map(_.io.write.ready).reduce(_||_)
are you sure it is really reduce (||) instead of reduce (&&)?
When writing aren't we supposed to write both Array Blocks?

I realized this when analyzing the code.

What is the current behavior?

What is the expected behavior?

Please tell us about your environment:
Not Important

What is the use case for changing the behavior?

@jerryz123
Copy link
Contributor

I think you are right, I suspect for some reason we never entered a condition where this mattered, but I think your suggestion is more correct.

@jerryz123
Copy link
Contributor

Feel free to open a PR with the fix

@Jerry-Tianchen
Copy link
Author

Oh I am working on older version right now. Once I have the time to look at the newest version I will do that~

Alenkruth added a commit to Alenkruth/riscv-boom that referenced this issue Jan 8, 2024
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

No branches or pull requests

2 participants