Skip to content

Fix: don't log error when getting non-top cells in -bb mode#164

Merged
akashlevy merged 1 commit into
mainfrom
feat/chunk-parallel-sim
May 12, 2026
Merged

Fix: don't log error when getting non-top cells in -bb mode#164
akashlevy merged 1 commit into
mainfrom
feat/chunk-parallel-sim

Conversation

@hsiang20
Copy link
Copy Markdown

Patch for PR#162.
update_cell is called when the update phase traverses all top modules and their submodules. Top modules are updated and the function returns. Submodules don't need to be updated (their values come from FST), so they need to be skipped. The cells that are not in module->design->module should still trigger the error.

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 12, 2026

Greptile Summary

This patch fixes a spurious log_error in -bb (blackbox-children) mode. In that mode, SimInstance's constructor never creates child instances for hierarchical cells, so when update_cell is later called for those cells they fall through to log_error — which is wrong because their outputs have already been sourced from the FST.

  • The old guard only silenced the error for cells whose module name appeared in instance_root_modules, missing every other hierarchical sub-cell visited during the traversal.
  • The new guard returns early for any cell whose type resolves to a module in the design, preserving the error only for truly unknown cell types (where module->design->module(cell->type) returns null).

Confidence Score: 4/5

Safe to merge; the fix eliminates a false log_error without changing any state or data flow.

The change is small and logically consistent with how -bb mode suppresses child SimInstance creation in the constructor. One edge worth noting: the constructor guards with !mod->get_blackbox_attribute(true) before skipping, while update_cell only checks if (mod), so an explicit-blackbox module in the design will also silently skip in -bb mode; this appears intentional but is not called out in the PR description.

passes/sat/sim.cc — only file changed; the logic change is contained to a single guard condition in update_cell.

Important Files Changed

Filename Overview
passes/sat/sim.cc Widens the early-return guard in update_cell to skip all design-known hierarchical cells in -bb mode, not just instance-root modules. Comment updated to reflect the broader intent.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[update_cell called] --> B{ff/formal/mem/children?}
    B -- yes --> C[handle & return]
    B -- no --> D{icg or evaluable cell?}
    D -- yes --> E[evaluate & return]
    D -- no --> F{cell type == $print?}
    F -- yes --> G[return]
    F -- no --> H{blackbox_children flag set?}
    H -- no --> J[log_error: unsupported cell]
    H -- yes --> I{module->design->module cell type != null?}
    I -- "yes (known hierarchical cell)" --> K["return early (outputs already from FST)"]
    I -- "no (unknown cell type)" --> J
Loading

Reviews (1): Last reviewed commit: "fix: don't log error when getting non-to..." | Re-trigger Greptile

@stanminlee
Copy link
Copy Markdown

Looks good to me

@akashlevy akashlevy merged commit 242d870 into main May 12, 2026
10 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