Skip to content

Conversation

@oharboe
Copy link
Collaborator

@oharboe oharboe commented Oct 27, 2025

@povik Is there a reasonable way to implement such a SRAM mocking feature?

Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
set size [memory_get $module $mem SIZE]

if {$size > $::env(SYNTH_MEMORY_MAX_BITS)} {
memory_set $module $mem SIZE 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@oharboe Is memory_set a hypothetical command?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see there are other dreamed up commands. Let me see if this can be implemented using real commands

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. In ChatGPT's defense it did say that it dreamt up these commands and then it looked for alternatives.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I offer this for testing:

memory_collect
foreach path [tee -q -s result.string select -list t:\$mem_v2] {
	set index [string first "/" $path]
	set module [string range $path 0 [expr {$index - 1}]]
	set instance [string range $path [expr {$index + 1}] end]

	set width [rtlil::get_param -uint $module $instance WIDTH]
	set size [rtlil::get_param -uint $module $instance SIZE]
	set nbits [expr $width * $size]
	puts "Memory $path has dimensions $size x $width = $nbits"
	if {$nbits > $::env(SYNTH_MEMORY_MAX_BITS)} {
		rtlil::set_param -uint $module $instance SIZE 1
		puts "Shrunk memory $path from $size rows to 1"
	}
}

Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
@oharboe
Copy link
Collaborator Author

oharboe commented Oct 27, 2025

@povik Mysterious error:

make DESIGN_CONFIG=designs/sky130hd/microwatt/config.mk synth
7. Executing MEMORY_COLLECT pass (generating $mem cells).
aminfr' which is not allowed in RTLIL identifiers string '$paramod$5409023ed62e51a90c7a1a5e2fb5908f52c323ea
Command exited with non-zero status 1
Elapsed time: 0:25.30[h:]min:sec. CPU time: user 25.08 sys 0.16 (99%). Peak memory: 149052KB.
make[1]: *** [Makefile:262: do-yosys] Error 1
make: *** [Makefile:272: results/sky130hd/microwatt/base/1_2_yosys.v] Error 2

@povik
Copy link
Contributor

povik commented Oct 27, 2025

I've found the message template in Yosys. It warns about a special character but at the same time it prints the string containing the special character without escaping hence the mangled output.

log_error("Found control character or space (0x%02x) in string '%s' which is not allowed in RTLIL identifiers\n", *c, p);

I'll take a look to see how to fix the script.

@oharboe
Copy link
Collaborator Author

oharboe commented Oct 28, 2025

@povik Purely for my planning purposes, how long do you think you'll need to see if there's a fix for this PR? Days, weeks, months?

If it is very far out, then I think I should close the issue and re-open when we have what we need.

@povik
Copy link
Contributor

povik commented Oct 28, 2025

@oharboe a day or two

@povik
Copy link
Contributor

povik commented Nov 7, 2025

@oharboe needs the fix below (can't push to your branch):

commit f467f5eb2dc99a914e336a54347c9dbb3f458806 (HEAD -> synth-mock-memories)
Author: Martin Povišer <povik@cutebit.org>
Date:   Fri Nov 7 15:22:19 2025 +0100

    synth: Fix foreach doing unwanted substitutions
    
    Signed-off-by: Martin Povišer <povik@cutebit.org>

diff --git a/flow/scripts/synth.tcl b/flow/scripts/synth.tcl
index 9875aed67..013f7b719 100644
--- a/flow/scripts/synth.tcl
+++ b/flow/scripts/synth.tcl
@@ -58,7 +58,8 @@ if { !$::env(SYNTH_HIERARCHICAL) } {
 
 if { $::env(SYNTH_MOCK_LARGE_MEMORIES) } {
   memory_collect
-  foreach path [tee -q -s result.string select -list t:\$mem_v2] {
+  set select [tee -q -s result.string select -list t:\$mem_v2]
+  foreach path [split [string trim $select] "\n"] {
     set index [string first "/" $path]
     set module [string range $path 0 [expr {$index - 1}]]
     set instance [string range $path [expr {$index + 1}] end]

povik and others added 3 commits November 7, 2025 15:34
Signed-off-by: Martin Povišer <povik@cutebit.org>
Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
@oharboe oharboe requested review from maliberty and povik November 7, 2025 14:41
@oharboe
Copy link
Collaborator Author

oharboe commented Nov 7, 2025

@maliberty Thoughts?

@oharboe
Copy link
Collaborator Author

oharboe commented Nov 7, 2025

@jeffng-or Thoughts?

Memories with a single 1 row will of course have unrealistically good
timing and area characteristics, but timing will still correctly terminate
in a register.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me also point out a concern that some of the logic driving the address input on the RAM will be optimized out when we override the RAM size

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm... I think these sort of downstream consequences are evident to anyone skilled in the art given the warning in the doc?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possibly. In my experience people are surprised by what can get optimized out when they mock something in their design

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see.... The idea is that whatever problems remain after mocking, they are real place/route/timing closure problems and worth sorting out while waiting for real RAM or some more accurate fake RAM.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps a more stern comment to the effect "you asked for it and you deserve the result" would be helpful, but I think it is fine the way it is.

Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
@oharboe
Copy link
Collaborator Author

oharboe commented Nov 9, 2025

SYNTH_MOCK_LARGE_MEMORIES only work for memories that are inferred by Yosys, the empty RAM512.v is not a memory that is inferred by Yosys. So neither fakeram nor these these gutted RAMs that I've seen in a couple of places are inferred memories by Yosys and therefore not affected by SYNTH_MOCK_LARGE_MEMORIES.

Some working notes before we talk Monday.

$ cat reports/sky130hd/microwatt/base/synth_mocked_memories.txt
decode1_0_bf8b4530d8d246dd74ac53a13471bba17941dff7:
  width: 39
  size: 64
decode1_0_bf8b4530d8d246dd74ac53a13471bba17941dff7:
  width: 1
  size: 2048
decode1_0_bf8b4530d8d246dd74ac53a13471bba17941dff7:
  width: 41
  size: 1024
decode1_0_bf8b4530d8d246dd74ac53a13471bba17941dff7:
  width: 17
  size: 512
fpu:
  width: 18
  size: 1024

The modules above with inferred memories are now kept by default:

image

Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
@oharboe
Copy link
Collaborator Author

oharboe commented Nov 9, 2025

Tried this on a larger design that I can't share, worked very well. I was able to get Yosys to assert for a 2048+ bits wide 2 row deep memory, but by tweaking configuration settings, I made it past that snag.

ERROR: Assert `!(size & mask)' failed in kernel/mem.cc:540.

@oharboe
Copy link
Collaborator Author

oharboe commented Nov 9, 2025

@povik The assert isn't holding me up, but I've left confidential instructions with @maliberty on how to reproduce it in case there's a need to fix it.

@oharboe
Copy link
Collaborator Author

oharboe commented Nov 9, 2025

Testing CoralNPU:

bazelisk build //fpga:rtl_files

Now do a quick build to see what we have...

make SYNTH_HIERARCHICAL=1 SYNTH_MEMORY_MAX_BITS=1024 SYNTH_MINIMUM_KEEP_SIZE=0 SYNTH_HDL_FRONTEND=slang  DESIGN_CONFIG=designs/asap7/minimal/config.mk DESIGN_NAME=SRAM_2048x128 VERILOG_FILES=~/coralnpu/bazel-bin/fpga/ip/coralnpu_chisel_subsystem_default/CoralNPUChiselSubsystem.sv do-yosys-canonicalize do-yosys

This lists memories that are too large:

(running)

@povik Hmmm... I wonder if slang has difficulty inferring memories. The above doesn't infer a memory.

@oharboe
Copy link
Collaborator Author

oharboe commented Nov 9, 2025

Yosys slang may be having trouble inferring memories.

untar yosys-no-memories.tar.gz

Run:

make --file=~/OpenROAD-flow-scripts/flow/Makefile SYNTH_HIERARCHICAL=0 SYNTH_MEMORY_MAX_BITS=1024 SYNTH_HDL_FRONTEND=slang  DESIGN_CONFIG=~/OpenROAD-flow-scripts/flow/designs/asap7/minimal/config.mk DESIGN_NAME=Spi2TLUL  SDC_CONSTRAINTS=~/OpenROAD-flow-scripts/flow/designs/asap7/mock-alu/constraints.sdc VERILOG_FILES=./home/oyvind/coralnpu/bazel-bin/fpga/ip/coralnpu_chisel_subsystem_default/CoralNPUChiselSubsystem.sv SDC_CONSTRAINTS=designs/asap7/mock-alu/constraints.sdc do-yosys-canonicalize do-yosys

I expected this error to kick in:

# Run report and check here so as to fail early if this synthesis run is doomed
exec -- $::env(PYTHON_EXE) $::env(SCRIPTS_DIR)/mem_dump.py \
--max-bits $::env(SYNTH_MEMORY_MAX_BITS) $::env(RESULTS_DIR)/mem.json

f"Error: Synthesized memory size {args.max_bits} exceeds SYNTH_MEMORY_MAX_BITS"

But it doesn't...

[deleted]
3.16. Executing MEMORY pass.
3.16.1. Executing OPT_MEM pass (optimize memories).
3.16.2. Executing OPT_MEM_PRIORITY pass (removing unnecessary memory write priority relations).
3.16.3. Executing OPT_MEM_FEEDBACK pass (finding memory read-to-write feedback paths).
3.16.4. Executing MEMORY_BMUX2ROM pass (converting muxes to ROMs).
3.16.5. Executing MEMORY_DFF pass (merging $dff cells to $memrd).
3.16.6. Executing OPT_CLEAN pass (remove unused cells and wires).
3.16.7. Executing MEMORY_SHARE pass (consolidating $memrd/$memwr cells).
3.16.8. Executing OPT_MEM_WIDEN pass (optimize memories where all ports are wide).
3.16.9. Executing OPT_CLEAN pass (remove unused cells and wires).
3.16.10. Executing MEMORY_COLLECT pass (generating $mem cells).
3.17. Executing OPT_CLEAN pass (remove unused cells and wires).

@oharboe
Copy link
Collaborator Author

oharboe commented Nov 9, 2025

@povik FYI, unrelated to this PR as such, but when using SYNTH_HDL_FRONTEND=slang, yosys fails to infer memories.

Untar slang-no-infer-mem.tar.gz

Run the below

make --file=~/OpenROAD-flow-scripts/flow/Makefile SYNTH_HIERARCHICAL=0 SYNTH_MEMORY_MAX_BITS=1024   DESIGN_CONFIG=~/OpenROAD-flow-scripts/flow/designs/asap7/minimal/config.mk DESIGN_NAME=SRAM_2048x128  SDC_CONSTRAINTS=~/OpenROAD-flow-scripts/flow/designs/asap7/mock-alu/constraints.sdc VERILOG_FILES=blah.sv SDC_CONSTRAINTS=designs/asap7/mock-alu/constraints.sdc do-yosys-canonicalize do-yosys

Fails quickly, as expected, memories are inferred:

ERROR: TCL interpreter returned an error: Source files actually used in the design:
 blah.sv
Memories found in the design:
 Rows | Width |   Bits | Module               | Instances                                                                       
---------------------------------------------------------------------------------------------------------------------------------
 2048 |   128 |      0 | SRAM_2048x128        |                                                                             

Rerun the above, but with SYNTH_HDL_FRONTEND=slang:

  1. takes "forever", instead of seconds, indicative that a ton of logic instead of memory is detected
  2. doesn't produce the error message above

@povik
Copy link
Contributor

povik commented Nov 9, 2025

@oharboe I now recall we use the --no-implicit-memories flag with slang. This disables inference of memories which for most use cases is fine but it prevents the SYNTH_MEMORY_MAX_BITS and SYNTH_MOCK_LARGE_MEMORIES options from working.

We might be able to drop the flag. It was a workaround for an issue which may have been fixed since.

@povik
Copy link
Contributor

povik commented Nov 9, 2025

The flag is used with read_slang in synth_preamble.tcl

@oharboe
Copy link
Collaborator Author

oharboe commented Nov 10, 2025

Recording notes while I kick off a larger run...

diff --git a/flow/scripts/synth_preamble.tcl b/flow/scripts/synth_preamble.tcl
index a37231275..c20bb2836 100644
--- a/flow/scripts/synth_preamble.tcl
+++ b/flow/scripts/synth_preamble.tcl
@@ -47,7 +47,7 @@ proc read_design_sources { } {
     # slang requires all files at once
     plugin -i slang
     yosys read_slang -D SYNTHESIS --keep-hierarchy --compat=vcs \
-      --ignore-assertions --no-implicit-memories --top $::env(DESIGN_NAME) \
+      --ignore-assertions --top $::env(DESIGN_NAME) \
       {*}$vIdirsArgs {*}$::env(VERILOG_FILES) {*}[env_var_or_empty VERILOG_DEFINES]
     # Workaround for yosys-slang#119
     setattr -unset init

After 15 minutes:

make SYNTH_HIERARCHICAL=1 SYNTH_MEMORY_MAX_BITS=1024 SYNTH_MINIMUM_KEEP_SIZE=0 SYNTH_HDL_FRONTEND=slang  DESIGN_CONFIG=designs/asap7/minimal/config.mk DESIGN_NAME=CoralNPUChiselSubsystem VERILOG_FILES=~/coralnpu/bazel-bin/fpga/ip/coralnpu_chisel_subsystem_default/CoralNPUChiselSubsystem.sv do-yosys-canonicalize do-yosys
[deleted]
 Rows | Width |   Bits | Module               | Instances                                                                       
---------------------------------------------------------------------------------------------------------------------------------
 2048 |   128 | 262144 | Sram_2048x128$CoralNPUChiselSubsystem.instantiatedModules_0_2.coreAxi.dtcm.sram.sramModules_0 | CoralNPUChiselSubsystem.instantiatedModules_0_2.rvv_coreTlul$CoralNPUChiselSubsystem.instantiatedModules_0_2.coreAxi.rvv_coreAxi$CoralNPUChiselSubsystem.instantiatedModules_0_2.coreAxi.dtcm.TCM128_1$CoralNPUChiselSubsystem.instantiatedModules_0_2.coreAxi.dtcm.sram.SRAM_2048x128$CoralNPUChiselSubsystem.instantiatedModules_0_2.coreAxi.dtcm.sram.sramModules_0
  512 |   128 |  65536 | Sram_512x128$CoralNPUChiselSubsystem.instantiatedModules_0_2.coreAxi.itcm.sram.sramModules_0 | CoralNPUChiselSubsystem.instantiatedModules_0_2.rvv_coreTlul$CoralNPUChiselSubsystem.instantiatedModules_0_2.coreAxi.rvv_coreAxi$CoralNPUChiselSubsystem.instantiatedModules_0_2.coreAxi.itcm.TCM128$CoralNPUChiselSubsystem.instantiatedModules_0_2.coreAxi.itcm.sram.SRAM_512x128$CoralNPUChiselSubsystem.instantiatedModules_0_2.coreAxi.itcm.sram.sramModules_0
  256 |   128 |  32768 | read_data_buffer_sram_256x128$CoralNPUChiselSubsystem.instantiatedModules_1_2.read_data_buffer_sram_ext | CoralNPUChiselSubsystem.instantiatedModules_1_2.Spi2TLUL$CoralNPUChiselSubsystem.instantiatedModules_1_2.read_data_buffer_sram_ext
  256 |   128 |  32768 | write_data_buffer_sram_256x128$CoralNPUChiselSubsystem.instantiatedModules_1_2.write_data_buffer_sram_ext | CoralNPUChiselSubsystem.instantiatedModules_1_2.Spi2TLUL$CoralNPUChiselSubsystem.instantiatedModules_1_2.write_data_buffer_sram_ext
[deleted]
Error: Synthesized memory size 1024 exceeds SYNTH_MEMORY_MAX_BITS
Command exited with non-zero status 1
Elapsed time: 14:57.75[h:]min:sec. CPU time: user 896.68 sys 0.82 (99%). Peak memory: 1071536KB.

Next run to see if we can get this past synthesis so we can check the sizes of the various modules in gui_synth:

make SYNTH_MOCK_LARGE_MEMORIES=1 SYNTH_HIERARCHICAL=1 SYNTH_MEMORY_MAX_BITS=1024 SYNTH_MINIMUM_KEEP_SIZE=0 SYNTH_HDL_FRONTEND=slang  DESIGN_CONFIG=designs/asap7/minimal/config.mk DESIGN_NAME=CoralNPUChiselSubsystem VERILOG_FILES=~/coralnpu/bazel-bin/fpga/ip/coralnpu_chisel_subsystem_default/CoralNPUChiselSubsystem.sv do-yosys-canonicalize do-yosys

Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
@@ -0,0 +1,26 @@
# Tips on building large design

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@povik Any thoughts on synthesis optimizations that I can turn off to speed up builds?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For instance synthesis is gnawing on some bone here "forever":

8.4.544. Executing OPT_EXPR pass (perform const folding).
8.4.545. Executing OPT_MUXTREE pass (detect dead branches in mux trees).
8.4.546. Executing OPT_EXPR pass (perform const folding).
8.4.547. Executing OPT_MUXTREE pass (detect dead branches in mux trees).
8.4.548. Executing OPT_EXPR pass (perform const folding).
8.4.549. Executing OPT_MUXTREE pass (detect dead branches in mux trees).
8.4.550. Executing OPT_EXPR pass (perform const folding).
8.4.551. Executing OPT_MUXTREE pass (detect dead branches in mux trees).
8.4.552. Executing OPT_EXPR pass (perform const folding).
8.4.553. Executing OPT_MUXTREE pass (detect dead branches in mux trees).
8.4.554. Executing OPT_EXPR pass (perform const folding).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've seen people disable the share pass as runtime intensive (option synth -noshare). In your case it's stuck elsewhere and I don't have a suggestion.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to generate a report with sizes for the modules before the build starts?

That could clue me in to what is going on, much like the max memory bits early failure.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can run stat at the end of the canonicalize step. The printed statistics would be in terms of internal yosys cells but that might be useful too.

Copy link
Collaborator Author

@oharboe oharboe Nov 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we get stat per module and create a table in github .md syntax?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not out of the box but with an investment we could

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filed a feature request: #3645

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's what the canonicalize stat output looks like on the jpeg design: https://gist.github.com/povik/deb2a3c32ef989a5371ce606516661c1

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like it contains all the information, but the formatting is terrible.

If this was in a table tailored to the use-case of finding pathologically large modules, I think we'd have something.

@oharboe
Copy link
Collaborator Author

oharboe commented Nov 10, 2025

This PR is good to go for my part now.

Things are working pretty well. When I push yosys/synthesis hard enough, I, no surprise, find that there is always room for improvement, but that's beyond this PR.

| <a name="SYNTH_HDL_FRONTEND"></a>SYNTH_HDL_FRONTEND| Select an alternative language frontend to ingest the design. Available option is "slang". If the variable is empty, design is read with the Yosys read_verilog command.| |
| <a name="SYNTH_HIERARCHICAL"></a>SYNTH_HIERARCHICAL| Enable to Synthesis hierarchically, otherwise considered flat synthesis.| 0|
| <a name="SYNTH_HIER_SEPARATOR"></a>SYNTH_HIER_SEPARATOR| Separator used for the synthesis flatten stage.| .|
| <a name="SYNTH_KEEP_MOCKED_MEMORIES"></a>SYNTH_KEEP_MOCKED_MEMORIES| Keeping the mocked memories(not flattening them), preserves some of the access logic complexity and avoids optimizations outside of the mocked memory.| 1|
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mention that this only has an effect if SYNTH_MOCK_LARGE_MEMORIES is set.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not addressed

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@oharboe oharboe requested a review from maliberty November 10, 2025 19:09
Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
@maliberty maliberty enabled auto-merge November 10, 2025 22:06
@maliberty maliberty merged commit ba1ee5d into The-OpenROAD-Project:master Nov 11, 2025
8 checks passed
@oharboe oharboe deleted the synth-mock-memories branch November 18, 2025 14:05
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

Successfully merging this pull request may close these issues.

3 participants