Skip to content

[SYCL] Revert support for templated call operators #8047

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 2 commits into from
Jan 19, 2023

Conversation

srividya-sundaram
Copy link
Contributor

@srividya-sundaram srividya-sundaram commented Jan 18, 2023

This PR reverts the support for templated call operators in functors and functors without call operators which were introduced in #7104 and #7970

We found a regression internally with #7104 that #7970 attempted to fix. A subsequent review of #7970 internally, lead to identifying some pre-existing caveats with regards to supporting templated call operators in functor, supporting cases where there are multiple call operators in functor and making sure the right instantiated version is selected etc.

In order to better address the pre-existing issues as well as gaps in #7970 , it was decided to revert the above two
PRs.

@srividya-sundaram srividya-sundaram requested a review from a team as a code owner January 18, 2023 22:50
@srividya-sundaram srividya-sundaram changed the title Revert Revert support for templated call operators in functors and functors without call operators Jan 18, 2023
@smanna12
Copy link
Contributor

LGTM. Can you please add some description why you are reverting the patch for future reference?

@srividya-sundaram srividya-sundaram temporarily deployed to aws January 18, 2023 23:17 — with GitHub Actions Inactive
@srividya-sundaram srividya-sundaram temporarily deployed to aws January 19, 2023 00:05 — with GitHub Actions Inactive
@premanandrao
Copy link
Contributor

LGTM. Can you please add some description why you are reverting the patch for future reference?

We found a regression internally with #7104 that #7970 attempted to fix. A subsequent review of #7970 internally, found some deficiencies with the patch (with templated call operators). Rather than rush a fix for that, @srividya-sundaram is backing out both PRs and intends to architect a solution that addresses the concerns raised. She may be able to provide a better message to add in the description.

@smanna12
Copy link
Contributor

LGTM. Can you please add some description why you are reverting the patch for future reference?

We found a regression internally with #7104 that #7970 attempted to fix. A subsequent review of #7970 internally, found some deficiencies with the patch (with templated call operators). Rather than rush a fix for that, @srividya-sundaram is backing out both PRs and intends to architect a solution that addresses the concerns raised. She may be able to provide a better message to add in the description.

Thanks @premanandrao for the explanation!

@srividya-sundaram
Copy link
Contributor Author

LGTM. Can you please add some description why you are reverting the patch for future reference?

Updated the PR description with details.

@bader bader changed the title Revert support for templated call operators in functors and functors without call operators [SYCL] Revert support for templated call operators Jan 19, 2023
@bader bader merged commit 4afeea5 into intel:sycl Jan 19, 2023
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.

4 participants