Skip to content

Fix/false load-use stall by gating rs1 amd rs2 hazard check #8

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.

Sign up for GitHub

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

Open
jgw0915 wants to merge 1 commit into
base: main
Choose a base branch
from
+19 −0
Open

Fix/false load-use stall by gating rs1 amd rs2 hazard check #8

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions 3-pipeline/src/main/scala/riscv/core/fivestage_final/CPU.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,8 @@ class CPU extends Module {
ctrl.io.rd_ex := id2ex.io.output_regs_write_address
ctrl.io.memory_read_enable_mem := ex2mem.io.output_memory_read_enable
ctrl.io.rd_mem := ex2mem.io.output_regs_write_address
ctrl.io.uses_rs2_id := id.io.uses_rs2_id
ctrl.io.uses_rs1_id := id.io.uses_rs1_id

regs.io.write_enable := mem2wb.io.output_regs_write_enable
regs.io.write_address := mem2wb.io.output_regs_write_address
Expand Down
5 changes: 5 additions & 0 deletions 3-pipeline/src/main/scala/riscv/core/fivestage_final/Control.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ class Control extends Module {
val rd_ex = Input(UInt(Parameters.PhysicalRegisterAddrWidth)) // id2ex.io.output_regs_write_address
val memory_read_enable_mem = Input(Bool()) // ex2mem.io.output_memory_read_enable //
val rd_mem = Input(UInt(Parameters.PhysicalRegisterAddrWidth)) // ex2mem.io.output_regs_write_address //
val uses_rs1_id = Input(Bool()) // true only if current ID instruction really reads rs1
val uses_rs2_id = Input(Bool()) // true only if current ID instruction really reads rs2

val if_flush = Output(Bool())
val id_flush = Output(Bool())
Expand All @@ -84,6 +86,9 @@ class Control extends Module {
// 1. Load-use hazard: Load result used immediately by next instruction
// 2. Jump-related hazard: Jump instruction needs register value not ready
// 3. Control hazard: Branch/jump instruction changes PC
// Use the decoded `uses_rs1_id` / `uses_rs2_id` flags to test whether the
// current ID-stage instruction actually reads rs1/rs2; only then should a
// register-number match be considered a true RAW dependency.
//
// Control signals:
// - pc_stall: Freeze PC (don't fetch next instruction)
Expand Down
12 changes: 12 additions & 0 deletions 3-pipeline/src/main/scala/riscv/core/fivestage_final/InstructionDecode.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,13 +152,25 @@ class InstructionDecode extends Module {
val clint_jump_address = Output(UInt(Parameters.AddrWidth)) // clint.io.jump_address
val if_jump_flag = Output(Bool()) // ctrl.io.jump_flag , inst_fetch.io.jump_flag_id
val if_jump_address = Output(UInt(Parameters.AddrWidth)) // inst_fetch.io.jump_address_id
val uses_rs1_id = Output(Bool()) // tells Control/Forwarding whether rs1 is valid for this instruction
val uses_rs2_id = Output(Bool()) // tells Control/Forwarding whether rs2 is valid for this instruction
})
val opcode = io.instruction(6, 0)
val funct3 = io.instruction(14, 12)
val funct7 = io.instruction(31, 25)
val rd = io.instruction(11, 7)
val rs1 = io.instruction(19, 15)
val rs2 = io.instruction(24, 20)
val usesRs2 = (opcode === InstructionTypes.RM) ||
(opcode === InstructionTypes.S) ||
(opcode === InstructionTypes.B)

val usesRs1 = !(opcode === Instructions.jal) &&
!(opcode === Instructions.lui) &&
!(opcode === Instructions.auipc)
Comment on lines +164 to +170
Copy link
Contributor

@jserv jserv Dec 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mixed camelCase and snake_case:

  val usesRs2 = ...  // camelCase                                                                                                                      
  io.uses_rs2_id := usesRs2  // snake_case

Should be consistent - use snake_case per project conventions:

  val uses_rs2 = ...                                                                                                                                   
  io.uses_rs2_id := uses_rs2

Comment on lines +168 to +170
Copy link
Contributor

@jserv jserv Dec 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing edge cases:

  • CSR immediate variants (csrrwi, csrrsi, csrrci) use zimm[4:0] in rs1 field, not actual rs1
  • fence instruction doesn't use rs1 for data dependency purposes

Suggested fix:

  val usesRs1 = !(opcode === Instructions.jal) &&
                !(opcode === Instructions.lui) &&
                !(opcode === Instructions.auipc) &&
                !(opcode === Instructions.fence) &&
                !(opcode === Instructions.csr &&
                  (funct3 === InstructionsTypeCSR.csrrwi ||
                   funct3 === InstructionsTypeCSR.csrrsi ||
                   funct3 === InstructionsTypeCSR.csrrci))


io.uses_rs2_id := usesRs2
io.uses_rs1_id := usesRs1

io.regs_reg1_read_address := Mux(opcode === Instructions.lui, 0.U(Parameters.PhysicalRegisterAddrWidth), rs1)
io.regs_reg2_read_address := rs2
Expand Down