Probe the last word when num_words is not a power of 2

[TheAnimatrix]

lib.py and sram.save() characterize at the address with every bit high. That address is a real word only when num_words is a power of 2. With 24 words there are 5 address bits, so the probe is word 31 and characterization fails with Could not find bl net in timing paths. This is the failure in #251 (96 and 100 words), which was folded into #228.

This adds get_probe_address() to charutils.py and uses it in both places. Without spare rows it returns num_words - 1, which is still all ones for power-of-2 sizes, so existing results do not change. The spare-row branch is unchanged because 0 followed by all ones is always below num_words.

#282 already fixed which end of the address the column bits are read from. This is the part it does not cover.

Where we hit it

sky130 macro with 192 words, words_per_row=4, no spare rows. All ones is word 255, past the last word (191). With this change the stimulus probes word 191, row 47, mux column 3.

Tests on dev (3608704)

New 23_lib_sram_nonpow2_test.py builds a 2-bit, 24-word SRAM, checks the probe address, and runs SPICE characterization through lib. On scn4m_subm it fails after 1 s on dev with Could not find bl net in timing paths and passes with this change. On sky130 it timed out after 40 minutes both before and after the change on my machine, so I have no sky130 result for it. sky130 uses spare rows in this test, so it takes the unchanged branch.

Test	Tech	Before	After
23_lib_sram_nonpow2	scn4m_subm	fail (bl net)	pass
21_ngspice_delay	scn4m_subm	pass	pass
21_ngspice_delay_extra_rows	scn4m_subm	pass	pass
22_sram_1bank_nomux_func	scn4m_subm	pass	pass
22_sram_1bank_nomux_func	sky130	fail	fail
23_lib_sram	scn4m_subm	golden mismatch	golden mismatch

Tools were ngspice, Magic 8.3.363 and netgen from the conda environment. I did not run the full regression.

compiler/gen_stimulus.py has the same all-ones probe, but that script is already broken in other ways (it reads self.num_spare_rows at module level), so I left it out of this PR.

[mguthaus]

Thanks — the diagnosis and the fix are both right, and I reproduced everything you described. I'd like one change to the test before merging, described at the bottom.

The fix is verified

I reproduced the failure on unmodified dev (3608704) with a 24-word, 2-bit scn4m_subm SRAM: addr_size is 5, dev probes 11111 = 31, past the last word 23, and characterization stops with

ERROR: file simulation.py: line 613: Could not find bl net in timing paths.

exactly as in #251/#228.

I then checked get_probe_address() across a matrix of configurations, including the two sizes from #251:

words	words_per_row	addr_size	probe	decimal	old (all ones)
16	1	4	1111	15	15	unchanged (power of 2)
64	1	6	111111	63	63	unchanged
128	2	7	1111111	127	127	unchanged
24	1	5	10111	23	31	fixed
17	1	5	10000	16	31	fixed
96	2	7	1011111	95	127	fixed (#251)
100	4	7	1100011	99	127	fixed (#251)
192	4	8	10111111	191	255	fixed (your example)
16, 24	1 (spare rows)	5	01111	15	—	spare-row branch, unchanged

In every case the probe is below num_words and exactly addr_size bits wide, and your backward-compatibility claim holds: for power-of-2 sizes the address is bit-for-bit what dev produced. That is also guaranteed, since addr_size = ceil(log2(num_rows)) + log2(words_per_row) + log2(num_banks) gives 2**addr_size >= num_words, so num_words - 1 always fits.

Your new test passes on freepdk45 as well as scn4m_subm here, which you hadn't been able to check. Full regression on both technologies (30 tests): identical pass/fail to dev, plus the two new passes.

The one change I'd like

compiler/tests/Makefile picks tests up with TEST_SRCS = $(wildcard *_test.py) and runs them for all of TECHS = scn4m_subm freepdk45 sky130. The four sibling sky130 entries are excluded:

sky130/23_lib_sram_linear_regression_test.ok \
sky130/23_lib_sram_model_corners_test.ok \
sky130/23_lib_sram_model_test.ok \
sky130/23_lib_sram_test.ok \

but sky130/23_lib_sram_nonpow2_test.ok is not, so the regression would pick up the 40-minute sky130 run you measured. Two ways to fix it, and I'd prefer the second:

1. Add sky130/23_lib_sram_nonpow2_test.ok \ to BROKEN_STAMPS next to those four.

2. Better: drop the lib(...) call and keep the test as a fast check of the address itself. The SPICE path is already covered by 23_lib_sram_test, and what regressed in #251 was the address, not the characterization. Something like:

probe_address = get_probe_address(s.s)
self.assertLess(int(probe_address, 2), c.num_words)
self.assertEqual(len(probe_address), s.s.addr_size)
if num_spare_rows == 0:
    self.assertEqual(probe_address, "10111")

That runs in seconds on all three technologies, needs no simulator, and still fails on dev. If you take this route the test no longer needs OPTS.analytical_delay = False, the reload(characterizer) dance, or the OPTS.spice_exe check.

Either is fine with me — option 2 just gives sky130 real coverage of this instead of an exclusion. Ping me when you push and I'll re-run and merge.

One aside: compiler/gen_stimulus.py having the same all-ones probe is worth a separate issue so it isn't lost, even though you rightly kept it out of this PR.

[TheAnimatrix]

went with option 2, pushed caa4169. test just builds the netlist and checks the address now (below num_words, addr_size bits, 10111 for the no spare rows case). dropped the lib() call and the reload hack

~20s on sky130, ~1s on the other two. fails on dev with an import error as expected

will open a separate issue for gen_stimulus

[mguthaus]

Merged. Thanks for reworking the test — that is exactly what I was after.

Verified on all three technologies that build here (freepdk45, scn4m_subm, sky130 with sky130_fd_bd_sram fc63b128 installed):

	freepdk45	scn4m_subm	sky130
23_lib_sram_nonpow2_test	PASS, 1.0 s	PASS, 1.2 s	PASS, 1.1 s

So the 40-minute sky130 run is gone and no BROKEN_STAMPS entry is needed — which is the better outcome, since sky130 now actually gets coverage of this instead of an exclusion.

The rest of the run was unchanged against dev: 21_ngspice_delay_test, 23_lib_sram_prune_test, 20_sram_1bank_nomux_test and 30_openram_front_end_test all pass on freepdk45 and scn4m_subm, and 23_lib_sram_prune_test passes on sky130.

For the record, the fix itself I had already checked in detail: I reproduced the failure on clean dev (24 words gives addr_size 5, probe 11111 = 31 against last word 23, stopping in simulation.py:613 with Could not find bl net in timing paths), and confirmed get_probe_address() across ten configurations — including both sizes from #251 — always lands below num_words, is exactly addr_size bits wide, and is bit-for-bit unchanged for power-of-2 sizes.

Thanks also for filing #326 for the gen_stimulus.py probe separately.

[TheAnimatrix]

thanks for the thorough re-run, and for the fast turnaround on all of these honestly. didnt expect a review this detailed on a 20 line change. openram is a big deal for anyone trying to do open silicon with real memories so im glad to be able to put something back into it