Skip to content

Conversation

RossBrunton
Copy link
Contributor

No description provided.

@RossBrunton RossBrunton requested a review from a team as a code owner September 25, 2024 13:54
@github-actions github-actions bot added the level-zero L0 adapter specific issues label Sep 25, 2024
RossBrunton added a commit to RossBrunton/intel-llvm that referenced this pull request Sep 25, 2024
@RossBrunton RossBrunton requested a review from pbalcer September 25, 2024 13:58
Copy link
Contributor

@pbalcer pbalcer left a comment

Choose a reason for hiding this comment

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

why?

@RossBrunton
Copy link
Contributor Author

RossBrunton commented Sep 25, 2024

why?

i-- < 0 is always false, since i is unsigned. Even if it was signed, it would still be false anyway, since i is always positive. I think this was meant to be a greater than rather than a less than?

@pbalcer
Copy link
Contributor

pbalcer commented Sep 25, 2024

why?

i-- < 0 is always false, since i is unsigned. Even if it was signed, it would still be false anyway, since i is always positive. I think this was meant to be a greater than rather than a less than?

:| It was supposed to be >. How is this passing tests?

@pbalcer pbalcer added the v0.10.x Include in the v0.10.x release label Sep 25, 2024
@alycm
Copy link
Contributor

alycm commented Sep 25, 2024

How is this passing tests?

Can a test be added to trigger this issue? It seems like maybe this entire case is currently missed (or would indeed surely be failing).

@pbalcer
Copy link
Contributor

pbalcer commented Sep 25, 2024

How is this passing tests?

Can a test be added to trigger this issue? It seems like maybe this entire case is currently missed (or would indeed surely be failing).

I added a test for this code path, but what that test verified is that the correct events are not removed from the list, not whether the intended events are removed. So not removing any events makes the test pass :)

This loop is intended to remove unneeded events from a waitlist. It's a performance optimization and may be needed to workaround (what I think is) a driver issue.

@nrspruit
Copy link
Contributor

preferred change:
#2134

@RossBrunton RossBrunton deleted the ross/rangefix branch February 19, 2025 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

level-zero L0 adapter specific issues v0.10.x Include in the v0.10.x release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants