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

Undeclared reference exceptions when using SyncReadMem #775

Closed
rimasaviz opened this issue Feb 11, 2018 · 2 comments
Closed

Undeclared reference exceptions when using SyncReadMem #775

rimasaviz opened this issue Feb 11, 2018 · 2 comments
Milestone

Comments

@rimasaviz
Copy link

rimasaviz commented Feb 11, 2018

When generating verilog for a module with two SyncReadMem instances, I get an undeclared reference exception error that seems wrong (or at the very least inaccurate and counterintuitive):

[error] firrtl.passes.CheckHighForm$UndeclaredReferenceException:  @[memcrap.scala 26:12:@28.6]: [module CrapMem] Reference mem1 is not declared.
[error] firrtl.passes.CheckHighForm$UndeclaredReferenceException:  @[memcrap.scala 28:14:@31.8]: [module CrapMem] Reference mem1 is not declared.

See attached code and error log. The lines of code the error message refers to are inside a when statement, the memories are instantiated just afterwards:

  when (doread) {
    rdaddr := rdaddr + 1.U ***
    when (rdaddr === 47.U) {
      rdaddr := 0.U ***
      rdsel  := !rdsel
      doread := false.B
    }
  }
  val mem0 = SyncReadMem(48, UInt(8.W))
  val mem1 = SyncReadMem(48, UInt(8.W))

Moving the SyncReadMem instantiations above the when statements makes the error go away. Is this the expected behavior?? Doesn't seem right.

memcrap.scala.txt
memcrap.log.txt

@rimasaviz
Copy link
Author

Upon further investigation, it appears that while moving the SyncReadMem instantiations above the when statements gets rid of the error messages, the Verilog that gets generated is not correct (reads from one of the two memories don't work). Here's the problematic verilog (from the always @(posedge clock) block at the end of the file).

    if(mem0__T_46_en & mem0__T_46_mask) begin
      mem0[mem0__T_46_addr] <= mem0__T_46_data; // @[memcrap.scala 15:25:@13.4]
    end
    if (_GEN_22) begin
      mem0_mem0_dout_addr_pipe_0 <= rdaddr;
    end
    if(mem1__T_48_en & mem1__T_48_mask) begin
      mem1[mem1__T_48_addr] <= mem1__T_48_data; // @[memcrap.scala 16:25:@14.4]
    end
    if (_GEN_28) begin
      mem1_mem1_dout_addr_pipe_0 <= rdaddr;
    end

and here's the source of the problem (earlier in the file):

wire  _GEN_22;
assign _GEN_22 = 1'h0;

wire  _GEN_28;
assign _GEN_28 = doread;

Using a single SyncReadMem seems to work OK. The use case that brought this issue to light necessitates using a single 128 deep memory in place of 2 x 48 deep memories so some space is wasted (not an issue for me at the moment). Here's the chisel code that results in incorrect verilog (but no error messages):

package crap
import chisel3._
import chisel3.util._

class CrapMem extends Module {
  val io = IO( new Bundle {
    val din  = Flipped(ValidIO(UInt(8.W)))
    val dout = ValidIO(UInt(8.W))
  })
  val wraddr = RegInit(0.U(6.W))
  val wrsel  = RegInit(false.B)
  val rdaddr = RegInit(0.U(6.W))
  val rdsel  = RegInit(false.B)
  val doread = RegInit(false.B)
  val mem0 = SyncReadMem(48, UInt(8.W))
  val mem1 = SyncReadMem(48, UInt(8.W))
  val mem0_dout = mem0.read(rdaddr)
  val mem1_dout = mem1.read(rdaddr)

  when (io.din.valid) {
    wraddr := wraddr + 1.U
    when (wraddr === 47.U) {
      wraddr  := 0.U
      wrsel   := !wrsel
      doread  := true.B
    }
  }

  when (doread) {
    rdaddr := rdaddr + 1.U
    when (rdaddr === 47.U) {
      rdaddr := 0.U
      rdsel  := !rdsel
      doread := false.B
    }
  }

  when (io.din.valid && !wrsel) {
    mem0.write(wraddr, io.din.bits)
  }

  when (io.din.valid && wrsel) {
    mem1.write(wraddr, io.din.bits)
  }

  io.dout.bits  := Mux(rdsel, mem1_dout, mem0_dout)
  io.dout.valid := doread
}

object BuildCrapMem {
  def main(args: Array[String]): Unit = {
    chisel3.Driver.execute(Array("-X", "verilog", "-td", "./verilog"), () => new CrapMem )
  }
}



@rimasaviz rimasaviz reopened this Feb 12, 2018
@jackkoenig
Copy link
Contributor

I think this is related to chipsalliance/firrtl#727

@azidar azidar added this to the 3.1.1 milestone Mar 2, 2018
aswaterman added a commit that referenced this issue Mar 7, 2018
SyncReadMem.read with an enable signal currently only works in
compatibility mode, where Wires are implicitly initialized to
DontCare.  Fix by explicitly assigning DontCare to the Wire.

This might fix #775.
aswaterman added a commit that referenced this issue Mar 7, 2018
SyncReadMem.read with an enable signal currently only works in
compatibility mode, where Wires are implicitly initialized to
DontCare.  Fix by explicitly assigning DontCare to the Wire.

This might fix #775.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants