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.

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

[ESI] Add hostmem write support to cosim #8059

Merged
merged 1 commit into from
Jan 14, 2025
Merged

Conversation

teqdruid
Copy link
Contributor

@teqdruid teqdruid commented Jan 9, 2025

Adds cosim support to both the cosim BSP in PyCDE and the corresponding support in the runtime. As with hostmem reads, write only supports one client and no gearboxing.

@teqdruid teqdruid added ESI PyCDE Python CIRCT Design Entry API labels Jan 9, 2025
@teqdruid teqdruid marked this pull request as draft January 9, 2025 22:08
@teqdruid teqdruid force-pushed the teqdruid/cosim-hostmem-write branch from 8b29dfd to 5fa406f Compare January 9, 2025 22:31
@teqdruid teqdruid force-pushed the teqdruid/cosim-hostmem-write branch from 5fa406f to c4e3cb7 Compare January 9, 2025 22:38
@teqdruid teqdruid marked this pull request as ready for review January 14, 2025 15:33
@teqdruid teqdruid requested a review from Copilot January 14, 2025 16:07

Choose a reason for hiding this comment

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

Copilot reviewed 5 out of 9 changed files in this pull request and generated no comments.

Files not reviewed (4)
  • lib/Dialect/ESI/runtime/cpp/lib/backends/Cosim.cpp: Language not supported
  • lib/Dialect/ESI/runtime/cpp/tools/esitester.cpp: Language not supported
  • frontends/PyCDE/test/test_esi.py: Evaluated as low risk
  • frontends/PyCDE/src/pycde/signals.py: Evaluated as low risk
Comments suppressed due to low confidence (4)

frontends/PyCDE/src/pycde/bsp/common.py:339

  • [nitpick] The variable name 'write_req_bundle_type' is ambiguous. It should be renamed to 'write_req_bundle_type_def' to clarify that it is a type definition.
write_req_bundle_type = esi.HostMem.write_req_bundle_type(data_type)

frontends/PyCDE/src/pycde/bsp/common.py:305

  • The new functionality for handling write requests should be covered by tests to ensure it works as expected. Specifically, the behavior introduced in the 'build_tagged_write_mux' method should be tested.
def build_tagged_write_mux(ports, reqs: List[esi._OutputBundleSetter]) -> BundleSignal:

frontends/PyCDE/integration_test/esitester.py:130

  • Ensure that cmd.offset is of type UInt(32) to avoid type mismatch errors.
write_loc_ce = mmio_xact & cmd.write & (cmd.offset == UInt(32)(0))

frontends/PyCDE/integration_test/esitester.py:127

  • [nitpick] Improve the error message in cmd_chan_wire.unwrap to be more descriptive.
cmd, cmd_valid = cmd_chan_wire.unwrap(resp_ready_wire)
@teqdruid teqdruid merged commit 380b2ad into main Jan 14, 2025
4 checks passed
@teqdruid teqdruid deleted the teqdruid/cosim-hostmem-write branch January 14, 2025 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ESI PyCDE Python CIRCT Design Entry API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant