Skip to content

TR conn fix#759

Merged
maliberty merged 5 commits into
The-OpenROAD-Project:masterfrom
The-OpenROAD-Project-staging:TR_conn_fix
Jun 19, 2021
Merged

TR conn fix#759
maliberty merged 5 commits into
The-OpenROAD-Project:masterfrom
The-OpenROAD-Project-staging:TR_conn_fix

Conversation

@openroad-robot

Copy link
Copy Markdown
Contributor

No description provided.

stephano added 3 commits June 17, 2021 11:36
@Stephanommg Stephanommg requested a review from maliberty June 18, 2021 00:45
@maliberty maliberty marked this pull request as ready for review June 18, 2021 03:34

@maliberty maliberty left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

How have you tested this change? Is there any runtime impact?

Comment on lines +111 to +115
// if (pinFigIn.getPin()) {
// os << "PINFIG (PINNAME/LAYER) " <<
// pinFigIn.getPin()->getTerm()->getName()
// << " " << pinFigIn.getLayerNum() << endl;
// }

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why remove this?

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.

It does not make sense to me that the name "PINFIG" appears before the name of the obj I want to print. This was very misleading to me. I think that detailed info of the sub-objects of the obj being print should be omitted, or at least being printed after it.

Comment thread src/TritonRoute/src/global.cpp
Comment thread src/TritonRoute/src/global.h Outdated
@Stephanommg Stephanommg requested a review from maliberty June 18, 2021 15:56
@Stephanommg

Copy link
Copy Markdown
Contributor

How have you tested this change? Is there any runtime impact?

I tested in the QOR site. 0 drvs.
Comparing against the via-in-pin fix branch, there was an average runtime increase of 9%. However, this comparison considers only 1 run of each branch, and also the runtimes are low in general. In the design with the higher runtime (8 min, sky130hd/ibex), there was a decrease of 19%.

If there was really a significant increase, it is due to the removal of truncated style checks. The alternative solution is to guarantee that truncated style means it is physically connected to a pin. One of the conn check problems was that there was a wire connecting to a boundary pin (an thus not truncated) but also above a M2 pin.

@Stephanommg

Copy link
Copy Markdown
Contributor

How have you tested this change? Is there any runtime impact?

The comparison I did before was flawed (it was using 2 different builds). I reran the experiments with branch master and this branch merged to master, 3x for each branch, and there is no significant runtime decrease/increase (there was an average decrease of 2%).

@maliberty maliberty merged commit f4bd032 into The-OpenROAD-Project:master Jun 19, 2021
@maliberty maliberty deleted the TR_conn_fix branch June 19, 2021 05:41
@maliberty maliberty mentioned this pull request Jul 9, 2026
openroad-ci pushed a commit to The-OpenROAD-Project-staging/OpenROAD that referenced this pull request Jul 9, 2026
Bazel 9 removes several long-deprecated globals and native rules
(CcInfo, cc_test, cc_library/cc_binary, sh_binary). Update or pin the
dependencies whose .bzl/BUILD files still used the removed forms:

- rules_go: force 0.61.1. The MVS-selected 0.53.0 (transitive via
  or-tools, gazelle, grpc-java, aspect_rules_lint -> rules_buf)
  referenced the removed CcInfo global in cgo.bzl; 0.59.0+ loads it
  from @rules_cc.
- spdlog: 1.15.1 -> 1.17.0. Its BUILD.bazel used the native cc_test.
- scip: 9.2.3 -> 9.2.3.bcr.1. The overlay used native cc_library/
  cc_binary; .bcr.1 loads them from @rules_cc (same source, so the
  tinycthread patch still applies).
- freetype: 2.13.3 -> 2.13.3.bcr.2 (transitive via qt-bazel ->
  harfbuzz). The overlay used native cc_library; .bcr.2 loads it from
  @rules_cc (same 2.13.3 source).
- bazel-orfs: bump to 551d4c7. Its generated mock_klayout BUILD loads
  sh_binary from @rules_shell, and its generated gnumake BUILD overlay
  loads cc_binary from @rules_cc -- both native rules Bazel 9 removed
  (upstream PRs The-OpenROAD-Project#759 and The-OpenROAD-Project#761).

Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
openroad-ci pushed a commit to The-OpenROAD-Project-staging/OpenROAD that referenced this pull request Jul 9, 2026
Bazel 9 removes several long-deprecated globals and native rules
(CcInfo, cc_test, cc_library/cc_binary, sh_binary). Update or pin the
dependencies whose .bzl/BUILD files still used the removed forms:

- rules_go: force 0.61.1. The MVS-selected 0.53.0 (transitive via
  or-tools, gazelle, grpc-java, aspect_rules_lint -> rules_buf)
  referenced the removed CcInfo global in cgo.bzl; 0.59.0+ loads it
  from @rules_cc.
- spdlog: 1.15.1 -> 1.17.0. Its BUILD.bazel used the native cc_test.
- scip: 9.2.3 -> 9.2.3.bcr.1. The overlay used native cc_library/
  cc_binary; .bcr.1 loads them from @rules_cc (same source, so the
  tinycthread patch still applies).
- freetype: 2.13.3 -> 2.13.3.bcr.2 (transitive via qt-bazel ->
  harfbuzz). The overlay used native cc_library; .bcr.2 loads it from
  @rules_cc (same 2.13.3 source).
- bazel-orfs: bump to 551d4c7. Its generated mock_klayout BUILD loads
  sh_binary from @rules_shell, and its generated gnumake BUILD overlay
  loads cc_binary from @rules_cc -- both native rules Bazel 9 removed
  (upstream PRs The-OpenROAD-Project#759 and The-OpenROAD-Project#761).

Also suppress -Wc++11-narrowing in third-party/mockturtle's emap test:
its vendored headers narrow ints in braced initializer lists, which
clang treats as an error by default.

Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
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