Minor fixes with block handling#3208
Conversation
Do not export GDS_FILES with GDS files for macro blocks. This variable should be set in the platform specific config.mk file via ADDITIONAL_GDS_FILES, which has the additional macro GDS files included. Signed-off-by: Daniel Schultz <dnltz@aesc-silicon.de>
Use WORK_HOME instead of relative paths to result, logs, reports and objects. Signed-off-by: Daniel Schultz <dnltz@aesc-silicon.de>
Also provide ADDITIONAL_LIB variables for SLOW and FAST corners to allow generating blocks with Multi Corner analysis enabled. Add an additional ADDITIONAL_TYP_LIB variable as default for typical timings. Typical timings is required to generate a lib file for Yosys to replace the actual Verilog module. Signed-off-by: Daniel Schultz <dnltz@aesc-silicon.de>
Also generate all .lib files when timing corners are enabled in a design. Signed-off-by: Daniel Schultz <dnltz@aesc-silicon.de>
|
@oharboe FYI |
|
@maliberty My vision for ORFS is to make it less complex and leave all advanced flows to bazel and bazel-orfs. BLOCKS in ORFS is inflexible and an eyesore, but useful for now for a few ORFS test cases, but as soon as you want to do something interesting it breaks apart as a solution. Similarly, I wonder if ORFS already has everything for corners and that corners should be done in bazel-orfs on top of ORFS. This PR has some breaking changes, but I want to first discuss direction. |
|
I think you can advocate for people to use bazel-orfs but we should not block people from using orfs is they choose. The BLOCKS code already exists so having it not work well isn't great. It would be easier to make such a case against a totally new feature. Does bazel-orfs handle multi-corner analysis already? This is a chance for a sale 😄 |
|
I believe that there are lots of more advanced flows that ORFS will struggle to handle and that ORFS has no good way of testing. You will be adding buggy code that complicates the codebase. Proof in point: this already breaks use-cases. If I can understand corner support today, I dont quite understand the flow, I think I can create an example in bazel-orfs. This does not appear to be a big challenge for a garden variety user specific flow. Here, for instance, I can imagine that there is a heterogenous flow where typical timing is used for some macros, but not others, which would not be supported by this code. |
|
Another thing: I dont think new PDKs and their test cases should go into ORFS git, but their own git repository(perhaps one per tech or foundry). Example use cases for such PDKs should into their own git repository a la megaboom. Fixes to ORFS should generally be PDK agnostic. All of this is easy to tie together with bazel-orfs and getting easier. |
|
This is getter further afield from this PR. If you want to discuss the future broadly let's have a separate discussion. |
I'm arguing that trying to push BLOCKS further will end in tears. Better to leave it as-is as a limited feature intended only for ORFS test flows of the existing complexity and move more advanced flow tests and usage to use ORFS as a component in a solution. My next step is to understand how corners work in OpenROAD and provide an example use-case in bazel-orfs so that we have something specific to compare against and discuss. |
| export ADDITIONAL_LIBS += $(BLOCK_LIBS) | ||
| export ADDITIONAL_GDS += $(BLOCK_GDS) | ||
| export GDS_FILES += $(BLOCK_GDS) | ||
| ifneq ($(CDL_FILES),) |
There was a problem hiding this comment.
@maliberty Isn't this a standalone bugfix? Split out into separate PR?
I'm not very familiar with the gds part of the flow.
There was a problem hiding this comment.
Yes, platforms should set GDS_FILES and use ADDITIONAL_GDS.
|
@maliberty Is there any OpenROAD GUI support for multiple corners? |
|
@maliberty What about rules-base.json testing? Today it ORFS can set a single corner via CORNER for the entire flow and then test at the end, but rules-base.json would have to be compare against something against all CORNERS. Does having multiple CORNERS defined have an effect on the quality of results or is it purely a reporting issue? |
I agree. We either remove this feature entirely or fix it. Otherwise someone else will give it a try and fix it (again). |
We need this variable in the BLOCKS flow before variables.mk gets included. Copy the same export to have this variable set in case WORK_HOME is not set. Signed-off-by: Daniel Schultz <dnltz@aesc-silicon.de>
dnltz
force-pushed
the
WIP/dnltz/fix-block-logic-general-changes
branch
from
170a8d8 to
deb4195
Compare
|
@maliberty can you take a look at this PR and make a decision? |
maliberty
left a comment
maliberty
left a comment
There was a problem hiding this comment.
I think this is ok. The change to use _typ when no corners is defined simplifies the code but is slightly different than the current operation where no suffix is used. I think the tradeoff is ok.
| export DESIGN_HOME ?= $(FLOW_HOME)/designs | ||
| export PLATFORM_HOME ?= $(FLOW_HOME)/platforms | ||
| export WORK_HOME ?= . | ||
| # WORK_HOME is set up in flow/Makefile |
There was a problem hiding this comment.
Nit: I'm removing this comment in a followup PR. This is a comment left in place for a deleted line. Several variables are defined in flow/Makefile, WORK_HOME is not special in that regard.
3 participants
.libfile to_typ.lib.WORK_HOMEvariable to BLOCKS in the base MakefileGDS_HOMEin base Makefile when BLOCKS is set because this file should be assigned in the platform config