search

hdl / conda-eda

Conda recipes for FPGA EDA tools for simulation, synthesis, place and route and bitstream generation.
https://anaconda.org/LiteX-Hub
Apache License 2.0
94 stars 27 forks source link

OpenROAD: Fix package #366

Closed ubfx closed 7 months ago

ubfx commented 8 months ago

Currently, the OpenROAD build fails, because this patch can't be applied.

I actually had to do some more changes in my fork to get the build working again, but I'm not sure how much of that is because of my runner. So maybe this will do.

Fix https://github.com/hdl/conda-eda/issues/368

proppy commented 7 months ago

Thanks for the patch! Let me check w/ the CI if it fixes #368

proppy commented 7 months ago

still seeing:

2024-02-21T10:08:47.1051513Z 10:08:47 | Error was: 
2024-02-21T10:08:47.1053469Z 10:08:47 | Command '['/root/conda-eda/conda-eda/workdir/conda-env/bin/patch', '--no-backup-if-mismatch', '--batch', '-Np1', '-i', '/tmp/tmpg6s06yi_/disable-cluster-flops-cmd.patch.native', '--binary', '--dry-run']' returned non-zero exit status 1.
ubfx commented 7 months ago

Sorry, it seems like the files changed again since I last looked at it. I'm updating the patch now

ubfx commented 7 months ago

Seems like this fixed it in my fork: https://github.com/ubfx/conda-eda/actions/runs/7987639828/job/21810507800#step:5:1480

proppy commented 7 months ago

@ubfx thanks for the quick turnaround, running the CI.

proppy commented 7 months ago

Looks like we miss an stdlib include?

2024-02-21T11:23:40.7739118Z 11:23:40 | /root/conda-eda/conda-eda/workdir/conda-env/conda-bld/openroad_1708513964138/work/src/dpl/src/Opendp.cpp:664:14: error: 'accumulate' is not a member of 'std'
2024-02-21T11:23:40.7740773Z 11:23:40 |   664 |       = std::accumulate(grid_sites.begin(),
2024-02-21T11:23:40.7741574Z 11:23:40 |       |              ^~~~~~~~~~
2024-02-21T11:23:41.5824918Z 11:23:41 | make[2]: *** [src/dpl/CMakeFiles/dpl_lib.dir/build.make:76: src/dpl/CMakeFiles/dpl_lib.dir/src/Opendp.cpp.o] Error 1
2024-02-21T11:23:41.5826677Z 11:23:41 | make[1]: *** [CMakeFiles/Makefile2:2673: src/dpl/CMakeFiles/dpl_lib.dir/all] Error 2
2024-02-21T11:23:41.9383108Z 11:23:41 | make[1]: *** Waiting for unfinished jobs....
ubfx commented 7 months ago

The ortools patch problem seems fixed, now we're seeing:

py37:

11:23:40 | /root/conda-eda/conda-eda/workdir/conda-env/conda-bld/openroad_1708513964138/work/src/dpl/include/dpl/Opendp.h: In member function 'int dpl::GridInfo::getSitesTotalHeight() const':
  11:23:40 | /root/conda-eda/conda-eda/workdir/conda-env/conda-bld/openroad_1708513964138/work/src/dpl/include/dpl/Opendp.h:230:17: error: 'accumulate' is not a member of 'std'

py38 and py310:

/root/conda-eda/conda-eda/workdir/conda-env/conda-bld/openroad_1708513921126/work/src/gui/src/layoutTabs.cpp:113:11: error: 'qOverload' was not declared in this scope
  11:22:30 |   113 |           qOverload<const Selected&>(&LayoutViewer::addSelected),

The accumulate error I also saw on my runner, and can be fixed by adding #include <numeric>. Not sure about the qOverload one because I haven't seen that before.

proppy commented 7 months ago

Let's see what the CI says ? https://github.com/hdl/conda-eda/actions/runs/7988449270/job/21813113707?pr=366 :)

proppy commented 7 months ago

still getting:

2024-02-21T11:59:28.9049370Z 11:59:28 | /root/conda-eda/conda-eda/workdir/conda-env/conda-bld/openroad_1708516137315/work/src/gui/src/layoutTabs.cpp:113:11: error: 'qOverload' was not declared in this scope
2024-02-21T11:59:28.9051137Z 11:59:28 |   113 |           qOverload<const Selected&>(&LayoutViewer::addSelected),

for python > 3.7 . but openroad-linux-py37 seems to be passing! \o/ https://github.com/hdl/conda-eda/actions/runs/7988449270/job/21813113707?pr=366

proppy commented 7 months ago

maybe missing include or diverging CXX_STANDARD version?

ubfx commented 7 months ago

maybe missing include or diverging CXX_STANDARD version?

Seems like qOverload was introduced in Qt 5.7, but the py37 build runs on 5.6.8. So there is probably a switch somewhere which disables qOverload for the py37 build, explaining why it doesn't see this error. For the 3.8 and 3.10 builds, building with newer Qt versions, we might have to #include <QtGlobal>. I'll give that a try.

proppy commented 7 months ago

For the 3.8 and 3.10 builds, building with newer Qt versions, we might have to #include . I'll give that a try.

should we pin the qt version to 5.6.8?

proppy commented 7 months ago

Looking at the logs it looks like for py38 and py310 the version of qt that gets resolved is older!

2024-02-21T13:22:37.0321311Z 13:22:37 |     - qt 5.6.3 h8bf5577_3
2024-02-21T13:22:50.5482877Z 13:22:50 |     - qt 5.6.3 h8bf5577_3

while py37 resolves to:

2024-02-21T13:22:26.3106916Z 13:22:26 |     - qt 5.9.7 h5867ecd_1
ubfx commented 7 months ago

it looks like for py38 and py310 the version of qt that gets resolved is older!

Ah that's probably it! Not sure where I got those version numbers from earlier. 😄 By the way - I created a PR for the numeric include issue over at https://github.com/The-OpenROAD-Project/OpenROAD/pull/4696 so we will probably have to remove that patch again once that is merged.

proppy commented 7 months ago

all green! image

proppy commented 7 months ago

@ubfx do you want me to wait for https://github.com/The-OpenROAD-Project/OpenROAD/pull/4696 to land before merging?

ubfx commented 7 months ago

@ubfx do you want me to wait for The-OpenROAD-Project/OpenROAD#4696 to land before merging?

Up to you, I can also create a new PR on here as soon as it lands.

proppy commented 7 months ago

Let's unblock this then, and you (or I!) can follow up with a PR removing the patches once https://github.com/The-OpenROAD-Project/OpenROAD/pull/4696 lands.

Thanks a lot for working thru this together!