veryl-lang / veryl

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

[Feature] Introduce `clock`/`reset` types #622

Closed taichi-ishitani closed 1 month ago

taichi-ishitani commented 3 months ago

clock and reset signals must be made explicit to implement following new features.

Introduce special data type clock and reset types for this purpose.

i_clk: input clock,
i_rst_n: input reset,
saturn77 commented 3 months ago

I think that this is a great approach to address this issue, while having multiple features as a result.

Basically in your approach "clock" and "reset" are data types allowing for flexible implementation. I think this will work.

Another option / idea is to introduce a special rust crate for global data types "clock_t" and "reset_t". This crate would support synthesis and language features of Veryl that are oriented towards synthesis. For example, including jitter in the clock struct as shown in the example below.

pub mod veryl_std {

/// This is a rust module / crate read by Veryl compiler 
/// Do not edit without special care 

enum : Polarity  {
positive(1), 
negative(0),
}

struct Frequency {
value : f32,
jitter : f32, 
}

// clock data type - includes name, polarity, frequency, and 
// whether the clock comes from a DLL / PLL (versus direct pin pad) 
struct clock {
name : String, 
polarity : Polarity, 
frequency : Frequency, 
dll_pll : bool, 
}

}

Then, in veryl code:

import veryl_std; 

module {
i_clk: input clock.name,
i_rst_n: input reset.name,
}
{
local clock = {
polarity : positive, 
freq.value : "100e6", 
freq.jitter : "1e-9", 
dll_pll : true,
};

}

Overall, this is just an idea if more meta-information would be added to clock / reset.

The existing approach seems like a good way to go for now.

taichi-ishitani commented 3 months ago

refs: https://github.com/veryl-lang/veryl/issues/575#issuecomment-2028741523

How abount reset type including polarity and synchronicity like below.

For #623, Veryl will insert prefix or suffix to reset ports of which type is reset only.

By reset types including reset properties, Veryl can report error when reset signals of which properties are not matched are connected.

i_rst: input reset
...

inst u_foo: foo (
  i_foo_rst: i_foo // i_foo_rst is sync/active high reset but i_rst's properties are not specified.
                   // Veryl reports reset mismatch error.
);

For this case, user need to put type cast explicitly.

i_rst: input reset
...

let foo_rst: reset_sync_high = i_rst as reset_sync_high;
inst u_foo: foo (
  i_foo_rst: foo_rst // reset properties are matched so Veryl reports no errors.
);

Generated SV:

input var i_rst;
...
logic foo_rst;

always_comb begin
  foo_rst = ~i_rst;
end

foo u_foo (
  .i_foo_rst (foo_rst)
);
saturn77 commented 2 months ago

@taichi-ishitani The approach for reset you propose seems like a good approach, particularly for the reset signal.

Overall I still favor a clock/reset signal that is a datatype structure. Elements of the structure can be added in the future which give flexibility to the details of the clock and reset, but mostly on the clock. For example, frequency, jitter, etc.

From a high level perspective of the language, my general thoughts are :

  1. clock and reset should be a datatype to create flexibility
  2. the datatype could be a struct/enum but suggesting a struct
  3. the struct holds information for SystemVerilog based digital design and synthesis
  4. Veryl parser/compiler extracts information for SystemVerilog and build scripts based on clock and reset struct's

An example is below, where the clock type as data structure is defined in an interface, and then the interface could be used in any number of modules.


interface fpga  {

import veryl_std; 

var clock : veryl_std::clock_t = {
polarity : positive, 
freq.value : "100e6", 
freq.jitter : "1e-9", 
dll_pll : true,
};
    var reset : reset_t; 

    modport clock_type {
        clock : input ,
    }
   modport reset_type {
       reset : input, 
   }
}

with an instantiation something like :

module adder #() (
i_clock : modport fpga::clock_type,    // captures details - polarity, name, freq, jitter, etc. 
i_reset : modport fpga::reset_type,
i_arg_a : logic<8>,
i_arg_b : logic<8>,
o_adder_result : logic<9>, 
)
{ 
  always_ff(i_clock, i_reset) {
    if_reset {
        o_adder_result = 0;
    } else {
        o_adder_result = i_arg_a + i_arg_b; 
    }
  }

}
taichi-ishitani commented 2 months ago

In my opinion, I think Veryl.toml should contain these kind of clock information but not RTL body.

saturn77 commented 2 months ago

Yes, I think there are positive and negatives to the clock information being in the RTL versus toml file.

The toml file implements a "singleton pattern" for all the clocks, while the RTL view is more of a configurable pattern.

I think both ways are okay.

dalance commented 1 month ago

I'll merge #701 having simple clock and reset types. The idea of complex struct is interesting too, but I think parameters like frequency should be placed out of source code because it is affected by elements which can't be expressed by RTL. For example, device (ASIC or FPGA?), process (which Fab?, which technology node?), voltage and so on.