OpenXiangShan / XiangShan

Open-source high-performance RISC-V processor
https://xiangshan.cc
Other
4.85k stars 663 forks source link

fix(BPU): remove reg of reset_vector #3669

Closed Tang-Haojin closed 1 month ago

Tang-Haojin commented 1 month ago

Currently, DelayNWithValid generates code like:

module DelayNWithValid(
  input         clock,
  input  [47:0] io_in_bits,
  input         io_in_valid,
  output [47:0] io_out_bits
);

  reg        valid_REG, valid_REG_1, valid_REG_2, valid_REG_3;
  reg [47:0] data, data_1, data_2, data_3, res_bits;
  always @(posedge clock) begin
    valid_REG <= io_in_valid;
    if (io_in_valid)
      data <= io_in_bits;
    valid_REG_1 <= valid_REG;
    if (valid_REG)
      data_1 <= data;
    valid_REG_2 <= valid_REG_1;
    if (valid_REG_1)
      data_2 <= data_1;
    valid_REG_3 <= valid_REG_2;
    if (valid_REG_2)
      data_3 <= data_2;
    if (valid_REG_3)
      res_bits <= data_3;
  end // always @(posedge)
  assign io_out_bits = res_bits;
endmodule

If we pass reset as io_in_valid, it is actually:

// ...
  always @(posedge clock)
    if (reset)
      data <= io_in_bits;
// ...

This is synchronous reset which violates our design rule.

Actually, we do not need DelayNWithValid or RegNext here because io.reset_vector is connected to Vcc or Vdd and will never change. We can set multi-cycle-path to avoid false timing violation.

eastonman commented 1 month ago

However, I believe there are a few more? For example https://github.com/OpenXiangShan/XiangShan/blob/bbaa6b7caaea546354d49f5179a8159f8e4bacdc/src/main/scala/xiangshan/frontend/Frontend.scala#L108

XiangShanRobot commented 1 month ago
[Generated by IPC robot] commit: ac165c173d22c368c15b4a5720630f157fb09b4d commit astar copy_and_run coremark gcc gromacs lbm linux mcf microbench milc namd povray wrf xalancbmk
ac165c1 1.934 0.451 2.704 1.194 2.812 2.465 2.403 0.913 1.401 1.626 3.421 2.743 2.429 3.271
master branch: commit astar copy_and_run coremark gcc gromacs lbm linux mcf microbench milc namd povray wrf xalancbmk
97e37a2 2.704 1.194 0.913 1.401 3.271
d275ad0 2.704 2.401 0.913 1.401 3.271
979d98a 0.451 2.704 2.400 1.401 2.743
7543e8e 0.451 2.704 1.194 2.465 2.400 0.913 1.401 1.626 3.415 2.743 3.271
46e9ee7 1.934 0.451 2.704 1.194 2.812 2.465 2.400 0.913 1.401 1.626 3.421 2.743 2.429 3.271
bbaa6b7 1.934 0.451 2.704 1.194 2.812 2.465 2.400 0.913 1.401 1.626 3.421 2.743 2.429 3.271
4446722 1.934 0.451 2.704 1.194 2.812 2.465 2.402 0.913 1.401 1.626 3.421 2.743 2.429 3.271
e2216ec 1.934 0.451 2.704 1.194 2.812 2.465 2.400 0.913 1.401 1.626 3.415 2.743 2.429 3.271
65b2b1e 1.934 0.451 2.704 1.194 2.812 2.465 2.400 0.913 1.401 1.626 3.415 2.743 2.429 3.271
aa55b9f 1.934 0.451 2.704 1.194 2.812 2.465 2.402 0.913 1.401 1.626 3.421 2.743 2.429 3.271
XiangShanRobot commented 1 month ago
[Generated by IPC robot] commit: 7fb682e74348492af2ab198984e6c72e8c6f006b commit astar copy_and_run coremark gcc gromacs lbm linux mcf microbench milc namd povray wrf xalancbmk
7fb682e 1.934 0.451 2.704 1.194 2.812 2.465 2.400 0.913 1.401 1.626 3.415 2.743 2.429 3.271
master branch: commit astar copy_and_run coremark gcc gromacs lbm linux mcf microbench milc namd povray wrf xalancbmk
1856091 1.934 0.451 2.704 1.194 2.812 2.465 2.401 0.913 1.401 1.626 3.421 2.743 2.429 3.271
bbb9b7b 1.934 0.451 2.704 1.194 2.812 2.465 2.400 0.913 1.401 1.626 3.421 2.743 2.429 3.271
97e37a2 1.934 0.451 2.704 1.194 2.812 2.465 2.402 0.913 1.401 1.626 3.415 2.743 2.429 3.271
d275ad0 1.934 0.451 2.704 1.194 2.812 2.465 2.401 0.913 1.401 1.626 3.421 2.743 2.429 3.271
979d98a 1.934 0.451 2.704 1.194 2.812 2.465 2.400 0.913 1.401 1.626 3.421 2.743 2.429 3.271
7543e8e 1.934 0.451 2.704 1.194 2.812 2.465 2.400 0.913 1.401 1.626 3.415 2.743 2.429 3.271
46e9ee7 1.934 0.451 2.704 1.194 2.812 2.465 2.400 0.913 1.401 1.626 3.421 2.743 2.429 3.271
bbaa6b7 1.934 0.451 2.704 1.194 2.812 2.465 2.400 0.913 1.401 1.626 3.421 2.743 2.429 3.271
4446722 1.934 0.451 2.704 1.194 2.812 2.465 2.402 0.913 1.401 1.626 3.421 2.743 2.429 3.271