veryl-lang / veryl

Veryl: A Modern Hardware Description Language
Other
478 stars 22 forks source link

[Feature] Omit specifying clock and reset signals #569

Closed taichi-ishitani closed 4 months ago

taichi-ishitani commented 6 months ago

Most modules have one pair of clock and reset only so we can omit spacyfying clock and reset singals on always_ff blocks.

always_ff {
  if_reset {
    q = 0;
  } else {
    q = i_d;
  }
}

To realize this feature, special types named clock and reset would be instroduced. These types are to tell Veryl which signals are clock and reset signals. If a module have only one pair of signals of which types are clock and reset then Veryl can insert that clock and reset signals to always_ff blocks automatically.

module foo (
  i_clk:   input  clock,
  i_rst_n: input  reset,
  i_d:     input  logic,
  o_q:     output logic
) {
  var q: logic;

  assign o_q = q;
  always_ff { // Veryl can convert this code to 'always_ff (i_clk, i_rst_n)'
    if_reset {
      q = 0;
    } else {
      q = i_d;
    }
  }
}
module foo (
  i_clk_a:   input  clock,
  i_rst_a_n: input  reset,
  i_clk_b:   input  clock,
  i_rst_b_n: input  reset,
  i_d:       input  logic,
  o_q:       output logic
) {
  var q: logic;

  assign o_q = q;
  always_ff { // Veryl doesn't know which clock/reset pairs should be used. This code causes an error.
    if_reset {
      q = 0;
    } else {
      q = i_d;
    }
  }
}
module foo (
  i_clk:   input  clock<4>,
  i_rst_n: input  reset<4>,
  i_d:     input  logic<4>,
  o_q:     output logic<4>
) {
  for i in 0..4 :g {
    let clk:   clock = i_clk[i];
    let rst_n: reset = i_rst_n[i];
    var q:     logic;

    assign o_q[i] = q;
    always_ff { // block 'g' has one clock/reset pair only so Veryl can convert this to 'always_ff (clk, rst_n)'
      if_reset {
        q = 0;
      } else {
        q = i_d[i];
      }
    }
  }

}

In addition, I think synthesis tools require that clock and reset signals should be single bit signals. Therefore, Veryl should check if signals speciled as clock/reset are single bit signals.

saturn77 commented 6 months ago

I think this is a good feature that will make the code more compact.

My suggestion is to allow for naming the actual clock and reset signals in the toml file, such as

[build]
clock_name = "clock"
reset_name = "reset"

and then there are the positive / negative options for clock, and sync/async positive/negative options for reset.

Some people may want the final SystemVerilog code to have "i_reset_n" versus "i_rst_n" for example.

dalance commented 6 months ago

I prefer this idea, but the forth code block may be bit confusable. In general programming language including SV, both i_clk and clk are visible at the always_ff. Even if the case is not allowed, almost all cases will be covered.

My suggestion is to allow for naming the actual clock and reset signals in the toml file, such as

How about #623 ? Specifing the name directly may be difficult to apply the multi-clock modules.