The-OpenROAD-Project / OpenROAD

OpenROAD's unified application implementing an RTL-to-GDS Flow. Documentation at https://openroad.readthedocs.io/en/latest/
https://theopenroadproject.org/
BSD 3-Clause "New" or "Revised" License
1.62k stars 562 forks source link

Placement results in path that appears to lap macro #5971

Open jeffng-or opened 1 month ago

jeffng-or commented 1 month ago

Describe the bug

In megaboom v6, the resulting floorplan/macro/std cell placement results in paths that appear to "lap" one of the macros multiple times, which intuitively seems wrong:

CTS_Timing_Path_Match_GRT

Expected Behavior

I'm not sure where the exact issue is, so hopefully the development team can provide insight into it.

Environment

[WARNING] Your current OpenROAD version is outdated.
It is recommened to pull the latest changes.
If problem persists, file a github issue with the re-producible test case.
kernel: Linux 6.5.0-1025-gcp
os: Ubuntu 22.04.4 LTS (Jammy Jellyfish)
cmake version 3.24.2
-- The CXX compiler identification is GNU 11.4.0
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- OpenROAD version: v2.0-16316-gf9cfd9383
-- System name: Linux
-- Compiler: GNU 11.4.0
-- Build type: RELEASE
-- Install prefix: /usr/local
-- C++ Standard: 17
-- C++ Standard Required: ON
-- C++ Extensions: OFF
-- The C compiler identification is GNU 11.4.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Performing Test C_COMPILER_SUPPORTS__-Wall
-- Performing Test C_COMPILER_SUPPORTS__-Wall - Success
-- Performing Test CXX_COMPILER_SUPPORTS__-Wall
-- Performing Test CXX_COMPILER_SUPPORTS__-Wall - Success
-- Performing Test C_COMPILER_SUPPORTS__-Wno-array-bounds
-- Performing Test C_COMPILER_SUPPORTS__-Wno-array-bounds - Success
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-array-bounds
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-array-bounds - Success
-- Performing Test C_COMPILER_SUPPORTS__-Wno-nonnull
-- Performing Test C_COMPILER_SUPPORTS__-Wno-nonnull - Success
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-nonnull
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-nonnull - Success
-- Performing Test C_COMPILER_SUPPORTS__-Wno-maybe-uninitialized
-- Performing Test C_COMPILER_SUPPORTS__-Wno-maybe-uninitialized - Success
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-maybe-uninitialized
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-maybe-uninitialized - Success
-- Performing Test C_COMPILER_SUPPORTS__-Wno-format-overflow
-- Performing Test C_COMPILER_SUPPORTS__-Wno-format-overflow - Success
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-format-overflow
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-format-overflow - Success
-- Performing Test C_COMPILER_SUPPORTS__-Wno-unused-variable
-- Performing Test C_COMPILER_SUPPORTS__-Wno-unused-variable - Success
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-unused-variable
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-unused-variable - Success
-- Performing Test C_COMPILER_SUPPORTS__-Wno-unused-function
-- Performing Test C_COMPILER_SUPPORTS__-Wno-unused-function - Success
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-unused-function
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-unused-function - Success
-- Performing Test C_COMPILER_SUPPORTS__-Wno-write-strings
-- Performing Test C_COMPILER_SUPPORTS__-Wno-write-strings - Success
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-write-strings
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-write-strings - Success
-- Performing Test C_COMPILER_SUPPORTS__-Wno-sign-compare
-- Performing Test C_COMPILER_SUPPORTS__-Wno-sign-compare - Success
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-sign-compare
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-sign-compare - Success
-- Performing Test C_COMPILER_SUPPORTS__-Wno-deprecated
-- Performing Test C_COMPILER_SUPPORTS__-Wno-deprecated - Success
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-deprecated
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-deprecated - Success
-- Performing Test C_COMPILER_SUPPORTS__-Wno-c++11-narrowing
-- Performing Test C_COMPILER_SUPPORTS__-Wno-c++11-narrowing - Failed
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-c++11-narrowing
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-c++11-narrowing - Failed
-- Performing Test C_COMPILER_SUPPORTS__-Wno-register
-- Performing Test C_COMPILER_SUPPORTS__-Wno-register - Failed
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-register
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-register - Success
-- Performing Test C_COMPILER_SUPPORTS__-Wno-format
-- Performing Test C_COMPILER_SUPPORTS__-Wno-format - Success
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-format
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-format - Success
-- Performing Test C_COMPILER_SUPPORTS__-Wno-reserved-user-defined-literal
-- PerformingCMake Warning (dev) at src/sta/CMakeLists.txt:32 (option):
  Policy CMP0077 is not set: option() honors normal variables.  Run "cmake
  --help-policy CMP0077" for policy details.  Use the cmake_policy command to
  set the policy and suppress this warning.

  For compatibility with older versions of CMake, option is clearing the
  normal variable 'USE_TCL_READLINE'.
This warning is for project developers.  Use -Wno-dev to suppress it.

 Test C_COMPILER_SUPPORTS__-Wno-reserved-user-defined-literal - Failed
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-reserved-user-defined-literal
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-reserved-user-defined-literal - Failed
-- Performing Test C_COMPILER_SUPPORTS__-fpermissive
-- Performing Test C_COMPILER_SUPPORTS__-fpermissive - Failed
-- Performing Test CXX_COMPILER_SUPPORTS__-fpermissive
-- Performing Test CXX_COMPILER_SUPPORTS__-fpermissive - Success
-- Performing Test C_COMPILER_SUPPORTS__-x
-- Performing Test C_COMPILER_SUPPORTS__-x - Failed
-- Performing Test CXX_COMPILER_SUPPORTS__-x
-- Performing Test CXX_COMPILER_SUPPORTS__-x - Failed
-- Performing Test C_COMPILER_SUPPORTS__c++
-- Performing Test C_COMPILER_SUPPORTS__c++ - Failed
-- Performing Test CXX_COMPILER_SUPPORTS__c++
-- Performing Test CXX_COMPILER_SUPPORTS__c++ - Failed
-- Performing Test C_COMPILER_SUPPORTS__-std=c++17
-- Performing Test C_COMPILER_SUPPORTS__-std=c++17 - Failed
-- Performing Test CXX_COMPILER_SUPPORTS__-std=c++17
-- Performing Test CXX_COMPILER_SUPPORTS__-std=c++17 - Success
-- Performing Test C_COMPILER_SUPPORTS__-Wno-unused-but-set-variable
-- Performing Test C_COMPILER_SUPPORTS__-Wno-unused-but-set-variable - Success
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-unused-but-set-variable
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-unused-but-set-variable - Success
-- TCL library: /usr/lib/x86_64-linux-gnu/libtcl.so
-- TCL header: /usr/include/tcl/tcl.h
-- TCL readline library: /usr/lib/x86_64-linux-gnu/libtclreadline.so
-- TCL readline header: /usr/include/x86_64-linux-gnu
-- Found SWIG: /home/jeffng/dev/main/OpenROAD-flow-scripts/dependencies/bin/swig (found suitable version "4.1.0", minimum required is "4.0")  
-- Using SWIG >= 4.1.0 -flatstaticmethod flag for python
-- Found Boost: /home/jeffng/dev/main/OpenROAD-flow-scripts/dependencies/lib/cmake/Boost-1.80.0/BoostConfig.cmake (found version "1.80.0")  
-- boost: 1.80.0
-- Found GTest: /home/jeffng/dev/main/OpenROAD-flow-scripts/dependencies/lib/cmake/GTest/GTestConfig.cmake (found version "1.13.0")  
-- GTest: 1.13.0
-- Found Python3: /usr/include/python3.10 (found version "3.10.12") found components: Development Development.Module Development.Embed 
-- Found ZLIB: /usr/lib/x86_64-linux-gnu/libz.so (found version "1.2.11") 
-- Found Threads: TRUE  
-- spdlog: 1.8.1
-- Found BISON: /usr/bin/bison (found version "3.8.2") 
-- Could NOT find Doxygen (missing: DOXYGEN_EXECUTABLE) 
-- STA version: 2.6.0
-- STA git sha: b5f3a02b33b8ae1739ace8a329fde94434711dd6
-- System name: Linux
-- Compiler: GNU 11.4.0
-- Build type: RELEASE
-- Build CXX_FLAGS: -O3 -DNDEBUG
-- Install prefix: /usr/local
-- Found FLEX: /usr/bin/flex (found version "2.6.4") 
-- TCL library: /usr/lib/x86_64-linux-gnu/libtcl.so
-- TCL header: /usr/include/tcl/tcl.h
-- TCL readline library: /usr/lib/x86_64-linux-gnu/libtclreadline.so
-- TCL readline header: /usr/include/x86_64-linux-gnu/tclreadline.h
-- CUDD library: /usr/local/lib/libcudd.a
-- CUDD header: /usr/local/include/cudd.h
-- SSTA: 0
-- Found SWIG: /home/jeffng/dev/main/OpenROAD-flow-scripts/dependencies/bin/swig (found suitable version "4.1.0", minimum required is "3.0")  
-- STA executable: /home/jeffng/dev/main/OpenROAD-flow-scripts/tools/OpenROAD/src/sta/app/sta
-- Found re2: /opt/or-tools/lib/cmake/re2/re2Config.cmake (found version "11.0.0") 
-- Found Clp: /opt/or-tools/lib/cmake/Clp/ClpConfig.cmake (found version "1.17.7") 
-- Found Cbc: /opt/or-tools/lib/cmake/Cbc/CbcConfig.cmake (found version "2.10.7") 
-- Found SCIP: /opt/or-tools/lib/cmake/scip/scip-config.cmake (found version "9.0.0") 
-- Found OpenMP_CXX: -fopenmp (found version "4.5") 
-- Found OpenMP: TRUE (found version "4.5")  
-- Found OR-Tools: /opt/or-tools/lib/cmake/ortools (version: 9.10.4067)
-- TCL library: /usr/lib/x86_64-linux-gnu/libtcl.so
-- TCL header: /usr/include/tcl/tcl.h
-- Found OpenMP_C: -fopenmp (found version "4.5") 
-- Found OpenMP: TRUE (found version "4.5")  
-- Found OpenMP: TRUE (found version "4.5")  
-- GUI is enabled
-- Charts widget is enabled
-- FounNumber of processor cores: 32
d Boost: /home/jeffng/dev/main/OpenROAD-flow-scripts/dependencies/lib/cmake/Boost-1.80.0/BoostConfig.cmake (found version "1.80.0") found components: serialization 
-- Could NOT find VTune (missing: VTune_LIBRARIES VTune_INCLUDE_DIRS) 
-- Found Boost: /home/jeffng/dev/main/OpenROAD-flow-scripts/dependencies/lib/cmake/Boost-1.80.0/BoostConfig.cmake (found suitable version "1.80.0", minimum required is "1.78")  
-- TCL library: /usr/lib/x86_64-linux-gnu/libtcl.so
-- TCL header: /usr/include/tcl/tcl.h
-- Found Boost: /home/jeffng/dev/main/OpenROAD-flow-scripts/dependencies/lib/cmake/Boost-1.80.0/BoostConfig.cmake (found version "1.80.0") found components: serialization system thread 
-- Found Boost: /home/jeffng/dev/main/OpenROAD-flow-scripts/dependencies/lib/cmake/Boost-1.80.0/BoostConfig.cmake (found version "1.80.0")  
-- Found Eigen3: /home/jeffng/dev/main/OpenROAD-flow-scripts/dependencies/share/eigen3/cmake/Eigen3Config.cmake (found version "3.4.0") 
-- TCL readline enabled
-- Tcl Extended disabled
-- Python3 enabled
-- Configuring done
-- Generating done
-- Build files have been written to: /tmp/tmp.Bt0W42tFUE

To Reproduce

  1. unpack the tarball https://drive.google.com/file/d/1eVCheqXsRstEgBQLqIi-H5i4qwO9Z6Dh/view?usp=sharing
  2. cd cts_BoomTile_asap7_base_2024-10-16_10-40
  3. ./run-me-BoomTile-asap7-base.sh gui <- I added the second argument to just pull up the GUI instead of running CTS
  4. type gui::show if the GUI doesn't come up (might be fixed)
  5. click Update on the Timing Report
  6. Pick the first path (although any of the ones with -6k slack would suffice

Relevant log output

No response

Screenshots

No response

Additional Context

No response

maliberty commented 1 month ago

At what point in the flow is your image from? Trying to narrow down where in the flow the problem starts is helpful.

stefanottili commented 1 month ago

Give the “visualize the whole cone” feature a try. I found it very useful to explain why placers put cells in unintuitive locations. There are usually more things pulling at cells than you see by just looking at the critical path.

jeffng-or commented 1 month ago

At what point in the flow is your image from? Trying to narrow down where in the flow the problem starts is helpful.

This is after the CTS step, but the path after GRT is effectively the same

maliberty commented 1 month ago

Are you using global placement with -timing_driven? If not you should turn it on.

jeffng-or commented 1 month ago

Are you using global placement with -timing_driven? If not you should turn it on.

No, the bazel var GPL_TIMING_DRIVEN is set to 0. I can run it with the option to see what changes.

jeffng-or commented 1 month ago

With GPL_TIMING_DRIVEN enabled, the previous WC path improved -1673 slack vs -6393. Here's what the path looks like now:

GRT_Timing_Path_With_TD_prev_WC_path

So, it's still looking like it's taking a lap around the macro.

The fanout cone visualization looks like:

image

FWIW, the new WC path is -1930 slack.

stefanottili commented 1 month ago

The fan out cone needs some more refinement to be really useful: 1) draw the flight lines bottom up, from least to most critical 2) get an adjustable filter to get rid of the not so critical ones ..

But it’s the only way that I know to visualize both connectivity and timing criticality.

maliberty commented 1 month ago

The data is organized by level rather than by criticality currently. We use the top 1000 paths to the pin of interest. Do you have a suggestion for a good default filter until it can be made user selectable?

stefanottili commented 1 month ago

Anything that reduces the amount of data to a point that it's "fast to draw" and "useful" would help, so "user selectable level" would be nice, but criticality is obviously better.

Even when there was a tight collaboration between RTL writer and P&R tool jockey, I was always surprised by the lack of knowledge about the "size of the cones", both # and location of starting/ending ff's of a cone and the routing that goes with it. There are only few RTL guy's (architects) that take the time to "to have a look" at how their netlist is placed.

So anything that gives insight into why things end up where they are and "what else is critical" in the vicinity is helpful.

No P&R tool that I'm aware of has "a nice way" to visualize "input/output cones color coded by criticality", so OR's cone viewing is a great first step, it just needs widespread usage and user feedback to get the proper refinement. Useful visualization is more than "just" showing the one critical path and the clk skew at its input and output ff.

jeffng-or commented 3 weeks ago

@precisionmoon's input:

All the cells in report_checks are std cells. There are buffers added by repair_design but their placements are already impacted by pre-placed non-buffer cells. Timing-driven option seems to be the way to go. -timing_driven_net_reweight_overflow can be changed to decrease runtime.

oharboe commented 5 days ago

More examples https://github.com/The-OpenROAD-Project/megaboom/pull/208

image

maliberty commented 5 days ago

Its more helpful to show the macros than the placement blockages