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

[FIRRTL] Can not canonicalize away memories without more care #7880

Open
youngar opened this issue Nov 22, 2024 · 0 comments
Open

[FIRRTL] Can not canonicalize away memories without more care #7880

youngar opened this issue Nov 22, 2024 · 0 comments
Labels
bug Something isn't working FIRRTL Involving the `firrtl` dialect

Comments

@youngar
Copy link
Member

youngar commented Nov 22, 2024

I have some doubts about the validity of the canonicalizer in #7868

This doesn't seem safe to do when we are running with repl-seq-mems, which should mock out the memory with an extmodule. Instead it is lowered to a register and no memory shows up in the metadata.

./bin/firtool --disable-all-randomization --repl-seq-mem --repl-seq-mem-file=mems.txt ./mem.fir

FIRRTL version 4.1.0
circuit Foo:
  public module Foo:
    input clk : Clock
    input r : UInt<1>
    output rdata : UInt<8>
    input wdata : UInt<8>
    input addr : UInt<0>
    input en : UInt<1>
    input mode : UInt<1>


    mem mem:
      data-type => UInt<8>
      depth => 1
      readwriter => rw
      read-latency => 1
      write-latency => 1
      read-under-write => undefined

    connect mem.rw.clk, clk
    connect mem.rw.en, en
    connect mem.rw.addr, addr
    connect mem.rw.wmode, mode
    connect mem.rw.wmask, UInt<8>(-1)
    connect mem.rw.wdata, wdata
    connect rdata, mem.rw.rdata

gives:

// ----- 8< ----- FILE "metadata/seq_mems.json" ----- 8< -----

[]

// ----- 8< ----- FILE "mems.txt" ----- 8< -----

We should also make sure that this is working correctly for cmems, especially that it is giving identical results when we are using MemToRegOfVec, FullAsynchronousResetTransform, PrefixModules, etc.

@youngar youngar added bug Something isn't working FIRRTL Involving the `firrtl` dialect labels Nov 22, 2024
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
Projects
None yet
Development

No branches or pull requests

1 participant