Fix invalid references generated by VerilogMemDelays

[Alan-Liang]

Transformation of mem readwriters whose address contain references to readwriters of mems declared before it would contain invalid references to untransformed memory readwriter, as the connection is not transformed. This commit fixes this issue.

In MemDelayAndReadwriteTransformer, the transform method traverses all memories in the module and finds connections to them. Other statements are checked for references of the old readwriters and these references will be swapped out for the new refs. However, the port connections of the memories themselves are not checked, while they may contain references to readwriters of other memories. For example:

circuit Test :
  module Test :
    input clock : Clock
    input sel : UInt<1>
    input en : UInt<1>
    output v1 : UInt<1>
    output v2 : UInt<1>

    mem m1 :
      data-type => UInt<1>
      depth => 2
      read-latency => 0
      write-latency => 1
      readwriter => rw1
      readwriter => rw2
      read-under-write => undefined
    mem m2 :
      data-type => UInt<1>
      depth => 2
      read-latency => 0
      write-latency => 1
      readwriter => rw1
      readwriter => rw2
      read-under-write => undefined
    v1 <= m1.rw2.rdata
    v2 <= m2.rw2.rdata
    m1.rw1.addr <= UInt<1>("h0")
    m2.rw1.addr <= UInt<1>("h0")
    m1.rw1.en <= UInt<1>("h1")
    m2.rw1.en <= UInt<1>("h1")
    m1.rw1.clk <= clock
    m2.rw1.clk <= clock
    m1.rw1.wmode <= en
    m2.rw1.wmode <= en
    m1.rw1.wdata <= UInt<1>("h1")
    m2.rw1.wdata <= UInt<1>("h0")
    m1.rw1.wmask <= en
    m2.rw1.wmask <= UInt<1>("h0")
    m1.rw2.addr <= m2.rw1.rdata
    m2.rw2.addr <= m2.rw1.rdata
    m1.rw2.en <= UInt<1>("h1")
    m2.rw2.en <= UInt<1>("h1")
    m1.rw2.clk <= clock
    m2.rw2.clk <= clock
    m1.rw2.wmode <= en
    m2.rw2.wmode <= en
    m1.rw2.wdata <= UInt<1>("h0")
    m2.rw2.wdata <= UInt<1>("h0")
    m1.rw2.wmask <= en
    m2.rw2.wmask <= UInt<1>("h0")

Prior to this change, firrtl would generate

circuit Test :
  module Test :
    input clock : Clock
    input sel : UInt<1>
    input en : UInt<1>
    output v1 : UInt<1>
    output v2 : UInt<1>

    mem m1 :
      data-type => UInt<1>
      depth => 2
      read-latency => 0
      write-latency => 1
      reader => rw1_r
      reader => rw2_r
      writer => rw1_w
      writer => rw2_w
      read-under-write => undefined
    mem m2 :
      data-type => UInt<1>
      depth => 2
      read-latency => 0
      write-latency => 1
      reader => rw1_r
      reader => rw2_r
      writer => rw1_w
      writer => rw2_w
      read-under-write => undefined
    ; unrelated things omitted
    m1.rw2_r.addr <= m2.rw1.rdata ; <-- bad reference to old readwriter
    ; unrelated things omitted
    m1.rw2_w.addr <= m2.rw1.rdata ; <-- bad reference to old readwriter
    ; unrelated things omitted

which contains m2.rw1.rdata, a reference to the old readwriter, and would generate invalid Verilog code. This change swaps out these references after all other statements are generated, so these references are updated to use the reader that we just splitted out.

This might have other side effects on codegen that I am not aware of, as I am really new to Chisel/FIRRTL.

Contributor Checklist

• [N/A] Did you add Scaladoc to every public function/method? (no functions added)
• [N/A] Did you update the FIRRTL spec to include every new feature/behavior? (no features added)
• Did you add at least one test demonstrating the PR?
• Did you delete any extraneous printlns/debugging code?
• Did you specify the type of improvement?
• Did you state the API impact?
• Did you specify the code generation impact?
• Did you request a desired merge strategy?
• Did you add text to be included in the Release Notes for this change? (I have no idea what release notes would look like, but I've included one for reference.)

Type of Improvement

bug fix

API Impact

None

Backend Code Generation Impact

Generated code may change when the driver of connections on readwriters contained references to readwriters of other memories.

Desired Merge Strategy

• Squash: The PR will be squashed and merged (choose this if you have no preference.

Release Notes

Fix invalid references generated by VerilogMemDelays

Reviewer Checklist (only modified by reviewer)

• Did you add the appropriate labels?
• Did you mark the proper milestone (1.2.x, 1.3.0, 1.4.0) ?
• Did you review?
• Did you check whether all relevant Contributor checkboxes have been checked?
• Did you mark as Please Merge?

[CLAassistant]

All committers have signed the CLA.

[Alan-Liang]

Would @jackkoenig or someone review this?

[jackkoenig]

LGTM Thank you!

Sorry for missing this originally and thank you for the fix!

[seldridge]

@Alan-Liang: Would you be able to also sign the CLA of the Chisel repository? (There should be a link in this comment: chipsalliance/chisel#2982 (comment)). I'm in the process of pulling all of this repository into Chisel and I just grabbed this change. Thanks!

[Alan-Liang]

Done!

[seldridge]

Perfect. Thanks!