The-OpenROAD-Project / OpenLane

OpenLane is an automated RTL to GDSII flow based on several components including OpenROAD, Yosys, Magic, Netgen and custom methodology scripts for design exploration and optimization.
https://openlane.readthedocs.io/
Apache License 2.0
1.3k stars 370 forks source link

[resizer] - Openlane Resizer Fails due to set_dont_touch { <Instance> } command in .SDC #1572

Closed dineshannayya closed 1 year ago

dineshannayya commented 1 year ago

Description

Typical SDC scripts uses option [set_dont_touch] to avoid the manually inserted cells get modified through flow.

When i try the command in SDC the Openlane resizer stage fails with below message

[ERROR]: during executing openroad script /openlane/scripts/openroad/resizer.tcl
[ERROR]: Log: peri_top/runs/peri_top/logs/placement/8-resizer.log
[ERROR]: Last 10 lines:
OpenROAD 7c85c140308f01b73f57ea1117f3e43f39abd437 
This program is licensed under the BSD-3 license. See the LICENSE file for details.
Components of this program may be licensed under more restrictive licenses which must be honored.
[INFO]: Setting RC values...
[ERROR ODB-0370] Attempt to disconnect iterm of dont_touch instance u_skew_peri.u_mux_level_30.genblk1.u_mux
Error: resizer.tcl, 40 ODB-0370
child process exited abnormally

Expected Behavior

Flow should honor the set_dont_touch constraint and should not modify those instance defined by the user.

The OpenRoad Read me also say's it support this feature.

Openroad Read.me: src/rsz/README.md

Set Don't Touch```

set_dont_touch instances_nets unset_dont_touch instances_nets ``Theset_dont_touch` command prevents the resizer commands from modifying instances or nets.

Environment report

<pre><font color="#4E9A06"><b>dinesha@lenovo-i3-10100-07IMB05</b></font>:<font color="#3465A4"><b>~/workarea/efabless/MPW-7/OpenLane</b></font>$ python3 ./env.py issue-survey
Kernel: Linux v5.15.0-56-generic
Distribution: ubuntu 20.04
Python: v3.8.10 (OK)
Container Engine: docker v20.10.21 (OK)
OpenLane Git Version: 73bc7f8a736d6f2a2d68168daea3e7718d4b6208
pip: INSTALLED
python-venv: INSTALLED
---
PDK Version Verification Status: OK
---
Git Log (Last 3 Commits)

73bc7f8 2022-12-09T13:20:53+02:00 Add new community guide for using RgGen (#1561) - Cra2yPierr0t -  (HEAD -&gt; master)
772a224 2022-12-05T15:31:29+02:00 [BOT] Update PDK (#1551) - Openlane Bot -  ()
010f5ad 2022-12-04T12:20:40+02:00 Add PDK_FAMILY for `volare enable` (#1545) - Kareem Farid -  ()
---
Git Remotes

origin  https://github.com/efabless/OpenLane.git (fetch)
origin  https://github.com/efabless/OpenLane.git (push)
</pre>

Reproduction material

issue_reproducible.tar.gz

Relevant log output

<pre>[INFO]: Running Global Placement (log: peri_top/runs/peri_top/logs/placement/7-global.log)...
[STEP 8]
[INFO]: Running Placement Resizer Design Optimizations (log: peri_top/runs/peri_top/logs/placement/8-resizer.log)...
[ERROR]: during executing openroad script /openlane/scripts/openroad/resizer.tcl
[ERROR]: Log: peri_top/runs/peri_top/logs/placement/8-resizer.log
[ERROR]: Last 10 lines:
OpenROAD 7c85c140308f01b73f57ea1117f3e43f39abd437 
This program is licensed under the BSD-3 license. See the LICENSE file for details.
Components of this program may be licensed under more restrictive licenses which must be honored.
[INFO]: Setting RC values...
[ERROR ODB-0370] Attempt to disconnect iterm of dont_touch instance u_skew_peri.u_mux_level_30.genblk1.u_mux
Error: resizer.tcl, 40 ODB-0370
child process exited abnormally
</pre>
vijayank88 commented 1 year ago

@maliberty fyi

maliberty commented 1 year ago

u_skew_peri.u_mux_level_30.genblk1.u_mux is dont_touch and its output is wbd_clk_peri. When you call buffer_outputs it can't be disconnected from wbd_clk_peri (due to dont_touch) so there is no way to buffer that output. Is it acceptable to skip output buffering in this case?

dineshannayya commented 1 year ago

When we define don't touch instance mean, the corresponding instance cell type and drive strength should not be modified by the tool. But still tool can add/delete additional buffer based on transition fix; which may need net changes. Which should be fine. For my case, if you can't add additional buffer to this port is fine.

maliberty commented 1 year ago

The current definition for dont-touch on a dbInst:

  ///
  /// Set/Reset the don't-touch flag
  ///
  /// Setting this implies:
  /// - The instance can't be destroyed
  /// - The instance can't be resized (ie swapMaster)
  /// - The associated iterms can't be connected or disconnected
  /// - The parent module can't be changed
  /// - The instance CAN be moved, have its orientation changed, or be
  ///   placed or unplaced

It sounds like you think connect/disconnect should be allowed on a dont_touch instance. I'm willing to change the definition if there is consensus - @gadfort @rovinski @louiic @msaligane

(it was hard to find any well documented semantics when I implemented this)

dineshannayya commented 1 year ago

As per my understanding set_dont_touch on instance and nets are two different functions.

Standard Definition I see as set_dont_touch: Sets the dont_touch attribute on cells, nets, designs, and library cells to prevent synthesis from replacing or modifying them during optimization. Reference: https://ipmart.micro-ip.com/Synopsys(PT)/dictionary_512_17/set_dont_touch.html

maliberty commented 1 year ago

But it doesn't define what constitutes a modification...

rovinski commented 1 year ago

I think it's really hard to find consistent behavior across tools with respect to set_dont_touch because it's not a standardized command in any way. I did find this one example:

The following commands prevent modification or removal of the block1 and
analog1 cells during optimization but allow the N1 net to be modified.

  > set_dont_touch [get_cells {block1 analog1}]
  1
  > set_dont_touch  [get_nets N1] false
  1

I see this as two competing semantics:

  1. dont_touch should ensure that the instance cannot be resized, optimized out, or disconnected from its net.
  2. dont_touch shouldn't prevent optimizations around the instance.

The only reason it seems these two are in contention here is because the dont_touch instance is connected to a port and therefore can't be disconnected from the port net, otherwise it'd be fine.

If we wanted to go for consistency with other tools, I think honestly it would require some experimentation to see how it's handled. If we wanted to go with my gut feeling as a designer, I would say the current interpretation of not allowing the iterm disconnection makes the most sense, maybe along with raising a warning that the port cannot be buffered for this reason.

I am definitely curious about how the example I found is supposed to work. Is the disabling of dont_touch supposed to then allow modification of the iterms by overriding the dont_touch on the cell?

Another option to make this behavior explicit may be to add a flag to set_dont_touch e.g. -allow_optimization which removes the restriction on not modifying the iterm.

rovinski commented 1 year ago

A workaround to this issue might simply be to do buffer_outputs before set_dont_touch, but I think this is annoying because this would not allow simply setting all of the dont_touches in the SDC file.

gadfort commented 1 year ago

I think the current definition makes sense to me. However, buffer ports should not attempt to make that change. Another option would be to indicate that buffer ports is allowed to temporarily change the dont_touch setting buffer_ports -ignore_dont_touch.

maliberty commented 1 year ago

Given the discussion and "For my case, if you can't add additional buffer to this port is fine." I think I will leave the semantics alone and not buffer this port in rsz.

rovinski commented 1 year ago

Adding a warning is probably sufficient for now. We may want to revisit this to allow easier user control.

Although I like @gadfort's idea of buffer_ports -ignore_dont_touch, I think this same circumstance can happen between two dont_touch instances. A new net couldn't be inserted because neither instance could be disconnected.

dineshannayya commented 1 year ago

@maliberty Is this issue fixed in latest openlane setup ? I am using latest openlane and still facing same issue.

dinesha@lenovo-i3-10100-07IMB05:~/workarea/efabless/MPW-9/OpenLane$ python3 ./env.py issue-survey
Kernel: Linux v5.15.0-56-generic
Distribution: ubuntu 20.04
Python: v3.8.10 (OK)
Container Engine: docker v20.10.22 (OK)
OpenLane Git Version: e570a6a5b428eddb19f8efe4f9d18d79f1050940
pip: INSTALLED
python-venv: INSTALLED
---
PDK Version Verification Status: OK
---
Git Log (Last 3 Commits)

e570a6a 2023-01-02T15:10:43+02:00 `run_designs` fixes (#1597) - Mohamed Gaber -  (HEAD -> master, tag: 2023.01.03, origin/master, origin/HEAD)
f507637 2022-12-28T16:44:56+02:00 Fix LEC (#1511) - Anton Blanchard -  (tag: 2022.12.29)
4e91061 2022-12-28T14:38:15+02:00 Makefile: disable seccomp to enable openroad ui (#1528) - Johan Euphrosine -  ()
---
Git Remotes

origin  https://github.com/The-OpenROAD-Project/OpenLane (fetch)
origin  https://github.com/The-OpenROAD-Project/OpenLane (push)

Error Message

[ERROR]: during executing openroad script /openlane/scripts/openroad/resizer.tcl
[ERROR]: Log: ycr_core_top/runs/ycr_core_top/logs/placement/8-resizer.log
[ERROR]: Last 10 lines:
OpenROAD 4f1108b6f558718ed142cbb6c1f5ba20958195ca 
This program is licensed under the BSD-3 license. See the LICENSE file for details.
Components of this program may be licensed under more restrictive licenses which must be honored.
[INFO]: Setting RC values...
[ERROR ODB-0370] Attempt to disconnect iterm of dont_touch instance u_skew_core_clk.u_mux_level_00.genblk1.u_mux
Error: resizer.tcl, 36 ODB-0370
child process exited abnormally

[ERROR]: Creating issue reproducible...
[INFO]: Saving runtime environment...
OpenLane TCL Issue Packager

Issue Database: issue_reproducible.tar.gz

dineshannayya commented 1 year ago

@maliberty Any input? .. Do i need to open separate issue here ?

maliberty commented 1 year ago

It is fixed in OR so it is just a matter of OL updating. @donn can comment on the schedule to do so as I don't control it. You can always locally switch to a later OR commit (see https://github.com/The-OpenROAD-Project/OpenLane/blob/master/docker/README.md)

vijayank88 commented 1 year ago

@maliberty I'm on latest OR. Try to run user test case and still its failing. Any configuration should be updated in the test case to complete the flow?

OpenROAD v2.0-6364-g6544dddc5 
This program is licensed under the BSD-3 license. See the LICENSE file for details.
Components of this program may be licensed under more restrictive licenses which must be honored.
openroad> source run.tcl
[INFO]: Setting RC values...
[ERROR ODB-0370] Attempt to disconnect iterm of dont_touch instance u_skew_core_clk.u_mux_level_00.genblk1.u_mux
Error: resizer.tcl, 36 ODB-0370
while evaluating {source run.tcl}
maliberty commented 1 year ago

@vijayank88 I just tried with that commit id and I get no error.

maliberty commented 1 year ago

I see many of

[WARNING ODB-0383] u_skew_peri.clkbuf_1.u_dly0 is marked do not touch and will 
be skipped in global connections.

do you get these?

vijayank88 commented 1 year ago

No. stopped with [ERROR ODB-0370]

maliberty commented 1 year ago

I ran the original reproducer which passes. I see there is a 2nd one that fails. I'll look at that.

maliberty commented 1 year ago

There was a similar issue with dont_touch and input buffering which is now fixed.