Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add targets for writing GDS files #1031

Merged
merged 2 commits into from
Dec 28, 2023
Merged

Add targets for writing GDS files #1031

merged 2 commits into from
Dec 28, 2023

Conversation

lpawelcz
Copy link
Contributor

@lpawelcz lpawelcz commented Jun 16, 2023 •
edited

Closes #1024

This PR bumps bazel_rules_hdl in order to:

Additionally:

  • 65a89c3 adds target which builds GDS files for find_index example based on the results from previous place_and_route targets for ASAP7 and SKY130 technologies.
  • b67d8b0 adds targets for generating GDS files for recently added RLE encoder and decoder examples, also for ASAP7 and SKY130.

EDIT:
This PR depends on #1230

@mtdudek mtdudek force-pushed the gds-write branch 2 times, most recently from 936f202 to f4d64f0 Compare June 16, 2023 13:58
@lpawelcz lpawelcz force-pushed the gds-write branch 3 times, most recently from 9639d6b to 1222858 Compare July 17, 2023 06:35
@lpawelcz lpawelcz marked this pull request as ready for review July 17, 2023 06:36
@lpawelcz
Copy link
Contributor Author

PR is ready for review, I've updated the description. @proppy please take a look.

proppy
proppy reviewed Jul 23, 2023
View reviewed changes
WORKSPACE Outdated Show resolved Hide resolved
proppy
proppy reviewed Jul 23, 2023
View reviewed changes
xls/examples/fill.json Outdated Show resolved Hide resolved
proppy
proppy reviewed Jul 23, 2023
View reviewed changes
xls/examples/klayout.lyt Outdated
@@ -0,0 +1,158 @@
<?xml version="1.0" encoding="utf-8"?>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the actual upstream is https://github.com/efabless/sky130_klayout_pdk/blob/main/sky130_tech/tech/sky130/sky130.lyt

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a part of this PR anymore. It was added here only for testing purpose on the early stage of development.
It could be removed because KLayout tech files can now be supplied in bazel_rules_hdl with hdl/bazel_rules_hdl@5e2ec27

proppy
proppy reviewed Jul 23, 2023
View reviewed changes
xls/examples/BUILD Outdated Show resolved Hide resolved
@lpawelcz lpawelcz marked this pull request as draft July 24, 2023 14:39
@lpawelcz lpawelcz mentioned this pull request Jul 27, 2023
Merged
@lpawelcz lpawelcz marked this pull request as ready for review July 28, 2023 06:06
@lpawelcz
Copy link
Contributor Author

hdl/bazel_rules_hdl#173 is merged. I bumped bazel_rules_hdl dependency, rebased the code and fixed the commit history. @proppy this PR is ready for review again, please take a look.

proppy
proppy reviewed Aug 24, 2023
View reviewed changes
xls/modules/rle/BUILD Outdated Show resolved Hide resolved
proppy
proppy approved these changes Aug 24, 2023
View reviewed changes
Copy link
Member

@proppy proppy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just nits, can you also mention in the description that this also add the rules for the rle example?

@lpawelcz lpawelcz force-pushed the gds-write branch 2 times, most recently from 744075c to 67183e5 Compare August 31, 2023 12:02
@lpawelcz
Copy link
Contributor Author

@proppy Sorry for the delay, I addressed your comments: renamed the rules and updated PR description.

proppy
proppy approved these changes Aug 31, 2023
View reviewed changes
Copy link
Member

@proppy proppy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a nit.

proppy
proppy reviewed Sep 19, 2023
View reviewed changes
xls/examples/BUILD Outdated
@@ -32,6 +32,7 @@ load(
load("@rules_hdl//verilog:providers.bzl", "verilog_library")
load("@rules_hdl//synthesis:build_defs.bzl", "synthesize_rtl")
load("@rules_hdl//place_and_route:build_defs.bzl", "place_and_route")
load("@rules_hdl//gds_write:build_defs.bzl", "gds_write")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Load statement is out of its lexicographical order.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

proppy
proppy reviewed Sep 19, 2023
View reviewed changes
xls/modules/rle/BUILD
load("@rules_hdl//verilog:providers.bzl", "verilog_library")
load("@rules_hdl//synthesis:build_defs.bzl", "synthesize_rtl")
load("@rules_hdl//place_and_route:build_defs.bzl", "place_and_route")
load("@rules_hdl//gds_write:build_defs.bzl", "gds_write")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Load statement is out of its lexicographical order.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@lpawelcz
Copy link
Contributor Author

Fixed the load statements order and rebased the code onto 4c05344

@proppy
Copy link
Member

proppy commented Sep 28, 2023

Note: the internal review is currently blocked by a bazel_rules_hdl import (#ref 566360201)

@rw1nkler rw1nkler mentioned this pull request Sep 29, 2023
Open
@proppy proppy mentioned this pull request Oct 18, 2023
Open
@lpawelcz lpawelcz force-pushed the gds-write branch 2 times, most recently from 511e9fc to 72958fa Compare October 18, 2023 08:07
@lpawelcz
Copy link
Contributor Author

Builds fail on GDS generation with:

  File "/home/runner/.cache/bazel/_bazel_runner/5bd26e4beeff37176a5bf6fac1c974c7/sandbox/linux-sandbox/27899/execroot/com_google_xls/bazel-out/k8-opt-exec-2B5CBBC6/bin/external/rules_hdl/gds_write/def2stream.runfiles/rules_hdl_pip_deps_klayout/site-packages/klayout/tl/__init__.py", line 1, in <module>
    import klayout.tlcore
ModuleNotFoundError: No module named 'klayout.tlcore'

This is caused by eb2fb6d. I was able to fix the build locally by changing the python version from 3.9 to 3.8 but it's not a good solution. I believe this could be fixed by hdl/bazel_rules_hdl#209

@proppy
Copy link
Member

proppy commented Nov 24, 2023

eb2fb6d does pass the python interpreter target here: https://github.com/google/xls/blob/main/dependency_support/initialize_external.bzl#L37 but also set the version to 3.9 here: https://github.com/google/xls/blob/main/dependency_support/rules_hdl/workspace.bzl#L53

Do you have more logging from bazel when pip_parse get executed?
https://github.com/hdl/bazel_rules_hdl/blob/c7da44c0e96a2b76b95e9b5d58b0272b8aa14796/init.bzl#L60-L67

@lpawelcz lpawelcz force-pushed the gds-write branch 3 times, most recently from 52db084 to 4becdd3 Compare December 8, 2023 15:04
@lpawelcz lpawelcz mentioned this pull request Dec 8, 2023
Merged
@lpawelcz
Copy link
Contributor Author

lpawelcz commented Dec 8, 2023

Hi @proppy, I was actually able to fix this issue by fixing hermetic python handling in bazel_rules_hdl. I've opened a PR there: hdl/bazel_rules_hdl#240. Please take a look at it.

In this PR for the sake of testing my changes I've added a temporary commit which changes the source of bazel_rules_hdl to branch from the linked PR. I will change it to bumping bazel_rules_hdl version once hdl/bazel_rules_hdl#240 is merged.

This was referenced Dec 12, 2023
Open
Merged
@lpawelcz
Copy link
Contributor Author

@proppy I've moved bumping bazel_rules_hdl to a separate PR: #1230
It should be merged first and only then we can merge this one.
I've also updated place_and_route rules for examples used for GDS write because some of them had timing violations.

@lpawelcz lpawelcz marked this pull request as draft December 19, 2023 09:01
@lpawelcz
xls: examples: find_index: Add GDS generation rule
bd6b2d8 
Generate GDS for process technologies:
* ASAP7
* SKY130

Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>
@lpawelcz
xls: modules: rle: Add GDS generation rules
010cb92 
Add GDS write examples for RLE encoder and decoder for process
technologies:
* ASAP7
* SKY130

Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>
@lpawelcz lpawelcz marked this pull request as ready for review December 21, 2023 14:11
@copybara-service copybara-service bot merged commit 978dd8b into google:main Dec 28, 2023
3 of 4 checks passed
@lpawelcz lpawelcz deleted the gds-write branch December 29, 2023 09:37
@lpawelcz lpawelcz mentioned this pull request Jan 2, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@proppy proppy proppy approved these changes

Assignees
No one assigned
Labels
Reviewing Internally
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add targets for writing GDS files
2 participants
@lpawelcz @proppy