Adding back the HWLoop #436

davideschiavone · 2020-08-04T20:49:12Z

This PR has the HIGHEST priority according to the Core Task Group Plan

The HWLoop has been added back to the core
It moves the RVC decoder from the IF to the ID stage
It uses a new prefetcher

It has major changes, so before merging:

2 or more users should check the synthesis results in TIMING and AREA
The CI check in core-v-verif should not be broken

In the PULP team, we already checked the synthesis. Functional bugs not yet.
I ask @Silabs-ArjanB to carefully review the PR and check at least the synthesis
Adding @MikeOpenHWGroup for the sanity check on core-v-verif

A new hwlp test has been added. However, it is here only temporanely as it must be moved to core-v-verif (together with the interrupt test)

…FIFO for 32-only instructions

…the ID stage as the IF stage is now acting only as a FIFO

… module

…state, add FSM state WAIT_ABORT_HWLOOP

…pdated only when id_valid_i is high

Silabs-ArjanB · 2020-08-17T06:39:49Z

rtl/cv32e40p_controller.sv

+                      // Todo: check this. The message does not seem coherent with the condition and why is this condition an error?
+                      if ( (hwlp_end_addr_i[1] == pc_id_i + 4 && hwlp_counter_i[1] > 1) &&  (hwlp_end_addr_i[0] == pc_id_i + 4 && hwlp_counter_i[0] > 1))
+                      begin
+`ifndef SYNTHESIS


No ifdef/ifndef allowed

@mp-17 change to illegal statements plus assertion pls

Silabs-ArjanB · 2020-08-17T06:43:21Z

rtl/cv32e40p_controller.sv

+                    hwlp_dec_cnt_o[1] = 1'b1;
+                end
+
+                 if (debug_single_step_i && ~debug_mode_q)


Why was this added? I am not saying it is wrong or right; it just seems to have nothing to do with HWLP. Is this a bug fix for something else?

I think it has to do with debug during HWLoop. However I don't think it has been tested

rtl/cv32e40p_controller.sv

Silabs-ArjanB

Please see comments. Biggest change I request is to move the aligner back to IF.

Please also provide a cycle count comparison as was done for the OBI change, i.e. run entire compliance suite with an without wait states and compare cycle count against master branch.

Silabs-ArjanB · 2020-08-17T07:35:45Z

rtl/cv32e40p_controller.sv

+    hwlp_mask_o           = 1'b0;
+    branch_is_jump_o        = jump_in_dec; // To the aligner, to save the JUMP if ID is stalled
+
+    hwlp_end0_eq_pc         = hwlp_end_addr_i[0] == pc_id_i;


Move line 325-338, 341 outside this always_comb block into a generate statement based on PULP_XPULP (maybe combine with the generate statement requested in line 1301.

I don't understand why is_hwlp_illegal is treated different.

done. Same question as above, why is it different? what do you mean?

rtl/cv32e40p_controller.sv

Silabs-ArjanB

We just received feedback from our backend team that the original design is already unbalanced and therefore we are now asking that both the compressed decoder and the aligner remain in the IF stage (to not make the problem even worse).

Moved Aligner and RVC to IF stage

davideschiavone · 2020-08-20T16:17:00Z

Please see comments. Biggest change I request is to move the aligner back to IF.

Please also provide a cycle count comparison as was done for the OBI change, i.e. run entire compliance suite with an without wait states and compare cycle count against master branch.

Done in the last commits

Silabs-ArjanB · 2020-08-21T06:47:00Z

rtl/cv32e40p_if_stage.sv

-        if (fetch_is_hwlp)
-          hwlp_dec_cnt_id_o   <= hwlp_dec_cnt_if;
-
+        pc_id_o             <= pc_if_o;
      end else if (clear_instr_valid_i) begin


Although this code was already there, could you please add a comment on why clear_instr_valid_i is lower priority than 'if_valid && instr_valid'?

Silabs-ArjanB · 2020-08-21T06:49:48Z

rtl/cv32e40p_if_stage.sv

+  (
+    .instr_i         ( instr_aligned        ),
+    .instr_o         ( instr_decompressed   ),
+    .is_compressed_o ( instr_compressed_int ),


rename instr_compressed_int to is_compressed in this file

Silabs-ArjanB

Thank you for moving aligner and RVC back to the IF stage!

davideschiavone · 2020-08-27T11:25:51Z

Hi @MikeOpenHWGroup , this branch seems to be ready to merge. Can we have the last CI "greenlight" from you on this branch?
thanks a lot

MikeOpenHWGroup · 2020-08-27T15:01:35Z

@davideschiavone, @Silabs-ArjanB, I will regress this now. Note that @strichmo, has a fork of the testbench that introduces a lot of new interrupt testing and this is also ready for merging. So Steve and I will co-ordinate regressions of this PR. Stay tuned.

MikeOpenHWGroup · 2020-08-27T18:00:30Z

@davideschiavone, I have run both the ci_check and MetricsCI regressions on a fork of the RTL that is a merge of the head of cv32e40p and this pull-request. No unexpected failures, so from my end you are good-to-go to merge in this pr.

Please allow @strichmo time to respond to this pr before merging.

davideschiavone · 2020-08-27T18:59:15Z

Amazing news @MikeOpenHWGroup ! Really good news! So @Silabs-ArjanB , let's wait for @strichmo and then we are good to go

strichmo · 2020-08-28T15:47:40Z

My regression tests are clean. I am OK to proceed with merge!

davideschiavone and others added 30 commits December 10, 2019 11:46

using always 32bits prefetcher

d147824

removed HWLoop, RVC and prefetcher fifo with RVC support, now just a …

38c944c

…FIFO for 32-only instructions

🚧 Adding back RVC support with aligned and compressed decoder now in …

dff4d79

…the ID stage as the IF stage is now acting only as a FIFO

adding riscv aligner

0f91fa2

fix halt if and update waves

729ad63

add support for branches

c6f71a6

🚧 keep on working on RVC support in the ID stage

beb1135

adding pc_if as next pc and change pipe_flush to wfi

2c2a2ca

updated rtl/riscv_aligner.sv

db48689

removed pc registers in IF

96f6aa9

🚧 reworked HWLoop in new pipeline

a9fa357

removed unused signal from controller

8513f91

tmp removed unused states in LSU

3bfe1ea

fixed WAIT RVALID on hwloop branches during stalls on iMem

89884e5

added ports hwloop_target_reg from ID to IF

1532dd9

fixed wait for grant when hwloop is used

d89347e

update hwloop controller

7c376e6

🚧 using the last version of FIFO from common cells

80f2716

lowering grant and valid on correct edge of the clock in random stall…

674e75e

… module

removed deprecated 128bit prefetcher

8ba9f4b

🐛 add flushing conditions in RVALID states during HWLoops

e003339

updated rtl/riscv_prefetch_buffer.sv: added default for fifo_flush

bcfa13d

🚓 updated rtl/riscv_prefetch_buffer.sv: fixed valid_o in WAIT_RVALID …

6fadc44

…state, add FSM state WAIT_ABORT_HWLOOP

Updated rtl/riscv_hwloop_controller.sv: HwloopTarget and pcisEnd is u…

40c3d00

…pdated only when id_valid_i is high

updated

f18ca3d

updated rtl/riscv_prefetch_buffer.sv. Fixed issue on ABORT HWLOOP

0434224

updated src_files.yml

8a4aca0

BUGFIX, fixed aligner and controller handshake

59178b9

✨ merging CLINT on new pipeline branch

fd1e5be

🎨 updating Bender

12915fe

Silabs-ArjanB reviewed Aug 17, 2020

View reviewed changes

rtl/cv32e40p_controller.sv Show resolved Hide resolved

Silabs-ArjanB reviewed Aug 17, 2020

View reviewed changes

rtl/cv32e40p_controller.sv Show resolved Hide resolved

Silabs-ArjanB reviewed Aug 17, 2020

View reviewed changes

rtl/cv32e40p_controller.sv Outdated Show resolved Hide resolved

Silabs-ArjanB requested changes Aug 17, 2020

View reviewed changes

Silabs-ArjanB reviewed Aug 17, 2020

View reviewed changes

rtl/cv32e40p_controller.sv Show resolved Hide resolved

Silabs-ArjanB requested changes Aug 17, 2020

View reviewed changes

2nd update reviews PR#436

d22d781

DavidM-EMUS mentioned this pull request Aug 18, 2020

Updated pulp_vectorial_complex test openhwgroup/core-v-verif#193

Merged

davideschiavone and others added 6 commits August 19, 2020 00:18

moving aligner to if

d5d724e

🐛 fix debug

0b4d9a7

moved rvc decoder in if

8eebae5

♻️ clean up the code

89d3045

removed extra variable in Makefile

6420cdf

Merge pull request #1 from davideschiavone/new_pipeline_aligner_rvc_if

e460eff

Moved Aligner and RVC to IF stage

Silabs-ArjanB reviewed Aug 21, 2020

View reviewed changes

davideschiavone changed the title ~~[DO NOT MERGE] Adding back the HWLoop~~ Adding back the HWLoop Aug 28, 2020

removed instr_width parameter

badb22f

davideschiavone merged commit 7c65d1a into openhwgroup:master Aug 28, 2020

MikeOpenHWGroup mentioned this pull request Aug 28, 2020

Use cv32e40p_wrapper to bind in RTL assertions. #421

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding back the HWLoop #436

Adding back the HWLoop #436

davideschiavone commented Aug 4, 2020 •

edited

Loading

Silabs-ArjanB Aug 17, 2020

davideschiavone Aug 18, 2020

Silabs-ArjanB Aug 17, 2020

davideschiavone Aug 18, 2020

Silabs-ArjanB left a comment

Silabs-ArjanB Aug 17, 2020

davideschiavone Aug 18, 2020

Silabs-ArjanB left a comment

davideschiavone commented Aug 20, 2020

Silabs-ArjanB Aug 21, 2020

Silabs-ArjanB Aug 21, 2020

Silabs-ArjanB left a comment

davideschiavone commented Aug 27, 2020

MikeOpenHWGroup commented Aug 27, 2020

MikeOpenHWGroup commented Aug 27, 2020

davideschiavone commented Aug 27, 2020

strichmo commented Aug 28, 2020

Adding back the HWLoop #436

Adding back the HWLoop #436

Conversation

davideschiavone commented Aug 4, 2020 • edited Loading

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

Silabs-ArjanB left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Silabs-ArjanB left a comment

Choose a reason for hiding this comment

davideschiavone commented Aug 20, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Silabs-ArjanB left a comment

Choose a reason for hiding this comment

davideschiavone commented Aug 27, 2020

MikeOpenHWGroup commented Aug 27, 2020

MikeOpenHWGroup commented Aug 27, 2020

davideschiavone commented Aug 27, 2020

strichmo commented Aug 28, 2020

davideschiavone commented Aug 4, 2020 •

edited

Loading