Add conservative Chisel module-only extraction and CIRCT-backed module graph support - #130
Add conservative Chisel module-only extraction and CIRCT-backed module graph support#130okcomputer2000 wants to merge 7 commits into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 84c924d0f3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| CHISEL_EXT_TO_LANG = { | ||
| "scala": "chisel", | ||
| "sc": "chisel", | ||
| } |
There was a problem hiding this comment.
When this mapping enables extraction of .scala/.sc Chisel modules, the verification stage still filters candidates through src/verification.py::EXT_TO_LANG, which has no .scala or .sc entries. In a normal full run those extracted modules are therefore never submitted for verification, so Chisel specs can be produced without any checker coverage.
Useful? React with 👍 / 👎.
| if lang_key == "chisel": | ||
| for callee_fqn in global_chisel_edges.get(fqn, set()) if global_chisel_edges else (): |
There was a problem hiding this comment.
When _build_call_graph is called directly outside generate_topdown_layers—for example the entry-selection path at src/entry_reasoning_pipeline.py:365-369—global_chisel_edges is left as None. This branch then takes the Chisel path and continues without adding any module-instantiation edges, so entry reachability treats every Chisel module as a leaf and can omit all submodules from an entry-focused run.
Useful? React with 👍 / 👎.
| chisel_callees = sorted(global_chisel_edges.get(fqn, set())) | ||
| entry[phase_callees_key] = sorted(set(chisel_callees) & phase_fqns) | ||
| entry["all_callees"] = chisel_callees |
There was a problem hiding this comment.
If extra_call_edges supplies a Chisel dependency, _build_call_graph has already merged it into phase_callees/all_callees, but this override replaces those fields with only global_chisel_edges. The generated prompt metadata then drops the supplemental callee/caller relationship, so caller expectations from explicit edge files are not surfaced for Chisel modules.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The PR does not use https://github.com/llvm/circt. All code about supporting a new language should be put under src/languages and register the interface in registry.py.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2015fc560b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| add_definitions(${LLVM_DEFINITIONS}) | ||
|
|
||
| add_subdirectory(include) | ||
| add_subdirectory(lib) |
There was a problem hiding this comment.
When ./install.sh --with-chisel reaches the CMake configure at tools/chisel-circt, this add_subdirectory(lib) requires a tools/chisel-circt/lib/CMakeLists.txt (confirmed by cmake --help-command add_subdirectory), but a repo-wide search of tools/chisel-circt shows no lib/ directory or implementation of the FMAgentChiselCirct target that the plugin links against. As a result the optional Chisel plugin cannot configure/build, so the advertised CIRCT-backed Chisel support is unusable.
Useful? React with 👍 / 👎.
|
|
||
| output_dir = os.path.join(proj_dir, "spec_prompts") | ||
| os.makedirs(output_dir, exist_ok=True) | ||
| global_chisel_edges = chisel_call_edges(proj_dir) |
There was a problem hiding this comment.
When users configure the new CIRCT path via FM_AGENT_CHISEL_CIRCT_INPUT, extraction builds/persists src/languages/chisel.py's .codegraph/chisel_module_graph.json, but this layer step calls the separate legacy src/chisel_backend.py path, which reads only fm_agent/chisel_circt_module_graph.json/extracted-source fallbacks; _build_call_graph then special-cases Chisel and skips the registry edges. In that CIRCT-backed run, any edge only known to the new pass is missing from phase*_callees/all_callees, so generated Chisel layer prompts are not using the authoritative module graph this commit adds.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 261aee6791
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| graph = self.load_graph() | ||
| if graph is not None or not allow_build: | ||
| return graph | ||
| return self.build_graph() |
There was a problem hiding this comment.
When FM_AGENT_CHISEL_CIRCT_INPUT is regenerated or changed after a prior run, this returns the persisted .codegraph/chisel_module_graph.json before ever calling build_graph. Although persist_graph records a fingerprint containing the input mtime/size, load_graph never validates it, so resume/direct API runs can silently use the old module set and edges, causing newly added modules to be excluded from extraction and stale dependencies to drive layer generation.
Useful? React with 👍 / 👎.
| parts = line.replace("(", " ").replace(")", " ").replace("{", " ").split() | ||
| for idx, token in enumerate(parts[:-1]): | ||
| if token == "new": | ||
| name = parts[idx + 1] | ||
| if name in known_names: |
There was a problem hiding this comment.
Without a CIRCT graph, a generic Chisel instantiation such as Module(new Child[Int]) is tokenized as Child[Int], which cannot match the extracted module name Child in known_names. The fallback therefore omits the Top→Child dependency and places both modules in the same layer (losing the caller/callee contract context); package-qualified instantiations have the same failure mode.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 846da77df7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if lang_key == "chisel": | ||
| for callee_fqn in chisel_edges.get(fqn, set()): | ||
| if callee_fqn == fqn: | ||
| continue | ||
| all_callees_map[fqn].add(callee_fqn) | ||
| if callee_fqn in phase_fqns: | ||
| callees_map[fqn].add(callee_fqn) | ||
| callers_map[callee_fqn].add(fqn) | ||
| for edge in extra_edges_by_caller_fqn.get(fqn, ()): | ||
| _add_resolved_extra_edge( | ||
| fqn, | ||
| edge, | ||
| phase_fqns, | ||
| callees_map, | ||
| callers_map, | ||
| all_callees_map, | ||
| edge_aliases_map, | ||
| ) | ||
| continue |
There was a problem hiding this comment.
When an extra-edge rule selects callers through caller.callsite_names rather than an exact FQN, Chisel modules never receive that edge: this branch only applies extra_edges_by_caller_fqn and immediately continues. For example, an edge targeting every module containing CallMe() leaves the corresponding Chisel module disconnected, despite src/call_graph_edges.py defining callsite selectors to apply to every matching function. Fresh evidence beyond the earlier supplemental-edge finding is that the direct-FQN loop is now present here, while the callsite loop used by every non-Chisel path is still absent.
Useful? React with 👍 / 👎.
|
This PR adds Chisel support as a language backend under Current status:
Known limitations:
Validation:
Next steps:
|
2 participants
Summary
This PR adds a conservative Chisel hardware extraction path on top of the current
mainbranch.The first version stays intentionally narrow:
Module,RawModule,BlackBox,ExtModule,MultiIOModuleBundleis treated as context only, not as a standalone unitApproach
The implementation adds a dedicated Chisel path instead of trying to stretch the generic Scala/function extraction flow:
src/main/...production files are not dropped by filename heuristicsThis keeps the Chisel path isolated and minimizes coupling with the existing non-Chisel pipeline.
What is included
What is intentionally not included
Bundleas an independent analysis unitValidation
Tested locally against public Chisel codebases and local sample trees, including:
schoeberl/chisel-examplesucb-bar/chisel2-deprecateducb-bar/chipyardfpga/src/main/scalagenerators/chipyard/src/main/scalagenerators/chipyard/src/main/resources/vsrcAlso validated the relevant FM-Agent pipeline stages locally by running the real
run_extractionandgenerate_topdown_layersflow on a local test project, and stopping before any LLM-driven stages.Current limitations
Bundleis only preserved as surrounding context inside extracted modulesNext steps