-
Notifications
You must be signed in to change notification settings - Fork 230
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
Removed test_harness_bridge and simplified harnesses #1442
Conversation
cac9e4a
to
6c2a8b3
Compare
92fbc89
to
1b8e3df
Compare
Instead, the test is now performed by `fasedtests_top`, in line with other tests. Also slightly simplified the test harnessed for both FASED and FireSim.
Something went wrong in the workflow monitor for CI run 4304296999. Verify CI instances are terminated properly. Must be checked before submitting the PR. Exception Message:
Traceback Message:
|
Instances for CI run 4304296999 were supposedly terminated. Verify termination manually. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The review was just left pending...
// Adds a new task to scheduler. | ||
void register_task(task_t &&task, uint64_t first_cycle); | ||
void register_task(uint64_t first_cycle, task_t &&task); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was there technical reason for switching the order here? If so i'm curious
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With clang-format, function calls look pretty bad unless the lambda is the last argument.
Co-authored-by: David Biancolin <[email protected]>
Instead, the test is now performed by
fasedtests_top
, in line with other tests. Also slightly simplified the test harnessed for both FASED and FireSim.Related PRs / Issues
UI / API Impact
Verilog / AGFI Compatibility
Contributor Checklist
changelog:<topic>
label?ci:fpga-deploy
label?Please Backport
label?Reviewer Checklist (only modified by reviewer)
Note: to run CI on PRs from forks, comment
@Mergifyio copy main
and manage the change from the new PR.changelog:<topic>
label?