Chisel/FIRRTL DefnameDifferentPortsException

207 views Asked by At

I recently updated the Chisel version of a big project of mine from 3.1.1 to 3.4.0; however, I am getting a bunch of firrtl.passes.CheckHighFormLike$DefnameDifferentPortsException:

firrtl.passes.CheckHighFormLike$DefnameDifferentPortsException: : ports of extmodule XilinxSimpleDualPortNoChangeBRAM with defname XilinxSimpleDualPortNoChangeBRAM are different for an extmodule with the same defname
firrtl.passes.CheckHighFormLike$DefnameDifferentPortsException: : ports of extmodule XilinxSimpleDualPortNoChangeBRAM_1 with defname XilinxSimpleDualPortNoChangeBRAM are different for an extmodule with the same defname
// and so on 241 times

Here is the definition of XilinxSimpleDualPortNoChangeBRAM, together with its dependencies:

class XilinxSimpleDualPortNoChangeBRAM(width: Int,
                                       depth: Int,
                                       performance: String="HIGH_PERFORMANCE",
                                       initFile: String="",
                                       ramStyle: String="block",
                                       val useReset: Boolean=false)
                                       extends BlackBox(Map("RAM_WIDTH" -> width,
                                                            "RAM_DEPTH" -> depth,
                                                            "RAM_PERFORMANCE" -> performance,
                                                            "INIT_FILE" -> initFile,
                                                            "RAM_STYLE" -> ramStyle))
                                       with HasBlackBoxResource with Memory {
    val io = IO(new XilinxSimpleDualPortBRAMBlackBoxIO(log2Ceil(depth), width))
    val acceptedRamStyles = Seq("block", "distributed", "registers", "ultra")
    require(acceptedRamStyles contains ramStyle)
    def write(wrAddr: UInt, wrData: UInt, wrEn: Bool): Unit = {
      io.wea := wrEn
      io.addra := wrAddr
      io.dina := wrData
    }

    def read(rdAddr: UInt, rdEn: Bool): UInt = {
      io.addrb := rdAddr
      io.regceb := rdEn
      io.enb := rdEn
      io.doutb
    }

    def defaultBindings(clock: Clock, reset: core.Reset): Unit = {
      io.clock := clock
      if(useReset)
        io.reset := reset
      else
        io.reset := false.B
    }
    setResource("/XilinxSimpleDualPortNoChangeBRAM.v")
}

trait Memory extends BaseModule {
  def read(rdAddr: UInt, rdEn: Bool): UInt
  def write(wrAddr: UInt, wrData: UInt, wrEn: Bool): Unit
  val latency: Int = 2
}

class XilinxSimpleDualPortBRAMIO(addrWidth: Int, dataWidth: Int) extends Bundle {
    val addra  = Input(UInt(addrWidth.W))
    val addrb  = Input(UInt(addrWidth.W))
    val dina   = Input(UInt(dataWidth.W))
    val wea    = Input(Bool())
    val enb    = Input(Bool())
    val regceb = Input(Bool())
    val doutb  = Output(UInt(dataWidth.W))

    override def cloneType = (new XilinxSimpleDualPortBRAMIO(addrWidth, dataWidth)).asInstanceOf[this.type]
}
class XilinxSimpleDualPortBRAMBlackBoxIO(addrWidth: Int, dataWidth: Int) extends XilinxSimpleDualPortBRAMIO(addrWidth, dataWidth) {
    val clock  = Input(Clock())
    val reset  = Input(Reset())

    override def cloneType = (new XilinxSimpleDualPortBRAMBlackBoxIO(addrWidth, dataWidth)).asInstanceOf[this.type]
}

The Verilog resource XilinxSimpleDualPortNoChangeBRAM.v is one of the BRAM instantiation templates available in Vivado:

module XilinxSimpleDualPortNoChangeBRAM #(
  parameter RAM_WIDTH = 64,                       // Specify RAM data width
  parameter RAM_DEPTH = 512,                      // Specify RAM depth (number of entries)
  parameter RAM_PERFORMANCE = "HIGH_PERFORMANCE", // Select "HIGH_PERFORMANCE" or "LOW_LATENCY"
  parameter INIT_FILE = "",                       // Specify name/location of RAM initialization file if using one (leave blank if not)
  parameter RAM_STYLE = "block"                   // Target memory type. Accepted values: block, distributed, registers, ultra (UltraScale+ only)
) (
  input [clogb2(RAM_DEPTH-1)-1:0] addra, // Write address bus, width determined from RAM_DEPTH
  input [clogb2(RAM_DEPTH-1)-1:0] addrb, // Read address bus, width determined from RAM_DEPTH
  input [RAM_WIDTH-1:0] dina,          // RAM input data
  input wea,                           // Write enable
  input enb,                           // Read Enable, for additional power savings, disable when not in use
  input regceb,                        // Output register enable
  output [RAM_WIDTH-1:0] doutb,         // RAM output data
  input clock,                          // Clock
  input reset                          // Output reset (does not affect memory contents)
);

  (* ram_style = RAM_STYLE *) reg [RAM_WIDTH-1:0] BRAM [RAM_DEPTH-1:0];
  reg [RAM_WIDTH-1:0] ram_data = {RAM_WIDTH{1'b0}};

  // The following code either initializes the memory values to a specified file or to all zeros to match hardware
  generate
    if (INIT_FILE != "") begin: use_init_file
      initial
        $readmemh(INIT_FILE, BRAM, 0, RAM_DEPTH-1);
    end else begin: init_bram_to_zero
      integer ram_index;
      initial
        for (ram_index = 0; ram_index < RAM_DEPTH; ram_index = ram_index + 1)
          BRAM[ram_index] = {RAM_WIDTH{1'b0}};
    end
  endgenerate

  always @(posedge clock) begin
    if (wea)
      BRAM[addra] <= dina;
    if (enb)
      ram_data <= BRAM[addrb];
  end

  //  The following code generates HIGH_PERFORMANCE (use output register) or LOW_LATENCY (no output register)
  generate
    if (RAM_PERFORMANCE == "LOW_LATENCY") begin: no_output_register

      // The following is a 1 clock cycle read latency at the cost of a longer clock-to-out timing
       assign doutb = ram_data;

    end else begin: output_register

      // The following is a 2 clock cycle read latency with improve clock-to-out timing

      reg [RAM_WIDTH-1:0] doutb_reg = {RAM_WIDTH{1'b0}};

      always @(posedge clock)
        if (reset)
          doutb_reg <= {RAM_WIDTH{1'b0}};
        else if (regceb)
          doutb_reg <= ram_data;

      assign doutb = doutb_reg;

    end
  endgenerate

  //  The following function calculates the address width based on specified RAM depth
  function integer clogb2;
    input integer depth;
      for (clogb2=0; depth>0; clogb2=clogb2+1)
        depth = depth >> 1;
  endfunction

endmodule

I tried to have a look at the file where this exception is thrown, CheckHighForm.scala, but I quickly got lost as I have no idea of what I should be looking for.

What I understood from the tests in CheckSpec.scala is that it should throw an exception if ExtModules have matching port names and widths, but a different order, so I tried to make the order of the inputs in the Chisel BlackBox the same as the ones in the Verilog module but I still get an exception.

The test throw an exception if parameterless ExtModules have the same ports, but different widths made me think that having multiple instantiations with different port widths could be the reason for the exception, but then there is another test that says that it should NOT throw an exception if ExtModules have parameters, matching port names, but different widths, which is the case here as the port widths are controlled by parameters.

What could be the reason for this exception?

Update: As requested, here is the FIRRTL IR of two instantiations of the black box:

  extmodule XilinxSimpleDualPortNoChangeBRAM : 
    input addra : UInt<14>
    input addrb : UInt<14>
    input dina : UInt<1>
    input wea : UInt<1>
    input enb : UInt<1>
    input regceb : UInt<1>
    output doutb : UInt<1>
    input clock : Clock
    input reset : Reset
    
    defname = XilinxSimpleDualPortNoChangeBRAM
    parameter RAM_STYLE = "block"
    parameter RAM_WIDTH = 1
    parameter RAM_DEPTH = 16384
    parameter RAM_PERFORMANCE = "HIGH_PERFORMANCE"
    parameter INIT_FILE = ""

  extmodule XilinxSimpleDualPortNoChangeBRAM_1 : 
    input addra : UInt<6>
    input addrb : UInt<6>
    input dina : UInt<518>
    input wea : UInt<1>
    input enb : UInt<1>
    input regceb : UInt<1>
    output doutb : UInt<518>
    input clock : Clock
    input reset : Reset
    
    defname = XilinxSimpleDualPortNoChangeBRAM
    parameter RAM_STYLE = "distributed"
    parameter RAM_WIDTH = 518
    parameter RAM_DEPTH = 64
    parameter RAM_PERFORMANCE = "HIGH_PERFORMANCE"
    parameter INIT_FILE = ""

Update 2: Apparently, some XilinxSimpleDualPortNoChangeBRAM were picking up an older version of XilinxSimpleDualPortBRAMBlackBoxIO where the reset was still of type Bool instead of Reset. Changing that solved the issue.

1

There are 1 answers

3
seldridge On BEST ANSWER

This check is supposed to disallow impossible situations when referring to a specific BlackBox. Namely, the following must be true:

  • If the BlackBox has no parameters, then all ports must have the same name, same width, and be in the same order
  • If the BlackBox has parameters, then all ports must have the same name and be in the same order (but may have different widths)

It sounds like either your example is producing BlackBoxes that violate the latter condition (since your BlackBoxes have parameters) or this has exposed a bug in that FIRRTL compiler check.

The actual Verilog module is never checked and can't cause any problems here.

Could you update your question to provide the FIRRTL IR that is producing these errors? Specifically, what does the FIRRTL IR for XilinxSimpleDualPortNoChangeBRAM and XilinxSimpleDualPortNoChangeBRAM_1 look like? This should be in a file like "Foo.fir". Alternatively, you can do something like:

import chisel3.stage.ChiselStage

/* Note: this is emitChirrtl ("chirrtl") as you want the FIRRTL emitted from Chisel. */
println(ChiselStage.emitChirrtl(new MyTopModule))