Skip to content

AUTO_MEMORIES detection cannot run as shipped - #4544

Merged
maliberty merged 2 commits into
The-OpenROAD-Project:masterfrom
oharboe:auto-memories-detection
Sep 22, 2026
Merged

maliberty merged 2 commits into
The-OpenROAD-Project:masterfrom
oharboe:auto-memories-detection

Conversation

@oharboe

@oharboe oharboe commented Sep 18, 2026

Copy link
Copy Markdown
Collaborator

AUTO_MEMORIES detection cannot run as shipped. Two bugs in
extract_memories.tcl, one commit each, in the order the flow hits them.

flow/Makefile orders the two steps correctly. Detection runs first and
produces the inventory, and gen_memories.py turns that into
results/memories/, which includes blackboxes.txt, the list of modules
synthesis is to blackbox:

$(RESULTS_DIR)/memories_inferred.json: $(VERILOG_FILES) $(SCRIPTS_DIR)/memories/extract_memories.tcl

do-auto-memories: $(RESULTS_DIR)/memories_inferred.json
	... --yosys-json $(RESULTS_DIR)/memories_inferred.json ...

1. The blackbox guard fires on the step that produces the blackbox
list.
extract_memories.tcl reads its sources through
synth_preamble.tcl's read_design_sources, and every frontend branch
of that proc calls auto_memories_blackboxes, which errors when
results/memories/blackboxes.txt is absent. So the pass whose output
that list is derived from fails because the list does not exist yet.
The first commit clears AUTO_MEMORIES for the extraction process only.

2. proc reaches Tcl, not yosys. yosys -import cannot shadow a Tcl
built-in, and proc is the one that defines a procedure, so the bare
word defines a procedure instead of running the pass:

ERROR: TCL interpreter returned an error:
wrong # args: should be "proc name args body"

memory -nomap on the next line is unaffected, because memory is not
a Tcl built-in. The second commit qualifies the call as yosys proc,
which is how the flow already disambiguates elsewhere
(yosys read_slang in synth_preamble.tcl).

Either bug stops the pass on its own, and the second is reached on every
run, so the detection pass cannot have completed as shipped.

🤖 Generated with Claude Code

oharboe and others added 2 commits September 18, 2026 11:28
AUTO_MEMORIES is a two step flow and flow/Makefile orders it that way.
extract_memories.tcl produces results/memories_inferred.json, and
gen_memories.py turns that into results/memories/, which includes
blackboxes.txt, the list of modules synthesis is to blackbox.

extract_memories.tcl reads its sources through synth_preamble.tcl's
read_design_sources, and every frontend branch of that proc calls
auto_memories_blackboxes, which errors when
results/memories/blackboxes.txt is absent:

  ERROR: AUTO_MEMORIES=1 but .../results/memories/blackboxes.txt is
  missing; the do-auto-memories step must run before synthesis

So the pass whose output that list is derived from fails because the
list does not exist yet. The guard is right for synthesis, where
reaching it without a blackbox list means a step was skipped, and wrong
for the detection pass, which is the one caller that has to tolerate
its absence.

Clear AUTO_MEMORIES for the extraction process only. Nothing else
clears it, so the guard keeps its full strength everywhere else.
Blackboxing during detection would be wrong on its own terms: a
blackboxed module has no memory to infer, so the pass would inventory
nothing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
extract_memories.tcl runs under yosys's Tcl shell and asks for the
process conversion pass by writing

    proc
    memory -nomap

yosys -import (synth_preamble.tcl line 1) imports yosys commands into
the Tcl namespace, but it cannot shadow a Tcl built-in, and proc is the
one that defines a procedure. The bare word reaches Tcl instead of
yosys and the pass dies:

    ERROR: TCL interpreter returned an error:
    wrong # args: should be "proc name args body"

memory -nomap on the next line is unaffected because memory is not a
Tcl built-in, which is why only one of the two needs qualifying.

The yosys prefix is how the rest of the flow already disambiguates:
synth_preamble.tcl calls yosys read_slang for the same reason.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request updates the extract_memories.tcl script to temporarily disable AUTO_MEMORIES by setting set ::env(AUTO_MEMORIES) 0 before reading design sources, preventing errors when the blackboxes file does not yet exist. Additionally, it replaces the bare proc command with yosys proc to ensure the Yosys command is executed rather than defining a Tcl procedure. There are no review comments, and I have no feedback to provide.

@openroad-ci

openroad-ci commented Sep 18, 2026 •

Copy link
Copy Markdown
Member

🔍 QoR check

Metrics reflect the PR merge build — i.e. what will land on the target branch.

Commit 8f986f2 · Jenkins build #2 · Baseline: build · View build on dashboard

62 design(s) checked — 0 with regression(s), 1 without a comparable baseline.

@oharboe
oharboe requested a review from maliberty September 18, 2026 17:43
@maliberty
maliberty merged commit 2d29bda into The-OpenROAD-Project:master Sep 22, 2026
10 of 12 checks passed
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