Parameter value range check wrong if used for Variable.width

[philipaxer]

Hi,

it seems that the actual parameter value (Parameter.value) is wrongly checked against Parameter.width. If I use a Parameter as here size=2**param_width, and param_width.value is larger than param_width.width, I get a Unable to static elaborate value 32'h2 ** ADDR_WIDTH error. This happens if value is 33 and width is 32. The check needs to go through a clog2 to check range if that was intended.

Some other things worthy of note:

• adding an always block that uses vars/ins/outs which are sized using parameters does only work if add_always after all values are fixed. Otherwise a stale parameter.value is used to evaluate range checks.
• a fake value=1 must be provided when the parameter is created through self.parameter. value=None will cause issues downstream when vars are created (e.g. for ports)
• I havent found any way to pass parameters through the hierarchy.

In my opinion, Ideally the width checks must happen deferred at evaluation time (i.e. final call to verilog generation), otherwise the net connectivity and width customization are kept dependent.

How does a generator know if it must emit two SV modules or one?

regards Philip

from kratos import *

class PassThrough(Generator):
    def __init__(self):
        super().__init__("Pass", debug = False)

        self.data_width = self.parameter("DATA_WIDTH", 32, value=1) # error if value is left to default (None)
        self.i_  = self.input("i", self.data_width)
        self.o_  = self.output("o", self.data_width)

        self.wire(self.o_, self.i_)

class SyncRam(Generator):
    def __init__(self, debug=False):
        super().__init__("MEM",  debug = debug)

        self.data_width = self.parameter("DATA_WIDTH", 32, value=1) # error if value is left to default (None)
        self.addr_width = self.parameter("ADDR_WIDTH", 32, value=1) # error if value is left to default (None)

        self.addr = self.input("addr", self.addr_width)
        self.din = self.input("din", self.data_width)
        self.dout = self.output("dout", self.data_width)

        self.we = self.input("we", 1)
        self.en = self.input("en", 1)

        self.clk = self.clock("clk")

        self.mem = self.var("mem", width=self.data_width, size= 2**self.addr_width)

    def finalize(self):
        @always_ff((posedge, "clk"))
        def seq(self):

            if self.en:
                if self.we:
                    self.mem[self.addr] = self.din
                else:
                    self.dout = self.mem[self.addr]

        # It seems that the add_always needs to happen once all parameters have a final value
        self.add_always(seq)

class Test1(Generator):
    def __init__(self, x):
        super().__init__("Top", False)

        i = self.input("i", x)        
        o = self.output("o", x)

        clk = self.clock("clk")
        rst = self.reset("rst")

        r = SyncRam()
        r.data_width.value = x
        r.addr_width.value = x
        r.finalize()

        self.add_child("ram0", r, clk=clk, addr=i, we=0, en=0, din=i, dout=o)

class Test2(Generator):
    def __init__(self, x):
        super().__init__("Top", False)

        x = clog2(x)
        self.width = self.parameter("DATA_WIDTH", 32, value=x)

        i = self.input("i", x)        
        o = self.output("o", x)

        d = self.var("d", x)

        clk = self.clock("clk")
        rst = self.reset("rst")

        r = SyncRam()
        r.data_width.assign(self.width) # That fails but would be cool to have
        r.data_width.assign(clog2(x)) # That fails but would be cool to have
        r.addr_width.value = x
        r.finalize()

        pt = PassThrough()
        pt.data_width.value = x

        self.add_child("pt0", pt, i=d, o=o)
        self.add_child("ram0", r, clk=clk, addr=i, we=0, en=0, din=i, dout=d)

try:
    verilog(Test1(32), optimize_passthrough=False)
except Exception as e:
    print("Test1:", e)

try:
    verilog(Test2(128), filename="test2.sv", optimize_passthrough=False)
except Exception as e:
    print("Test2:", e)

[Kuree]

If I use a Parameter as here size=2param_width, and param_width.value is larger than param_width.width, I get a Unable to static elaborate value 32'h2 ADDR_WIDTH error. This happens if value is 33 and width is 32.

This sounds like a bug. Normally there is an auto resizing for constants. I will double check where the check fails.

adding an always block that uses vars/ins/outs which are sized using parameters does only work if add_always after all values are fixed. Otherwise a stale parameter.value is used to evaluate range checks.

The type checks happens at two places:

1. when you first make assignments.
2. when you do RTL generation.

The indent is that if you don't do any modification after all the generators are instantiated, your circuit is valid. If you do modification at runtime, the mistakes will be caught at the last step. I personally prefer having these two checks instead of one.

a fake value=1 must be provided when the parameter is created through self.parameter. value=None will cause issues downstream when vars are created (e.g. for ports)

This is somehow related to the first one. The parameter interface is still a little bit wonky. Ideally you should provide an initial value and then set values when instantiating instances. I believe for downstream tools like synthesis, if your top module's parameters don't have default values, it is not a legal design. I will refactor the interface to make it more intuitive.

I havent found any way to pass parameters through the hierarchy.

This is related to the second one. Actually you can. Here is what I would do:

        self.width = self.parameter("DATA_WIDTH", 32, value=x)

        i = self.input("i", width=self.width)
        o = self.output("o", width=self.width)

        d = self.var("d", x)

        clk = self.clock("clk")
        rst = self.reset("rst")

        r = SyncRam()
        r.data_width.value = self.width # Directly assign the value
        r.addr_width.value = self.width #
        r.finalize()

        pt = PassThrough()
        pt.data_width.value = self.width

        self.add_child("pt0", pt, i=d, o=o)
        self.add_child("ram0", r, clk=clk, addr=i, we=0, en=0, din=i, dout=d)

There is an ordering issue when using such syntax:

self.add_child("ram0", r, clk=clk, addr=i, we=0, en=0, din=i, dout=d,
                       ADDR_WIDTH=self.width)

I can fix it soon.

[Kuree]

If I use a Parameter as here size=2param_width, and param_width.value is larger than param_width.width, I get a Unable to static elaborate value 32'h2 ADDR_WIDTH error. This happens if value is 33 and width is 32.

This sounds like a bug. Normally there is an auto resizing for constants. I will double check where the check fails.

This is caused by the fact that the static elaboration uses the parameter width to mask off the elaborated value. 1 << 33 when masked with 0xFFFFFFFF is 0, hence failed the check.

Also I think 32-bit width is the larger than the allowed bit width for array size. Just checked with SystemVerilog LRM, Xcelium, and VCS. Although there is no limitation on how big bit width is in the LRM, Xcelium errors out with the following message:

object width exceeds implementation limit (16000000 bits).

VCS errors out with the following message:

The size of vector is too large to handle by VCS. The maximum size for 
  vector is 2G - 1.

So although this is a limitation (sort of a bug) in the framework, it shouldn't affect any real-world usage, since the commercial simulator can't handle it. I will fix it later when I'm refactoring the built-in simulator.

Here is the test code:

module top;
parameter logic[63:0] width = 31;
logic [3:0][2**width-1:0] value; 
initial begin
    $display("size is: %d", $size(value)); 
end 
endmodule

If you set width to 33, I think the simulator runs, but it's due to integer overflow. Unpacked array also gives different error.

Please let me know if you have additional questions.

[philipaxer]

Could you paste the generated SV code from your example above? I agree wrt the 32 bit statement. Is the actual parameter passed on in the topology or just the value?

[Kuree]

Sure. Here is test2.sv:

module MEM #(
  parameter ADDR_WIDTH,
  parameter DATA_WIDTH
)
(
  input logic [ADDR_WIDTH-1:0] addr,
  input logic clk,
  input logic [DATA_WIDTH-1:0] din,
  input logic en,
  input logic we,
  output logic [DATA_WIDTH-1:0] dout
);

logic [DATA_WIDTH-1:0] mem [(32'h2 ** ADDR_WIDTH)-1:0];

always_ff @(posedge clk) begin
  if (en) begin
    if (we) begin
      mem[addr] <= din;
    end
    else dout <= mem[addr];
  end
end
endmodule   // MEM

module Pass #(
  parameter DATA_WIDTH
)
(
  input logic [DATA_WIDTH-1:0] i,
  output logic [DATA_WIDTH-1:0] o
);

assign o = i;
endmodule   // Pass

module Top #(
  parameter DATA_WIDTH = 32'h7
)
(
  input logic clk,
  input logic [DATA_WIDTH-1:0] i,
  input logic rst,
  output logic [DATA_WIDTH-1:0] o
);

logic [6:0] d;
Pass #(
  .DATA_WIDTH(DATA_WIDTH)) pt0 (
  .i(d),
  .o(o)
);

MEM #(
  .ADDR_WIDTH(DATA_WIDTH),
  .DATA_WIDTH(DATA_WIDTH)) ram0 (
  .addr(i),
  .clk(clk),
  .din(i),
  .en(1'h0),
  .we(1'h0),
  .dout(d)
);

endmodule   // Top

Is the actual parameter passed on in the topology or just the value?

It's passed on in the topology. Everytime you change the top-level parameter value, the framework will propagate the value change through the entire design.

[Kuree]

Close this issue for now since all the features have been implemented (I think). Feel free to re-open or create a new issue.