Skip to content

[SYCL][HIP] Support lowerWGScope and fix hip event #4951

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
Jan 12, 2022

Conversation

malixian
Copy link
Contributor

@malixian malixian commented Nov 12, 2021

Support lowerWGScope for AMDGPU and fix _pi_event::is_completed() function.

@malixian malixian changed the title support lowerWGScope and fix hip event [SYCL][HIP] Support lowerWGScope and fix hip event Nov 12, 2021
@malixian
Copy link
Contributor Author

malixian commented Nov 16, 2021

Any questions about this PR?

Copy link
Contributor

@kbobrovs kbobrovs left a comment

Choose a reason for hiding this comment

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

LowerWGScope.cpp LGTM. Good to see enabling HIP is that simple.

Should corresponding tests from https://github.com/intel/llvm-test-suite/tree/intel/SYCL/HierPar
be enabled for HIP as well?

@malixian
Copy link
Contributor Author

malixian commented Nov 17, 2021

Should corresponding tests from https://github.com/intel/llvm-test-suite/tree/intel/SYCL/HierPar be enabled for HIP as well?

I test from SYCL-CTS/tests/h_item, handler and stream.

@kbobrovs
Copy link
Contributor

I test from SYCL-CTS/tests/h_item, handler and stream.

OK. But llvm-test-suite/tree/intel/SYCL/HierPar should still pass - correct? If so, why not enable them?

@malixian
Copy link
Contributor Author

malixian commented Nov 17, 2021

OK. But llvm-test-suite/tree/intel/SYCL/HierPar should still pass - correct? If so, why not enable them?

Yes, it's a good case to test this feature. But I first encountered this problem in the sycl-cts benchmark.

@againull againull merged commit 9eb27d3 into intel:sycl Jan 12, 2022
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