added functionality for always taken and dynamic branch prediction #15

ghost · 2019-02-18T23:00:11Z

No description provided.

powerjg

This is a good first pass, but there's a lot that needs to be improved here before we can give it to the students.

One big thing: Could you print out the dinocpu pipeline diagram and write on it to show your new pipeline so I can understand this code? Without a diagram this is hard to understand. Plus, we have to give them a diagram anyway.

Also, I have you worked on the local history predictor?

It would have been best to split this into multiple commits. 1) Add branch always taken predictor with options to use that or always not taken 2) add global history predictor 3) add local history predictor.

The global history predictor object needs a lot more documentation both so the students can implement it and so I can understand the code.

Sorry if my review is short. I'm super busy working on a grant right now :).

powerjg · 2019-02-18T23:53:03Z

src/main/scala/pipelined/cpu.scala

@@ -97,6 +102,7 @@ class PipelinedCPU(implicit val conf: CPUConfig) extends Module {
  val branchAdd  = Module(new Adder())
  val forwarding = Module(new ForwardingUnit())  //pipelined only
  val hazard     = Module(new HazardUnit())      //pipelined only
+  val GlobalPred = Module(new GlobalPredictor())


follow the style of other variable names. This should be globalPred with the first letter not capitalized.

powerjg · 2019-02-18T23:53:17Z

src/main/scala/pipelined/cpu.scala

@@ -115,19 +121,52 @@ class PipelinedCPU(implicit val conf: CPUConfig) extends Module {
  // For wb back to other stages
  val write_data = Wire(UInt())

+  val predict_taken = Wire(UInt())
+  val branchins     = Wire(UInt())
+  val PCPlus4       = Wire(UInt())


Ditto style.

powerjg · 2019-02-18T23:53:33Z

src/main/scala/pipelined/cpu.scala

@@ -115,19 +121,52 @@ class PipelinedCPU(implicit val conf: CPUConfig) extends Module {
  // For wb back to other stages
  val write_data = Wire(UInt())

+  val predict_taken = Wire(UInt())
+  val branchins     = Wire(UInt())


What is a "branchins"?

powerjg · 2019-02-18T23:54:12Z

src/main/scala/components/global-predictor.scala

+import chisel3.util._
+
+
+class GlobalPredictor(implicit val conf: CPUConfig) extends Module {


Please include lots of detailed documentation on each of these I/O wires. I can only guess what they mean.

src/main/scala/components/global-predictor.scala

powerjg · 2019-02-18T23:58:22Z

src/main/scala/pipelined/cpu.scala

@@ -148,6 +187,33 @@ class PipelinedCPU(implicit val conf: CPUConfig) extends Module {
    if_id.pcplusfour  := 0.U
  }

+  // Connect the branchAdd unit
+  //immGen.io.instruction := io.imem.instruction


This seems weird to comment out.

powerjg · 2019-02-18T23:58:34Z

src/main/scala/pipelined/cpu.scala

+  // Connect the branchAdd unit
+  //immGen.io.instruction := io.imem.instruction
+  branchAdd.io.inputx := pc
+  when (io.imem.instruction(6,0) === "b1100011".U) { 


Huh? Can you not use the output from the control for this logic?

powerjg · 2019-02-18T23:59:10Z

src/main/scala/pipelined/cpu.scala

@@ -171,6 +237,8 @@ class PipelinedCPU(implicit val conf: CPUConfig) extends Module {
  // Send the instruction to the immediate generator
  immGen.io.instruction := if_id.instruction

+


extra newlines?

powerjg · 2019-02-18T23:59:35Z

src/main/scala/pipelined/cpu.scala

@@ -268,8 +340,8 @@ class PipelinedCPU(implicit val conf: CPUConfig) extends Module {
  alu.io.operation := aluControl.io.operation

  // Connect the branchAdd unit
-  branchAdd.io.inputx := id_ex.pc
-  branchAdd.io.inputy := id_ex.imm
+  //branchAdd.io.inputx := id_ex.pc


delete if unneeded!

powerjg · 2019-02-19T00:00:11Z

src/main/scala/pipelined/cpu.scala

@@ -280,17 +352,38 @@ class PipelinedCPU(implicit val conf: CPUConfig) extends Module {
  ex_mem.wbcontrol  := id_ex.wbcontrol

  // Calculate whether which PC we should use and set the taken flag (line 92 in single-cycle/cpu.scala)
-  when (branchCtrl.io.taken || id_ex.excontrol.jump === 2.U) {
-    ex_mem.nextpc := branchAdd.io.result
+  when ((id_ex.predict_taken === 0.U && branchCtrl.io.taken && id_ex.instruction(6,0) === "b1100011".U) || id_ex.excontrol.jump === 2.U) {


Why do you have to use the instruction bits like this. Can't you use the control information???

powerjg · 2019-02-21T18:45:10Z

Just thought of something else here: Let's make this be another file: pipelined-opt/cpu.scala and the name of the new CPU design be PipelinedOptCPU (for optimized).

The main difference for the optimized CPU will be that (most of) the branch resolution will be done in the decode stage instead of memory.

powerjg

Filipe, please update the comments before our meeting tomorrow as specified below.

I'm disappointed in the code created here. I'm going to have to re-do all of this to make it understandable and usable by the students. It would be most helpful if you could update the code with some comments so I can understand what's going on.

powerjg · 2019-02-24T22:39:03Z

src/main/scala/components/branch-predictor.scala

+//n: history bits
+
+class BranchPredictor(l:Int, m:Int, n:Int, typepredictor:String) extends Module {
+  val io = IO(new Bundle {


Please document this IO.

powerjg · 2019-02-24T22:39:45Z

src/main/scala/components/branch-predictor.scala

+
+  val regs           = Reg(Vec(1<<(m+n), UInt(l.W)))
+  val history        = Reg(UInt(n.W))
+  val branch_address = Wire(UInt((m+n).W))


What is this wire?

Will add comments explaining.

powerjg · 2019-02-24T22:40:04Z

src/main/scala/components/branch-predictor.scala

+    val prediction       = Output(UInt(1.W))
+  })
+
+  val regs           = Reg(Vec(1<<(m+n), UInt(l.W)))


Please make comments on what each of the variables are used for. What

powerjg · 2019-02-24T22:41:18Z

src/main/scala/pipelined/cpu.scala

-    val wbcontrol = new WBControl
-    val rs1       = UInt(5.W)    //pipelined only
-    val rs2       = UInt(5.W)    //pipelined only
+    val prediction   = Bool()


Why did this change so much? Was it just a whitespace change (which should not be in the diff) or am I missing something?

Its a whitespace change.

powerjg · 2019-02-24T22:42:28Z

src/main/scala/pipelined/cpu.scala

-  } .elsewhen (id_ex.excontrol.jump === 3.U) {
-    ex_mem.nextpc := alu.io.result & Cat(Fill(31, 1.U), 0.U)
-    ex_mem.taken  := true.B
+  val branchPredictorControl = Wire(UInt(2.W))


Wow! What is going on here??

If this logic really is this complicated, then it shouldn't be in the CPU, it should be in its own module. However, I don't understand at all what this logic is doing.

Please add significant documentation to explain what is going on here.

I will add comments explaining.

powerjg · 2019-02-24T22:50:44Z

src/main/scala/pipelined/cpu.scala

  // From memory back to fetch. Since we don't decide whether to take a branch or not until the memory stage.
  val next_pc      = Wire(UInt())

  // For wb back to other stages
-  val write_data = Wire(UInt())
+  val write_data           = Wire(UInt())
+  val counter              = Reg(UInt(32.W))


What is this counter?

the counter is a reg i kept to count the number of branch mispredictions.

Signed-off-by: Jason Lowe-Power <[email protected]>

ghost requested a review from powerjg February 18, 2019 23:00

powerjg reviewed Feb 19, 2019

View reviewed changes

powerjg reviewed Feb 24, 2019

View reviewed changes

Remove code for lab 4

5bc1d33

Signed-off-by: Jason Lowe-Power <[email protected]>

powerjg force-pushed the lab4 branch from 257e093 to 5bc1d33 Compare March 4, 2019 00:22

powerjg added 7 commits March 5, 2019 09:35

Merge branch 'master' into lab4

8b76980

Fix lab 4 template to run tests

f3532c9

Signed-off-by: Jason Lowe-Power <[email protected]>

Update build.sbt to only run lab4 tests

8602878

Signed-off-by: Jason Lowe-Power <[email protected]>

Update saturating counter bits to 2

f724f1f

Signed-off-by: Jason Lowe-Power <[email protected]>

Merge branch 'master' into lab4

f9d7a9c

Merge branch 'master' into lab4

7ff0cbb

Merge branch 'master' into lab4

26273d1

powerjg closed this Mar 21, 2019

powerjg deleted the lab4 branch March 21, 2019 17:09

powerjg mentioned this pull request Jul 31, 2019

Integrate combin memory into pipelined and single-cycle CPUs #82

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

added functionality for always taken and dynamic branch prediction #15

added functionality for always taken and dynamic branch prediction #15

ghost commented Feb 18, 2019

powerjg left a comment

powerjg Feb 18, 2019

powerjg Feb 18, 2019

powerjg Feb 18, 2019

powerjg Feb 18, 2019

powerjg Feb 18, 2019

powerjg Feb 18, 2019

powerjg Feb 18, 2019

powerjg Feb 18, 2019

powerjg Feb 19, 2019

powerjg commented Feb 21, 2019

powerjg left a comment

powerjg Feb 24, 2019

ghost Feb 25, 2019

powerjg Feb 24, 2019

ghost Feb 25, 2019

powerjg Feb 24, 2019

powerjg Feb 24, 2019

ghost Feb 25, 2019

powerjg Feb 24, 2019

ghost Feb 25, 2019

powerjg Feb 24, 2019

ghost Feb 25, 2019 •

edited by ghost

Loading

		import chisel3.util._


		class GlobalPredictor(implicit val conf: CPUConfig) extends Module {

		@@ -171,6 +237,8 @@ class PipelinedCPU(implicit val conf: CPUConfig) extends Module {
		// Send the instruction to the immediate generator
		immGen.io.instruction := if_id.instruction

added functionality for always taken and dynamic branch prediction #15

added functionality for always taken and dynamic branch prediction #15

Conversation

ghost commented Feb 18, 2019

powerjg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

powerjg commented Feb 21, 2019

powerjg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost Feb 25, 2019 • edited by ghost Loading

Choose a reason for hiding this comment

ghost Feb 25, 2019 •

edited by ghost

Loading