llvm / llvm/circt

[FIRRTL] Preserve aggregate of memory data type to make LEC friendly

Open
#9,076 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C++
Stars
2.2k
Forks
524
Avg merge
3d 2h
Merged PRs (30d)
46

Description

Currently a data type of a memory is flattened to an integer. However combined with unused field removal https://github.com/llvm/circt/blob/11ad8a882fbcacb75c40fbf9c946eb35e2e43c89/lib/Dialect/FIRRTL/FIRRTLFolds.cpp#L2812, sometimes it makes difficult to verify LEC between two versions.

Example:
```scala
FIRRTL version 5.0.0
circuit Dut_DV:
extmodule unknown_val:
output out: UInt<1>
public module Dut_DV :
input clock : Clock
input reset : UInt<1>
output io : { flip addr : UInt<3>, flip dataIn : {a: UInt<8>, b: UInt<8>}, flip wen : UInt<1>, dataOut : {a: UInt<8>, b: UInt<8>}}

inst inst of unknown_val

cmem rf : {a: UInt<8>, unused: UInt<1>, b: UInt<8>} [8]
infer mport read = rf[io.addr], clock
connect io.dataOut.a, read.a
connect io.dataOut.b, read.b
when io.wen :
infer mport write = rf[io.addr], clock
connect write.a, io.dataIn.a
connect write.b, io.dataIn.b
connect write.unused, inst.out

public module DUT_DV_STRIPPED:
input clock : Clock
input reset : UInt<1>
output io : { flip addr : UInt<3>, flip dataIn : {a: UInt<8>, b:UInt<8>}, flip wen : UInt<1>, dataOut : {a: UInt<8>, b:UInt<8>}}

cmem rf : {a: UInt<8>, b:UInt<8>} [8]
infer mport read = rf[io.addr], clock
connect io.dataOut, read
when io.wen :
infer mport write = rf[io.addr], clock
connect write, io.dataIn
```

```verilog
module rf_8x17(...);
reg [16:0] Memory[0:7];
...
endmodule
module Dut_DV(
...
rf_8x17 rf_ext (...)
endmodule

module rf_8x16(...);
reg [15:0] Memory[0:7];
...
endmodule
module DUT_DV_STRIPPED(
...
rf_8x16 rf_ext (...)
endmoule
```
Here `Dut_DV.rf_ext.Memory[0:7]` is equivalent to `DUT_DV_STRIPPED.rf_ext.Memory[0:7]` but `Dut_DV.rf_ext.Memory[16:9]` corresponds to `DUT_DV_STRIPPED.rf_ext.Memory[15:8]`. LEC tool would (rightly) match `Dut_DV.rf_ext.Memory[15:8]` = `DUT_DV_STRIPPED.rf_ext.Memory[15:8]` so it will cause LEC failure.

If we lowered the Memory to a packed struct, LEC tool can correctly infer the matching based on field name.
```verilog
module rf_8x17(...);
struct packed {logic [7:0] a;logic unused; logic [7:0]b;} Memory[0:7];
endmodule

module rf_8x16(...);
struct packed {logic [7:0] a; logic [7:0]b;} Memory[0:7];
endmodule
```

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in lib/Dialect/FIRRTL/FIRRTLFolds.cpp around the unused-field removal logic referenced by the issue, then trace where memory data types are flattened. Use the FIRRTL example and generated Verilog shown in the issue to verify that aggregate memory fields remain identifiable for LEC matching, including when an unused field is removed.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
compilers
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.