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

New commands for profiling test #1780

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

gwawiork
Copy link
Contributor

@gwawiork gwawiork commented Jul 6, 2023

  • clEnqueueMarkerWithWaitList
  • clEnqueueBarrierWithWaitList
    The test verifies if:
  • sync commands have proper timestamps
  • ndrange commands before and after sync have proper timestamps (marker, barrier works correctly)

* clEnqueueMarkerWithWaitList
* clEnqueueBarrierWithWaitList
test_error(error, "Unable to set argument for test kernel");

error = create_single_kernel_helper(context, &program, &kernel3, 1,
&test_kernel, "test1");
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be test3?

fn(queue_with_props, 3, &events_list_set2[0], &eventEnqueueMarkerSet2);
test_error(error, "Unable to enqueue sync command");

error = clWaitForEvents(1, &eventEnqueueMarkerSet2);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should clWaitForEvents be switched to eventEnqueueMarkerSet1 since that's what's being checked in the following lines, the command queue is out of order, and set 1 doesn't wait for set 2 to complete? Or maybe clFinish would be more readable?

@gwawiork
Copy link
Contributor Author

@lakshmih Thanks for review. Unfortunately I see some errors on our internal driver version. Until these issues will not be solved I do not plan any updates in this PR. I will let you know when it will be ready for review.

@lakshmih
Copy link
Contributor

@lakshmih Thanks for review. Unfortunately I see some errors on our internal driver version. Until these issues will not be solved I do not plan any updates in this PR. I will let you know when it will be ready for review.

Thank you. Could you review the focused review label and add it back when ready?

@gwawiork
Copy link
Contributor Author

@lakshmih Thanks for review. Unfortunately I see some errors on our internal driver version. Until these issues will not be solved I do not plan any updates in this PR. I will let you know when it will be ready for review.

Thank you. Could you review the focused review label and add it back when ready?

I think I do not have permission to modify labels.

@lakshmih
Copy link
Contributor

lakshmih commented Oct 2, 2023

@lakshmih Thanks for review. Unfortunately I see some errors on our internal driver version. Until these issues will not be solved I do not plan any updates in this PR. I will let you know when it will be ready for review.

Thank you. Could you review the focused review label and add it back when ready?

I think I do not have permission to modify labels.

Oh, thanks for letting me know. I removed it for now, we can add it back when ready.

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.

3 participants