Skip to content

Remove runtime assert for kernel finish_counter. #37

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

Merged
merged 1 commit into from
Dec 6, 2021

Conversation

sherry-yuan
Copy link
Contributor

@sherry-yuan sherry-yuan commented Dec 6, 2021

Currently the kernel's finish_counter assert > 0 is called whenever there is a kernel interrupt sent. However the kernel interrupt could be sent whenever:

  1. printf buffer is full
  2. The kernel change state: init -> running -> finish/stall
  3. clGetProfileInfoIntelFPGA is called

From the first reason, it already doesn't make sense to assert finish counter greater than 0, as is very possible for a printf buffer to be full before the kernel finishes.

Currently the kerne's finish_counter assert > 0 is called whenever there is a kernel interrupt sent. However the kernel interrupt could be sent whenever:
1. printf buffer is full
2. The kernel change state: init -> running -> finish/stall
3. clGetProfileInfoIntelFPGA is called

From the first reason, it already doesn't make sense to assert finish counter greater than 0, as is very possible for a printf buffer to be full before the kernel finishes.
@sherry-yuan sherry-yuan requested a review from pcolberg December 6, 2021 18:45
@sherry-yuan sherry-yuan marked this pull request as ready for review December 6, 2021 18:45
@pcolberg pcolberg force-pushed the remove_finish_count_assert branch from 22901bd to 8a023ba Compare December 6, 2021 19:22
@pcolberg pcolberg added this to the 2022.2 milestone Dec 6, 2021
Copy link
Contributor

@pcolberg pcolberg left a comment

Choose a reason for hiding this comment

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

Thanks @sherry-yuan!

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