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.59k stars 552 forks source link

Detailled Router complains about "Non orthogonal Routes" even though there aren't any #3385

Closed smunaut closed 1 year ago

smunaut commented 1 year ago

Describe the bug

When trying to use OpenLane2 with a custom routing script that manually routes some signal before the Global+Detailled router passes, it fails with tons of

[ERROR DRT-1010] Unsupported non-orthogonal wire begin=(1363820, 235100) end=(1513820, 1759000)
[ERROR DRT-1010] Unsupported non-orthogonal wire begin=(1363820, 235100) end=(1513820, 1759000)
[ERROR DRT-1010] Unsupported non-orthogonal wire begin=(1363820, 235100) end=(1513820, 1759000)
[ERROR DRT-1010] Unsupported non-orthogonal wire begin=(1363820, 235100) end=(1513820, 1759000)
[ERROR DRT-1010] Unsupported non-orthogonal wire begin=(1363820, 235100) end=(1513820, 1759000)

If in my custom routing script I leave the wires but remove the vias, then it works.

Test case data can be found on : https://people.osmocom.org/tnt/stuff/tt/bug_data.zip

It contains the reproductable test case from OpenLane2 directly and the DEF output. ( Athough apparently it uses the ODB data directly rather than going through DEF between the different steps and the DEF is just "informational" ).

Expected Behavior

Routing complete

Environment

Git commit: 675438bce1190bd3ade9d24d674a161b175455ac
kernel: Linux 5.10.0-19-cloud-amd64
os: Debian GNU/Linux 11 (bullseye)
cmake version 3.18.4
-- The CXX compiler identification is GNU 10.2.1
-- 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-8218-g675438bce
-- System name: Linux
-- Compiler: GNU 10.2.1
-- Build type: RELEASE
-- Install prefix: /usr/local
-- C++ Standard: 17
-- C++ Standard Required: ON
-- C++ Extensions: OFF
-- Found GTest: /usr/lib/x86_64-linux-gnu/libgtest.a  
-- The C compiler identification is GNU 10.2.1
-- 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 - Success
-- 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_SUPCMake Warning at src/CMakeLists.txt:226 (message):
  spdlog: SPDLOG_FMT_EXTERNAL=ON

CMake Error at src/mpl2/CMakeLists.txt:42 (find_package):
  By not providing "Findortools.cmake" in CMAKE_MODULE_PATH this project has
  asked CMake to find a package configuration file provided by "ortools", but
  CMake did not find one.

  Could not find a package configuration file provided by "ortools" with any
  of the following names:

    ortoolsConfig.cmake
    ortools-config.cmake

  Add the installation prefix of "ortools" to CMAKE_PREFIX_PATH or set
  "ortools_DIR" to a directory containing one of the above files.  If
  "ortools" provides a separate development package or SDK, be sure it has
  been installed.

PORTS__-Wno-reserved-user-defined-literal
-- Performing 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 - Success
-- 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__-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
-- Found SWIG: /usr/bin/swig4.0 (found suitable version "4.0.2", minimum required is "3.0")  
-- Found Boost: /usr/lib/x86_64-linux-gnu/cmake/Boost-1.81.0/BoostConfig.cmake (found version "1.81.0")  
-- boost: 1.81.0
-- Found Python3: /usr/include/python3.9 (found version "3.9.2") 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.7.5") 
-- Found Doxygen: /usr/bin/doxygen (found version "1.9.1") found components: doxygen missing components: dot
-- STA version: 2.4.0
-- STA git sha: 555493cba6e476bd2ff0b9a543de7a781276c2b3
-- System name: Linux
-- Compiler: GNU 10.2.1
-- 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
-- Check if compiler accepts -pthread
-- Check if compiler accepts -pthread - yes
-- SSTA: 0
-- STA executable: /mnt/pdk/OL2/native/OpenROAD/src/sta/app/sta
-- GPU is not enabled
-- Configuring incomplete, errors occurred!

To Reproduce

run ./run.sh from the tcl_reproductible directory

Relevant log output

No response

Screenshots

No response

Additional Context

No response

eder-matheus commented 1 year ago

@smunaut Could you share the script you've used to create the existing signal route? Also, is the attached DEF file the output from your script?

These errors are generated because the paths of these routings are non-orthogonal. Here's an example:

image

Note that the last endpoint creates an "L" shape with the others. I understand that DRT expects the paths to be fully vertical or horizontal. @maliberty do you know more about it?

smunaut commented 1 year ago

So the script looks like this :

        wire = odb.dbWire.create(net)
        encoder = odb.dbWireEncoder()
        encoder.begin(wire)

        # Create vertical spine
        # (split in 2 segments to help RCX be more correct)
        encoder.newPath(self.layer_v, 'FIXED')
        encoder.addPoint(x_spine, y_min)
        encoder.addPoint(x_spine, y_ctrl[0])

        encoder.newPath(self.layer_v, 'FIXED')
        encoder.addPoint(x_spine, y_ctrl[1])
        encoder.addPoint(x_spine, y_max)

        # Create horizontal link for each mux
        for y in y_mux:
            encoder.newPath(self.layer_h, 'FIXED')
            encoder.addPoint(x_min, y)
            encoder.addPoint(x_spine, y)
            encoder.addPoint(x_max, y)

            encoder.newPath(self.layer_v, 'FIXED')
            encoder.addPoint(x_spine, y)
            encoder.addTechVia(self.via)

        encoder.end()

I also tried a bunch of different possibilites and couldn't figure out one that works. The only think I could make work is remove the final part that adds the via and then it works fine. (and I re-run my script after detailled routing to add the vias afterward).

What it must generate is vertical segment in the middle and then a bunhc of branches going left and right. I start a new path every time and the DEF also reflects that AFAICT so it should be valid.

eder-matheus commented 1 year ago

Your script looks fine to me. When I run DRT with the DEF file you've attached, it ends without errors. But running from the ODB file, the errors happen.

At what stage of the flow are you running your script? Is it right before the global router? Perhaps something is changing the paths in the encoder before the detailed routing stage.

smunaut commented 1 year ago

Yes, just right before global routing step.

This is how it look in OpenLane 2, my list of steps :+1:

Steps: List[Type[Step]] = [
        # ....
        CustomRoute,
        OpenROAD.GlobalRouting,
        OpenROAD.DetailedRouting,
        # ....
smunaut commented 1 year ago

The DEF and ODB get written both at the output of the custom script from the internal state :

    DesignFormat.DEF: lambda reader, file: odb.write_def(reader.block, file),
    DesignFormat.ODB: lambda reader, file: odb.write_db(reader.db, file),

It's just that the ODB gets fed to the GloblaRouting step to continue and the DEF is just "informational". But maybe something is wrong when saving the ODB and internal state is mis-represented somehow.

rovinski commented 1 year ago

This is a dumb question, but does removing the write to DEF change the db? I think there might be a known issue that writing files can corrupt the wire encoder.

maliberty commented 1 year ago

Write def should not affect the DB

smunaut commented 1 year ago

I tried forcing the ODB write before the DEF write, no changes.

Also attached is the exact ODB and DEF written at the output of my script. (i.e. before it gets altered by GlobalRouting)

user_project_wrapper.zip

eder-matheus commented 1 year ago

@smunaut Could you try removing the check_antennas/repair_antennas commands in the global route script? These commands run order_wires for the nets when looking for antenna violations so these commands may be changing the routing.

eder-matheus commented 1 year ago

@smunaut Could you also create a reproducible for the global routing stage?

smunaut commented 1 year ago

repair_antenna is already disabled.

I'm trying to figure out if/how I can generate a same reproducible build for the GRT step (they're made automatically when the step fails but of course GRT works :sweat: )

eder-matheus commented 1 year ago

repair_antenna is already disabled.

I'm trying to figure out if/how I can generate a same reproducible build for the GRT step (they're made automatically when the step fails but of course GRT works 😓 )

OpenLane have this read the docs page. This part in specific might be helpful: https://openlane.readthedocs.io/en/latest/for_developers/using_or_issue.html#odd-behavior

smunaut commented 1 year ago

That's for OpenLane "original". Here I'm using OpenLane 2 ( https://github.com/efabless/openlane2 ) which apparently doesn't support such things yet :/

smunaut commented 1 year ago

Did you check the ODB I posted above ? (which hasn't gone through global routing), are the routes correct in that ODB ?

eder-matheus commented 1 year ago

@smunaut Could you try this: https://github.com/The-OpenROAD-Project/OpenROAD/pull/3411? We noticed that your DEF and ODB files were not wrong, but the way the detailed router read the existing routing was incorrect. My PR should fix the false non-orthogonal wires error. It worked in the test case you've attached, could you try it at your end?

smunaut commented 1 year ago

@eder-matheus Yup seems to work for me !

eder-matheus commented 1 year ago

@eder-matheus Yup seems to work for me !

That's great! We're finishing the reviews in my PR. It should be in the master branch soon.