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

Setup simulation-based testing flow #21

Merged
merged 6 commits into from
Jan 9, 2025

Conversation

Scheremo
Copy link
Collaborator

@Scheremo Scheremo commented Jan 7, 2025

This PR fills in the gaps for running chimera-sdk tests with simulation tools (VCS, Verilator).

Added

  • Variables TEST_MODE, SIMULATON_BINARY, PRELOAD_MODE which control the simulation launch

Changed

  • add_chimera_test now invokes SIMULATION_BINARY with plusargs for BINARY and PRELMODE corresponding to the chimera / cheshire testbench convention.

@Scheremo Scheremo requested review from Xeratec and viv-eth January 7, 2025 11:43
Copy link
Collaborator

@Xeratec Xeratec left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! In principle looks good, but I have a few comments. Most important, I would like to support other simulation platforms such as GVSoC in the future.

Furthermore, if you have some time and extend the Sphinx documentation with a minimal explanation of how to run the test, this would be great, but is not strictly necessary.

CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
cmake/Utils.cmake Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
Copy link
Collaborator

@Xeratec Xeratec left a comment

Choose a reason for hiding this comment

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

Thanks for the feedback. I was thinking on another level of abstraction, and most comments are resolved now. Open points:

  • Rename "Simulation Binary"
  • List of available models

CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
@Scheremo Scheremo requested a review from Xeratec January 8, 2025 13:58
@Scheremo
Copy link
Collaborator Author

Scheremo commented Jan 8, 2025

Fixed the open points, and actually flip-flopped on test building; I think it's not a bad idea to make sure the tests build in any case (Ensures the toolchain works, the code is not deeply broken,...), so I reenabled the default building of tests, but added a switch for adding them to the ctest pipeline, which is now contingent on TEST_MODE being "simulation" - this can be easily extended for gvsoc or other platforms.

Copy link
Collaborator

@Xeratec Xeratec left a comment

Choose a reason for hiding this comment

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

All good, except the formatting of the RST documentation for the CMake add_chimera_subdirectories function.

cmake/Utils.cmake Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
Copy link
Collaborator

@Xeratec Xeratec left a comment

Choose a reason for hiding this comment

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

LGTM

@Xeratec Xeratec merged commit 99ff2fe into pulp-platform:devel Jan 9, 2025
4 checks passed
@Scheremo Scheremo deleted the pr/testing branch January 9, 2025 09:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants