Skip to content

[SYCL] refactor annotated_arg operator forwarding #12232

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 9 commits into from
Jan 11, 2024

Conversation

broxigarchen
Copy link
Contributor

@broxigarchen broxigarchen commented Dec 21, 2023

Refactor annotated_arg operator forwarding regarding to #11971 and #12140

  1. add binary and unary operator forwarding. We don't need inc/dec and compound here since SYCL kernel arg are const member and cannot be modified
  2. replace the old impl with hidden friend operators

@broxigarchen broxigarchen requested review from a team as code owners December 21, 2023 17:44
@broxigarchen broxigarchen changed the title refactor annotated_arg operator forwarding [SYCL] refactor annotated_arg operator forwarding Dec 21, 2023
Copy link
Contributor

github-actions bot commented Dec 21, 2023

✅ With the latest revision this PR passed the C/C++ code formatter.

@broxigarchen
Copy link
Contributor Author

broxigarchen commented Dec 21, 2023

:white_check_mark: With the latest revision this PR passed the C/C++ code formatted.

There seems to be a conflict with the C++ code formatter and lint CI on the clang format

Hi @againull do you know about this?

@wangdi4
Copy link
Contributor

wangdi4 commented Jan 5, 2024

Hi @againull, the CI has passed except "Check code formatting". It turns out "Check code formatting" and lint have different rules when checking the format. How should I workaround it?

  • "Check code formatting": -> decltype(...
  • lint: ->decltype(...

@steffenlarsen
Copy link
Contributor

steffenlarsen commented Jan 5, 2024

I suggest we follow the old lint runs until we have fully migrated, as otherwise we will not get CI results. It will mean we need to bypass branch protection when merging patches with formatter disagreements. @aelovikov-intel - Do you have other preferences?

@aelovikov-intel
Copy link
Contributor

aelovikov-intel commented Jan 5, 2024

I suggest we follow the old lint runs until we have fully migrated, as otherwise we will not get CI results. It will mean we need to bypass branch protection when merging patches with formatter disagreements. @aelovikov-intel - Do you have other preferences?

I'm going to disable/remove old linter today (unless something goes wrong).

Update: uploaded #12308

@wangdi4
Copy link
Contributor

wangdi4 commented Jan 11, 2024

Hi @againull, is this PR ready for merge?

@aelovikov-intel
Copy link
Contributor

clang-format is failing, that needs to be fixed before merging.

@wangdi4
Copy link
Contributor

wangdi4 commented Jan 11, 2024

clang-format is failing, that needs to be fixed before merging.

Hi @aelovikov-intel, the code format has been fixed

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.

6 participants