veryl-lang / veryl

Veryl: A Modern Hardware Description Language
Other
439 stars 20 forks source link

[Feature] clock/reset port connection check #708

Open taichi-ishitani opened 2 months ago

taichi-ishitani commented 2 months ago

refs: https://github.com/veryl-lang/veryl/issues/622#issuecomment-2030848796

To prevent clock/reset type mismatch, we need to check type mismatch on clocl/reset port connections.


module ModA (
  i_clk:   input clock_posedge,
  i_rst_n: input reset_async_low
) {}

module ModB (
  i_clk: input clock,
  i_rst: input reset
){
  inst u_moda: ModA (
    i_clk:   i_clk, // these connection include type mismatch
    i_rst_n: i_rst  // so Veryl should report an error.
  );
}
nblei commented 1 month ago

I'm a bit confused.

What does clock_posedge mean? Does it mean that any flops driven by this clock must be positive edge-triggered? In which case, what is the issue with connecting i_clk : clock to i_clk : clock_posedge?

taichi-ishitani commented 1 month ago

For clock type, its polarity will be specified by the clock_type build option. So there is a case that its polarity is negedge. For clock_posedge type, its polarity is fixed to posedge.

nblei commented 1 month ago

I get that, I think I'm more questioning the notion of this being a property of the clock wire rather than a property of the registers.

taichi-ishitani commented 1 month ago

I'd like to simplify always_ff declarations so I've moved clock/reset prorperties from always_ff block to clock/reset types.

dalance commented 1 month ago

I think that using clock_posedge is more explicit than placing posedge in sensitivity list. Espacially I like it can show that the clock/reset connection should be careful at the module port declaration which is public API.

module ModuleA (
    i_clk: clock_posedge,
    i_rst: reset_async_low,
) {
    always_ff {
        if_reset {
        } else {
        }
    }
}
module ModuleB (
    i_clk: clock,
    i_rst: reset,
) {
    let w_clk: clock_negedge = i_clk;
    let w_rst: reset_async_high = i_rst;
    always_ff (w_clk, w_rst) {
        if_reset {
        } else {
        }
    }
}
module ModuleA (
    i_clk: clock,
    i_rst: reset,
) {
    always_ff (posedge i_clk, async_low i_rst) {
        if_reset {
        } else {
        }
    }
}
module ModuleB (
    i_clk: clock,
    i_rst: reset,
) {
    always_ff (negedge i_clk, async_high, i_rst) {
        if_reset {
        } else {
        }
    }
}