Update OpenSTA 5/14 code comes with report path field generalization, needs OpenROAD updation for mbff orig_name by dsengupta0628 · Pull Request #10437 · The-OpenROAD-Project/OpenROAD

dsengupta0628 · 2026-05-15T15:51:55Z

Summary

Migrate mbff orig_name reporting to use OpenSTA's generic makeReportPathField API instead of the hardcoded field_orig_name_ in OpenSTA.

Attribute orig_name is now stored on the tray pin instead of the tray instance

Changes

mbff.cpp: store orig_name property on the tray ITerm (key orig_name) instead of the tray instance (key
orig_name_<port>). One property per pin, no port suffix.
mbff.cpp: in MBFF ctor, register a custom orig_name field via sta_->makeReportPathField(...) with a getter
that resolves Path → Vertex → Pin → dbITerm → "orig_name" property. Idempotent — guarded by findReportPathField.

Why

OpenSTA dropped its hardcoded field_orig_name_ in favor of a generic field-registration API. mbff now owns the
field, the attribute storage, and the getter.
Pin-level (iterm) attribute is the natural granularity — one mapping per tray pin instead of N suffixed keys on the
parent instance.

Dependency

Blocked on upstream OpenSTA PR The-OpenROAD-Project/OpenSTA#363 that removes field_orig_name_ and exposes Sta::makeReportPathField / Sta::findReportPathField. OpenROAD submodule bump required.

Type of Change

Refactoring

Impact

None

Verification

I have verified that the local build succeeds (./etc/Build.sh).
I have run the relevant tests and they pass.
My code follows the repository's formatting guidelines.
I have included tests to prevent regressions.
I have signed my commits (DCO).

Related Issues

[Link issues here]

Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

gemini-code-assist

Code Review

This pull request updates the MBFF clustering logic to store original pin names as properties on dbITerm objects rather than instances, which improves tracking in hierarchical designs. It also registers a custom report field in STA to display these original names. The review feedback correctly identifies several compilation errors in src/gpl/src/mbff.cpp where .c_str() is incorrectly called on const char* pointers returned by the name() method.

gemini-code-assist · 2026-05-15T15:54:19Z

+      dbITerm* tray_iterm = nullptr;
      if (is_d && d_pin) {
-        tray_port = d_pin->name();
+        tray_iterm = tray_inst[tray_idx]->findITerm(d_pin->name().c_str());


The sta::LibertyPort::name() method returns a const char*. Calling .c_str() on a raw pointer is a compilation error. You should pass the result of name() directly to findITerm.

tray_iterm = tray_inst[tray_idx]->findITerm(d_pin->name());

Gemini you are wrong. LibertyPort inherits ConcretePort::name() which returns const std::string & . So c_str() on it is valid and necessary because dbInst::findITerm takes const char*

Current code:
tray_iterm = tray_inst[tray_idx]->findITerm(d_pin->name().c_str());

is correct,
Without .c_str() compile would fail (can't convert const std::string& to const char* implicitly).

gemini-code-assist · 2026-05-15T15:54:19Z

      } else if (is_q) {
        if (is_qn_inv && qn_pin) {
-          tray_port = qn_pin->name();
+          tray_iterm = tray_inst[tray_idx]->findITerm(qn_pin->name().c_str());


The sta::LibertyPort::name() method returns a const char*. Calling .c_str() on a raw pointer is a compilation error. You should pass the result of name() directly to findITerm.

tray_iterm = tray_inst[tray_idx]->findITerm(qn_pin->name());

same comment as above

gemini-code-assist · 2026-05-15T15:54:19Z

+          tray_iterm = tray_inst[tray_idx]->findITerm(qn_pin->name().c_str());
        } else if (q_pin) {
-          tray_port = q_pin->name();
+          tray_iterm = tray_inst[tray_idx]->findITerm(q_pin->name().c_str());


The sta::LibertyPort::name() method returns a const char*. Calling .c_str() on a raw pointer is a compilation error. You should pass the result of name() directly to findITerm.

tray_iterm = tray_inst[tray_idx]->findITerm(q_pin->name());

same comment as above

github-actions · 2026-05-15T15:55:51Z

clang-tidy review says "All clean, LGTM! 👍"

gemini-code-assist

Code Review

This pull request modifies the MBFF clustering logic to store original pin names as properties directly on tray pins (dbITerm) rather than on the tray instance. It also registers a new 'Orig Name' field in the STA report path to display these original names in timing reports. Review feedback suggests simplifying the pin-to-iterm mapping logic to improve robustness and performance, as well as adding a descriptive help string for the new report path field.

gemini-code-assist · 2026-05-15T15:59:49Z

+      dbITerm* tray_iterm = nullptr;
      if (is_d && d_pin) {
-        tray_port = d_pin->name();
+        tray_iterm = tray_inst[tray_idx]->findITerm(d_pin->name().c_str());
      } else if (is_q) {
        if (is_qn_inv && qn_pin) {
-          tray_port = qn_pin->name();
+          tray_iterm = tray_inst[tray_idx]->findITerm(qn_pin->name().c_str());
        } else if (q_pin) {
-          tray_port = q_pin->name();
+          tray_iterm = tray_inst[tray_idx]->findITerm(q_pin->name().c_str());
        }
      }


The logic for selecting tray_iterm for Q/QN pins can be simplified and made more robust. The current implementation might incorrectly fall back to a non-inverting pin (q_pin) if an inverting pin (qn_pin) is expected but missing in the tray, which would lead to misleading "Orig Name" reports. Additionally, consider pre-fetching the tray iterms for the current slot outside the iterm loop to avoid redundant findITerm calls (string lookups) for each pin of the original flop.

dbITerm* tray_iterm = nullptr; if (is_d && d_pin) { tray_iterm = tray_inst[tray_idx]->findITerm(d_pin->name().c_str()); } else if (is_q) { const sta::LibertyPort* tray_port = is_qn_inv ? qn_pin : q_pin; if (tray_port) { tray_iterm = tray_inst[tray_idx]->findITerm(tray_port->name().c_str()); } }

References

When analyzing code within a loop, consider the entire loop structure to identify and remove redundant operations inside the loop body.

I addressed this

gemini-code-assist · 2026-05-15T15:59:49Z

+  if (sta_->findReportPathField(kOrigNameProp) == nullptr) {
+    sta::dbNetwork* network = network_;
+    sta_->makeReportPathField(
+        kOrigNameProp,


It is recommended to provide a descriptive help string for the newly registered report path field to assist users in understanding its purpose when using the report_checks command.

kOrigNameProp, kOrigNameProp, "Orig Name", 36, true, "Original name of the pin before multi-bit flip-flop clustering.",

Sta::makeReportPathField has no help-string parameter (Sta.hh:991-997). Signature is (name, name_abrev, title, width, left_justify, Unit*, get_value). Hallucination

I added a comment

github-actions · 2026-05-15T16:01:43Z

clang-tidy review says "All clean, LGTM! 👍"

Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

github-actions · 2026-05-15T16:22:22Z

clang-tidy review says "All clean, LGTM! 👍"

dsengupta0628 · 2026-05-15T16:35:35Z

secure CI: http://secure-ci:8080/view/My/job/SB/job/secure-or_update_sta_0514/

dsengupta0628 · 2026-05-15T16:35:51Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cc1e711b46

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-05-15T16:41:22Z

+  if (sta_->findReportPathField(kOrigNameProp) == nullptr) {
+    sta::dbNetwork* network = network_;
+    sta_->makeReportPathField(


Register orig_name outside cluster_flops

Because this field is registered only from MBFF::MBFF, and Replace::runMBFF constructs MBFF only while running cluster_flops, a fresh OpenROAD session that reads an already-clustered database with existing orig_name properties and then runs report_checks -fields {orig_name} never creates this field and will warn that it is unknown instead of reporting the saved mappings. The old OpenSTA field was always available, so please register the report field during tool/STA initialization rather than behind a clustering run.

Useful? React with 👍 / 👎.

Meaningful review: "orig_name" property persists in .odb files, but field registration is in-memory and only triggered by MBFF ctor (which Replace::runMBFF builds per-cluster_flops invocation). Workflow:

Session A: read design → cluster_flops → write .odb (properties on iterms saved).

Session B (fresh): read same .odb → properties exist on iterms → report_checks -fields {orig_name} → STA findField("orig_name") returns null → sta_warn 168 "unknown field orig_name".

Data present, but field missing. Bug.

Needs fixing

Code before OpenSTA removed orig_name, had field_orig_name_ hardcoded as a built-in field:

src/sta/search/ReportPath.cc: field_orig_name_ = makeField("orig_name", "Orig Name", 36, ...);

It had ReportPath::makeFields() - invoked when STA constructs ReportPath at OpenROAD startup. Built-in, always present, no mbff/cluster_flops involvement.

That's why fresh-session reload (with read_db and no cluster_flop call) works there - the field was never tied to mbff. mbff only had to write data.

In our refactored setup, we removed that built-in from OpenSTA (from upstream).
After that removal, nothing in STA registers orig_name.

If the registration only ran in MBFF::MBFF, then a session that never instantiates MBFF (i.e., loads a pre-clustered .odb and reports) has no field "orig_name".

The Replace constructoir fix restores the always-available behavior at session init, matching what the old built-in did, but keeping STA generic (any tool can register its own custom field the same way).

… is available before any cluster_flops invocation Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

github-actions

clang-tidy made some suggestions

github-actions · 2026-05-15T18:55:59Z

+        true,
+        nullptr,
+        [network](const sta::Path* path,
+                  const sta::StaState* sta) -> std::string {


warning: no header providing "sta::StaState" is directly included [misc-include-cleaner]

src/gpl/src/replace.cpp:27:

- #include "timingBase.h" + #include "sta/StaState.hh" + #include "timingBase.h"

Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

dsengupta0628 · 2026-05-15T20:15:46Z

@codex review

github-actions · 2026-05-15T20:16:11Z

clang-tidy review says "All clean, LGTM! 👍"

dsengupta0628 · 2026-05-15T20:20:11Z

latest secure ci: http://secure-ci:8080/view/My/job/SB/job/secure-or_update_sta_0514/

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 437521a92a

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-05-15T20:20:15Z

+          odb::dbStringProperty* prop
+              = odb::dbStringProperty::find(iterm, kOrigNameProp);
+          return prop ? prop->getValue() : std::string{};


Fall back to old instance-level orig_name properties

When users load an ODB written by the previous implementation after cluster_flops, the saved mappings are still on the tray dbInst under orig_name_<port>, but this new getter only looks for orig_name on the dbITerm. That makes report_checks -fields {orig_name} silently blank out existing clustered databases even though the old data is present; please preserve compatibility by falling back to the instance property using the current iterm's port name when the pin-level property is missing.

Useful? React with 👍 / 👎.

This is factually correct about behavior: old .odb files written before this PR have orig_name_ on the tray dbInst (commit 649366b). New getter reads orig_name on dbITerm --> silent blank for those legacy .odb files. But the orig name storage was a recent April 2026 commit, not a long-standing release. It will be better to rerun cluster_flops to regenerate new-format properties automatically.

dsengupta0628 added 3 commits May 15, 2026 15:17

update latest STA module and use make report fields

ff4fde2

Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

enhance testcase to use hier design

32fc69e

Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

clang formatter

0912d5f

Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

dsengupta0628 requested review from a team as code owners May 15, 2026 15:51

dsengupta0628 self-assigned this May 15, 2026

dsengupta0628 requested review from LucasYuki and maliberty May 15, 2026 15:51

github-actions Bot added the size/M label May 15, 2026

tclfmt fix

3452e8e

Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

gemini-code-assist Bot reviewed May 15, 2026

View reviewed changes

dsengupta0628 added 2 commits May 15, 2026 16:11

address one legit gemini feedback

f97d94e

Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

add comment to accompany call to sta_->makeReportPathField

cc1e711

Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

chatgpt-codex-connector Bot reviewed May 15, 2026

View reviewed changes

dsengupta0628 mentioned this pull request May 15, 2026

Sta update latest 0514 The-OpenROAD-Project/OpenSTA#363

Open

dsengupta0628 added 2 commits May 15, 2026 18:43

moved makeReportPathField from MBFF ctor to Replace ctor so the field…

17c48eb

… is available before any cluster_flops invocation Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

tclfmt

2f9fdb6

Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

github-actions Bot reviewed May 15, 2026

View reviewed changes

dsengupta0628 added 3 commits May 15, 2026 19:10

regression

43286b9

Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

regression tclfmt

6b36f7c

Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

clang tidy comment

437521a

Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

chatgpt-codex-connector Bot reviewed May 15, 2026

View reviewed changes

Conversation

dsengupta0628 commented May 15, 2026

Summary

Changes

Why

Dependency

Type of Change

Impact

Verification

Related Issues

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot May 15, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 15, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 15, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented May 15, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot May 15, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 15, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented May 15, 2026

Uh oh!

github-actions Bot commented May 15, 2026

Uh oh!

dsengupta0628 commented May 15, 2026

Uh oh!

dsengupta0628 commented May 15, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot May 15, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot May 15, 2026

Choose a reason for hiding this comment

Uh oh!

dsengupta0628 commented May 15, 2026

Uh oh!

github-actions Bot commented May 15, 2026

Uh oh!

dsengupta0628 commented May 15, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment