-
Notifications
You must be signed in to change notification settings - Fork 799
[SYCL] Fix an assertion in graph processor #138
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
Conversation
Signed-off-by: Sergey Semenov <sergey.semenov@intel.com>
@@ -32,7 +32,6 @@ Scheduler::GraphProcessor::getWaitList(EventImplPtr Event) { | |||
|
|||
void Scheduler::GraphProcessor::waitForEvent(EventImplPtr Event) { | |||
Command *Cmd = getCommand(Event); | |||
assert(!Cmd && "Event has no associated command?"); |
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.
Could you clarify in the commit message the reason why this assertion is erroneous, please?
Test case would be great too.
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.
It seems there is just a typo in assert condition:
Should be: assert(Cmd && "Event has no associated command?");
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.
Right, fixed.
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.
I think it doesn't make sense to write test on this because the problems reproduces on any test which submits commands with asserts enabled.
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.
I think it doesn't make sense to write test on this because the problems reproduces on any test which submits commands with asserts enabled.
Are you trying to say that this code is already covered by existing LIT tests?
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.
Are you trying to say that this code is already covered by existing LIT tests?
Yes, this assertion leads to about 30 LIT test failures in debug mode without this patch.
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.
When asserts are enabled*
[vecz] Fix alignment of load operations
[vecz] Fix alignment of load operations
[vecz] Fix alignment of load operations
Signed-off-by: Sergey Semenov sergey.semenov@intel.com