Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[firtool] Memories are compiled with randomization logic, but not macros #8164

Closed
seldridge opened this issue Jan 31, 2025 · 0 comments · Fixed by #8170
Closed

[firtool] Memories are compiled with randomization logic, but not macros #8164

seldridge opened this issue Jan 31, 2025 · 0 comments · Fixed by #8170
Assignees
Labels
bug Something isn't working FIRRTL Involving the `firrtl` dialect Seq Involving the `seq` dialect

Comments

@seldridge
Copy link
Member

The memory randomization logic for FIRRTL memories is being included. However, the required Verilog macros are not being included. This can create situations where memories are not initialized.

Consider the following FIRRTL:

FIRRTL version 4.2.0
circuit Foo:
  public module Foo:
    input clock: Clock
    input r: {addr: UInt<3>, en: UInt<1>, flip data: UInt<32>}
    input w: {addr: UInt<3>, en: UInt<1>, data: UInt<32>, mask: UInt<1>}

    mem memory:
      data-type => UInt<32>
      depth => 16
      reader => r
      writer => w
      read-latency => 1
      write-latency => 1
      read-under-write => undefined

    connect memory.r.addr, r.addr
    connect memory.r.en, r.en
    connect memory.r.clk, clock
    connect r.data, memory.r.data

    connect memory.w.addr, w.addr
    connect memory.w.en, w.en
    connect memory.w.clk, clock
    connect memory.w.data, w.data
    connect memory.w.mask, w.mask

When compiled, this produces (with split-verilog to make it more obvious) the following memory:

// Generated by CIRCT firtool-1.101.0-44-g0cea50e9e
// VCS coverage exclude_file
module memory_16x32(
  input  [3:0]  R0_addr,
  input         R0_en,
                R0_clk,
  output [31:0] R0_data,
  input  [3:0]  W0_addr,
  input         W0_en,
                W0_clk,
  input  [31:0] W0_data
);

  reg [31:0] Memory[0:15];
  reg        _R0_en_d0;
  reg [3:0]  _R0_addr_d0;
  always @(posedge R0_clk) begin
    _R0_en_d0 <= R0_en;
    _R0_addr_d0 <= R0_addr;
  end // always @(posedge)
  always @(posedge W0_clk) begin
    if (W0_en & 1'h1)
      Memory[W0_addr] <= W0_data;
  end // always @(posedge)
  `ifdef ENABLE_INITIAL_MEM_
    `ifdef RANDOMIZE_REG_INIT
      reg [31:0] _RANDOM;
    `endif // RANDOMIZE_REG_INIT
    reg [31:0] _RANDOM_MEM;
    initial begin
      `INIT_RANDOM_PROLOG_
      `ifdef RANDOMIZE_MEM_INIT
        for (logic [4:0] i = 5'h0; i < 5'h10; i += 5'h1) begin
          _RANDOM_MEM = `RANDOM;
          Memory[i[3:0]] = _RANDOM_MEM;
        end
      `endif // RANDOMIZE_MEM_INIT
      `ifdef RANDOMIZE_REG_INIT
        _RANDOM = {`RANDOM};
        _R0_en_d0 = _RANDOM[0];
        _R0_addr_d0 = _RANDOM[4:1];
      `endif // RANDOMIZE_REG_INIT
    end // initial
  `endif // ENABLE_INITIAL_MEM_
  assign R0_data = _R0_en_d0 ? Memory[_R0_addr_d0] : 32'bx;
endmodule

This includes randomization logic, but it will never run unless other macros are defined. To highlight this, see all the macros generated for module Foo:

// Generated by CIRCT firtool-1.101.0-44-g0cea50e9e

// Include rmemory initializers in init blocks unless synthesis is set
`ifndef RANDOMIZE
  `ifdef RANDOMIZE_MEM_INIT
    `define RANDOMIZE
  `endif // RANDOMIZE_MEM_INIT
`endif // not def RANDOMIZE
`ifndef SYNTHESIS
  `ifndef ENABLE_INITIAL_MEM_
    `define ENABLE_INITIAL_MEM_
  `endif // not def ENABLE_INITIAL_MEM_
`endif // not def SYNTHESIS

// Standard header to adapt well known macros for register randomization.

// RANDOM may be set to an expression that produces a 32-bit random unsigned value.
`ifndef RANDOM
  `define RANDOM $random
`endif // not def RANDOM

// Users can define INIT_RANDOM as general code that gets injected into the
// initializer block for modules with registers.
`ifndef INIT_RANDOM
  `define INIT_RANDOM
`endif // not def INIT_RANDOM

// If using random initialization, you can also define RANDOMIZE_DELAY to
// customize the delay used, otherwise 0.002 is used.
`ifndef RANDOMIZE_DELAY
  `define RANDOMIZE_DELAY 0.002
`endif // not def RANDOMIZE_DELAY

// Define INIT_RANDOM_PROLOG_ for use in our modules below.
`ifndef INIT_RANDOM_PROLOG_
  `ifdef RANDOMIZE
    `ifdef VERILATOR
      `define INIT_RANDOM_PROLOG_ `INIT_RANDOM
    `else  // VERILATOR
      `define INIT_RANDOM_PROLOG_ `INIT_RANDOM #`RANDOMIZE_DELAY begin end
    `endif // VERILATOR
  `else  // RANDOMIZE
    `define INIT_RANDOM_PROLOG_
  `endif // RANDOMIZE
`endif // not def INIT_RANDOM_PROLOG_
module Foo(
  input         clock,
  input  [2:0]  r_addr,
  input         r_en,
  output [31:0] r_data,
  input  [2:0]  w_addr,
  input         w_en,
  input  [31:0] w_data,
  input         w_mask
);

  memory_16x32 memory_ext (
    .R0_addr ({1'h0, r_addr}),
    .R0_en   (r_en),
    .R0_clk  (clock),
    .R0_data (r_data),
    .W0_addr ({1'h0, w_addr}),
    .W0_en   (w_en & w_mask),
    .W0_clk  (clock),
    .W0_data (w_data)
  );
endmodule

Fix this by generating the macros for the memory.

@seldridge seldridge added bug Something isn't working FIRRTL Involving the `firrtl` dialect Seq Involving the `seq` dialect labels Jan 31, 2025
@seldridge seldridge self-assigned this Jan 31, 2025
seldridge added a commit that referenced this issue Feb 1, 2025
When generating modules as part of the SeqToSV conversion, put appropriate
memory generation fragments onto the created `hw.module.generated`.  No
longer put memory randomization fragments onto the original module.

Fixes #8164.

Signed-off-by: Schuyler Eldridge <[email protected]>
seldridge added a commit that referenced this issue Feb 1, 2025
When generating modules as part of the SeqToSV conversion, put appropriate
memory generation fragments onto the created `hw.module.generated`.  No
longer put memory randomization fragments onto the original module.

Fixes #8164.

Signed-off-by: Schuyler Eldridge <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working FIRRTL Involving the `firrtl` dialect Seq Involving the `seq` dialect
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant