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 cocotb framework #1057

Closed
wants to merge 3 commits into from
Closed

Add cocotb framework #1057

wants to merge 3 commits into from

Conversation

rw1nkler
Copy link
Contributor

@rw1nkler rw1nkler commented Jul 10, 2023 •
edited
Loading

This PR adds the Cocotb framework and simple driver and monitor classes to handle XLS channels in the cocotb simulation. A simple example is also given to verify the added functionality.

CC @proppy

@rw1nkler rw1nkler mentioned this pull request Jul 10, 2023
Merged
@proppy
Copy link
Member

proppy commented Jul 11, 2023 •
edited
Loading

I'm curious if you took a look at the existing iverilog integration ?

xls/xls/simulation/simulators/iverilog_simulator.cc

Line 45 in 835f742

absl::StatusOr<std::pair<std::string, std::string>> InvokeIverilog(

@rw1nkler rw1nkler force-pushed the 46586-cocotb-counter branch 2 times, most recently from e289186 to 5f57387 Compare July 18, 2023 10:51
@rw1nkler
Copy link
Contributor Author

rw1nkler commented Jul 18, 2023 •
edited
Loading

I'm curious if you took a look at the existing iverilog integration ?

xls/xls/simulation/simulators/iverilog_simulator.cc

Line 45 in 835f742

absl::StatusOr<std::pair<std::string, std::string>> InvokeIverilog(

@proppy Do you have something specific in mind? We moved the cocotb integration almost entirely to Bazel HDL rules so that we don't have to add (too much) code to XLS.

@rw1nkler rw1nkler marked this pull request as ready for review July 18, 2023 12:08
@proppy
Copy link
Member

proppy commented Jul 19, 2023

Do you have something specific in mind? We moved the cocotb integration almost entirely to Bazel HDL rules so that we don't have to add (too much) code to XLS.

I was wondering if it would make sense to expose a similar functionality to drive cocotb from the XLS simulator interface

xls/xls/simulation/verilog_simulator.h

Line 40 in 835f742

// Interface wrapping a Verilog simulator such Icarus verilog.

and the C++ test framework integration

xls/xls/simulation/verilog_test_base.h

Line 9 in 835f742

// Unless required by applicable law or agreed to in writing, software

but that would be challenging as I'm not sure cocotb expose any external facing API (and that would be redundant with the cocotb testbench itself).

I'm also curious however if the IR interpreter/JIT could fit with the https://docs.cocotb.org/en/stable/library_reference_c.html?highlight=GPI interface, so that you could drive DSLX/IR testbenches from cocotb.

@rdob-ant rdob-ant mentioned this pull request Jul 21, 2023
Closed
@rw1nkler rw1nkler force-pushed the 46586-cocotb-counter branch 3 times, most recently from f9fb8a4 to 0e11332 Compare July 21, 2023 20:24
@rw1nkler
Copy link
Contributor Author

rw1nkler commented Jul 21, 2023 •
edited
Loading

I'm also curious however if the IR interpreter/JIT could fit with the https://docs.cocotb.org/en/stable/library_reference_c.html?highlight=GPI interface, so that you could drive DSLX/IR testbenches from cocotb.

Cocotb is designed to interact directly with the RTL simulators and because of that, it relies on concepts like clock cycles which are not available in the XLS. I wonder whether this kind of integration is possible for DSLX/IR and if it would be beneficial.

@rw1nkler rw1nkler requested a review from proppy July 21, 2023 20:37
@proppy
Copy link
Member

proppy commented Jul 24, 2023

Cocotb is designed to interact directly with the RTL simulators and because of that, it relies on concepts like clock cycles which are not available in the XLS. I wonder whether this kind of integration is possible for DSLX/IR and if it would be beneficial.

#724 mentioned the ability to produce a schedule proto independently of running codegen, with that information it might be possible to run multiple concurrent IR JIT execution for each pipeline stage and produce a clock-aware and timing accurate simulation?

@rdob-ant rdob-ant mentioned this pull request Jul 25, 2023
Closed
@rw1nkler rw1nkler marked this pull request as draft August 22, 2023 13:02
@rw1nkler
Copy link
Contributor Author

Converted to draft while waiting for hdl/bazel_rules_hdl#175. If I manage to merge that PR I will update the git SHA of Bazel HDL Rules in this PR and mark it as "ready for review".

@rw1nkler
Copy link
Contributor Author

@proppy Can you take another look? I have changed the link to Bazel HDL Rules to a forked repository, but I will update the PR with a proper link once the corresponding PR in Bazel HDL Rules is merged.

proppy
proppy reviewed 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.

added more comments, sorry for the late review.

@proppy
Copy link
Member

proppy commented Aug 24, 2023

can we remove the draft label if it's ready for review?

@rw1nkler rw1nkler force-pushed the 46586-cocotb-counter branch 4 times, most recently from 073c5ad to 6493f0f Compare August 30, 2023 18:24
@rw1nkler rw1nkler marked this pull request as ready for review August 31, 2023 07:15
proppy
proppy reviewed Sep 7, 2023
View reviewed changes
xls/examples/passthrough.x Outdated Show resolved Hide resolved
proppy
proppy reviewed Sep 7, 2023
View reviewed changes
xls/examples/passthrough.x Outdated Show resolved Hide resolved
proppy
proppy reviewed Sep 7, 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.

Looks very good!

Just some remaining comment about the pytest integration and it should be good to merge.

@proppy
Copy link
Member

proppy commented Sep 19, 2023

@rw1nkler did you have a chance to take a look at my last round of comments?

@rw1nkler
dependency_support: Add cocotb and cocotb_bus to python requirements
398399a 
Internal-tag: [#46586]
Signed-off-by: Robert Winkler's avatarRobert Winkler <rwinkler@antmicro.com>
@rw1nkler
simulation: Add cocotb python library for XLS
4b09202 
The library contains XLSChannel, XLSChannelDriver and XLSChannelMonitor classes.

* XLSChannel - provides a mechanism for wrapping all signals related to XLS channels into one object.
* XLSChannelDriver - may be used to send data to XLS channel
* XLSChannelMonitor - may be used to monitor transaction taking place in XLS Channel

Internal-tag: [#46586]
Signed-off-by: Robert Winkler <rwinkler@antmicro.com>
@rw1nkler
examples: Add a simple passthrough example that uses cocotb for valid…
10b0a1c 
…ation

This commit adds a simple DSLX module that sends back the information
received on the input channel. The example contains tests written in
DSLX to verify the IR, as well as tests that use Cocotb framework to validate
behavior of the generated Verilog sources.

Internal-tag: [#46586]
Signed-off-by: Robert Winkler <rwinkler@antmicro.com>
@rw1nkler
Copy link
Contributor Author

rw1nkler commented Oct 6, 2023

I looked at your comments and applied them to this PR. Also, I asked the Cocotb maintainers for more details regarding the Cocotb-pytest integration.

@proppy
Copy link
Member

proppy commented Nov 24, 2023

I asked the Cocotb maintainers for more details regarding the Cocotb-pytest integration.

do you have a pointer to the conversation?

@rw1nkler
Copy link
Contributor Author

Sure, here is the link:
cocotb/cocotb#3369

Unfortunately, it turned out that the release didn't include my changes to cocotb. They will probably be added to the next release

@proppy
Copy link
Member

proppy commented Apr 3, 2024

Was there a new release including those change? If not, maybe showing that integration in a separate repo similar to https://github.com/antmicro/xls-cosimulation-demonstrator would be easier? wdyt?

@rw1nkler
Copy link
Contributor Author

rw1nkler commented Apr 4, 2024 •
edited
Loading

The integration already worked on the previous version of the Bazel environment, which allowed reusing the non-hermetic clang toolchain for building cocotb. To make things right in a hermetic environment, we have to either wait for the official release, build our version separately and expose it as an artifact, or provide rules for building the cocotb package inside the Bazel workspace.

@rw1nkler rw1nkler mentioned this pull request May 17, 2024
Closed
copybara-service bot pushed a commit that referenced this pull request May 24, 2024
@rw1nkler @copybara-github
Merge #1416 by rw1nkler: Add a passthrough example
af888be 
This PR adds a simple proc that forwards the received information from an input channel to an output channel. We found this particular proc surprisingly useful when testing various tools within the toolchain or when reporting bugs.

We used it in:
* #1415
* #1410
* #1392
* #1057

COPYBARA_INTEGRATE_REVIEW=#1416 from antmicro:passthrough 907a62b
PiperOrigin-RevId: 636954688
@proppy proppy mentioned this pull request Sep 24, 2024
Open
@proppy
Copy link
Member

proppy commented Sep 24, 2024

Should we close this in favor of #1616?

@rw1nkler
Copy link
Contributor Author

rw1nkler commented Sep 26, 2024 •
edited
Loading

Should we close this in favor of #1616?

A more detailed answer has been provided in #1616 (comment)

@rw1nkler
Copy link
Contributor Author

rw1nkler commented Oct 3, 2024

Closing in favor of #1616, as requested in #1616 (comment)

@rw1nkler rw1nkler closed this Oct 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@proppy proppy proppy left review comments

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

Successfully merging this pull request may close these issues.
2 participants
@rw1nkler @proppy