Skip to content
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

Remove FE dependency on SYCL header scope of accessor properties #3529

Closed
schittir opened this issue Apr 10, 2021 · 7 comments
Closed

Remove FE dependency on SYCL header scope of accessor properties #3529

schittir opened this issue Apr 10, 2021 · 7 comments
Assignees
Labels
upstream This change is related to upstreaming SYCL support to llorg.

Comments

@schittir
Copy link
Contributor

schittir commented Apr 10, 2021

Remove the fragility caused by the FE relying on SYCL header scope gathered from the DeclContext; This needs to be addressed for not only no_alias, but also buffer_location and other (future) accessor properties such as no_offset and noinit. This can be addressed using an attribute consistently for all the accessor properties. Creating this task to as a follow-up to the conversation here: #3452

Originally posted by @schittir in #3452 (comment)

@schittir
Copy link
Contributor Author

schittir commented Jun 2, 2021

Hi @AaronBallman - I am considering picking up this issue soon; there is still the case for it, but I don't think no_offset support is implemented yet. Could you please share your thoughts on this/what you would like to see?

@AaronBallman
Copy link
Contributor

We should check with @bader and probably @erichkeane as well, but my thinking was that we should have an undocumented attribute that only works in system headers along the lines of what's prototyped in #2091. I'm not certain where this PR ended up or why it was dropped though, so it'd be good to understand what was learned from it.

@erichkeane
Copy link
Contributor

Yeah, that is supposed to be the direction of the fix. @Fznamznon stopped working on CFE tasks, so I'm guessing that is why this was abandoned.

@bader bader added the upstream This change is related to upstreaming SYCL support to llorg. label Jun 3, 2021
@bader
Copy link
Contributor

bader commented Jun 3, 2021

@schittir, according to my understanding @zahiraam is about to start working on #2091.
Please, sync with @zahiraam to avoid work duplication.

@zahiraam
Copy link
Contributor

zahiraam commented Jun 3, 2021

@schittir, according to my understanding @zahiraam is about to start working on #2091.
Please, sync with @zahiraam to avoid work duplication.

Yes I have started looking at the #2091 that implements the sycl_class_attribute.

@schittir schittir assigned zahiraam and unassigned schittir Jun 3, 2021
@schittir
Copy link
Contributor Author

schittir commented Jun 3, 2021

Thank you all for your comments/updates! I transferred this issue to @zahiraam

@zahiraam
Copy link
Contributor

Implementation of the sycl_special_class attribute is here: #3892

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
upstream This change is related to upstreaming SYCL support to llorg.
Projects
None yet
Development

No branches or pull requests

5 participants