[PATCH] D92004: [OpenCL] add CL 3.0 optional feature support to opencl-c.h

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In D92004#2857511 , @airlied wrote:

> I'll have to rebase/rebuild my build env, I'll try and have something 
> rebasing this cleaner soon.

Cool, thanks! If you could split/partition into multiple patches in some way it 
would be good. FYI the clang 13 release branch will be taken in a few weeks 
(before end of July!) so we still have time to get this into the upcoming 
release.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92004/new/

https://reviews.llvm.org/D92004

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92004: [OpenCL] add CL 3.0 optional feature support to opencl-c.h

[Dave Airlie via Phabricator via cfe-commits]

airlied added a comment.

I'll have to rebase/rebuild my build env, I'll try and have something rebasing 
this cleaner soon.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92004/new/

https://reviews.llvm.org/D92004

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92004: [OpenCL] add CL 3.0 optional feature support to opencl-c.h

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

@airlied I think this patch is outdated as some features have been committed in 
the meantime. Are you still interested in working on this change? It might help 
splitting into smaller independent chunks. It is usually faster to review and 
test.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92004/new/

https://reviews.llvm.org/D92004

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92004: [OpenCL] add CL 3.0 optional feature support to opencl-c.h

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

We have discussed testing of headers within OpenCL developer community and 
wider:
https://lists.llvm.org/pipermail/cfe-dev/2021-April/068040.html

and the conclusions I have drawn are as follows:

- There seem to be no big interest in improving upstream testing of 
`opencl-c.h` and therefore improving the quality assurance processes in 
upstream development.
- There is no plan for `opencl-c.h` to be exposed to the end user in upstream. 
An alternative newer solution using Tablegen is currently enabled by default 
through the clang driver as a primary header.
- It is likely that `opencl-c.h` will be deprecated or even removed in the 
future releases however this discussion hasn't fully taken place yet.

Considering the above and the fact that the `opencl-c.h` header will continue 
to exist for the time being as it has been adopted by various out-of-tree 
implementations of OpenCL it would be reasonable to continue with this patch as 
it extends in a straightforward way existing functionality with the same 
experimental quality.

I would quite like to find one extra reviewer from a different vendor 
preferably with OpenCL 3 expertise.  @azabaznov would you be happy to review 
this?

I would also like to recommend using an experimental test 
(https://reviews.llvm.org/D97869) that is planned for the Tablegen header 
solution before committing this patch to gain more confidence. However, It 
might be good to check with @svenvh  first to see whether there are any know 
issues for running such test with `opencl-c.h`.

I would also like to loop @yaxunl  in for the extra pair of eyes wrt prior to 
OpenCL 3 functionality.

Ideally, it would be good if we don't commit it too close to the release branch 
i.e. good to leave a few weeks for the bugs to be discovered and fixed 
considering the possible propagation time to the external projects.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92004/new/

https://reviews.llvm.org/D92004

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92004: [OpenCL] add CL 3.0 optional feature support to opencl-c.h

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

FYI the testing of headers functionality is now being discussed in 
https://reviews.llvm.org/D97869


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92004/new/

https://reviews.llvm.org/D92004

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92004: [OpenCL] add CL 3.0 optional feature support to opencl-c.h

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In D92004#2423441 , @svenvh wrote:

>>> We probably don't want to hold up this patch until we have more thorough 
>>> testing in place, since we don't even have any concrete plans for such 
>>> testing at the moment.
>>
>> Well accepting such a significant change that implements the entire 
>> functionality of a new standard without a single test doesn't sound a 
>> workable approach. We should at least add some amount of testing in existing 
>> lib/Headers/opencl-c.h. However, instead of fighting with ad-hoc testing, it 
>> might save us time if we agree on a strategy and just do the testing 
>> properly. After all it is relatively clear what is to be tested. The issue 
>> is mainly with the testing time and amount of overloads to be tested. It 
>> should not be too difficult to guard such testing by a cmake option to avoid 
>> long execution time of default testing target of clang. However, it might be 
>> time-consuming to list all overloads. If we could use C++ templates it would 
>> help.
>
> Perhaps my assumption that we do not want to hold up this patch until we have 
> more thorough testing in place was wrong then.  I agree it would be good to 
> have better header testing of the header.  I merely wanted to point out that 
> testing the header thoroughly is a substantial piece of work.  So if we want 
> to have such testing in place first, we need to delay this patch and start a 
> thread outside of this review.

Yes, this is a substantial amount of work indeed and yes a separate discussion 
would make sense. One way to approach this could be to start by testing the 
functionality affected by this patch only. Then we wouldn't necessarily need to 
hold off until we have the full testing which would take a while. We could 
split the features being added here into separate patches with tests being 
added accordingly to each of them.  I also think it will simplify the reviewing 
process and allow the key functionality to be added earlier instead of waiting 
for everything to be ready at once.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92004/new/

https://reviews.llvm.org/D92004

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92004: [OpenCL] add CL 3.0 optional feature support to opencl-c.h

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh added a comment.

>> We probably don't want to hold up this patch until we have more thorough 
>> testing in place, since we don't even have any concrete plans for such 
>> testing at the moment.
>
> Well accepting such a significant change that implements the entire 
> functionality of a new standard without a single test doesn't sound a 
> workable approach. We should at least add some amount of testing in existing 
> lib/Headers/opencl-c.h. However, instead of fighting with ad-hoc testing, it 
> might save us time if we agree on a strategy and just do the testing 
> properly. After all it is relatively clear what is to be tested. The issue is 
> mainly with the testing time and amount of overloads to be tested. It should 
> not be too difficult to guard such testing by a cmake option to avoid long 
> execution time of default testing target of clang. However, it might be 
> time-consuming to list all overloads. If we could use C++ templates it would 
> help.

Perhaps my assumption that we do not want to hold up this patch until we have 
more thorough testing in place was wrong then.  I agree it would be good to 
have better header testing of the header.  I merely wanted to point out that 
testing the header thoroughly is a substantial piece of work.  So if we want to 
have such testing in place first, we need to delay this patch and start a 
thread outside of this review.

>> One idea for getting some confidence of not breaking OpenCL 2.0 too much, is 
>> to remove the `-fdeclare-opencl-builtins` flags from the SPIR-V LLVM 
>> translator  test 
>> suite and then run those tests to exercise `opencl-c.h` a bit.
>
> Ok, I think it is generally acceptable. LLVM testing is already very 
> heterogeneous. Would it mean we can test clang commits with such test 
> regularly or would it be done once on this patch only?

This idea would be a one-off test only, done locally.  The aim is to increase 
confidence in the patch, and only that.  This idea does not describe any form 
of regular testing, nor anything close to cover the entire header.  I don't see 
a trivial way of setting this up for regular testing, as the various translator 
tests should keep the `-fdeclare-opencl-builtins` flag for speed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92004/new/

https://reviews.llvm.org/D92004

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92004: [OpenCL] add CL 3.0 optional feature support to opencl-c.h

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In D92004#2420121 , @svenvh wrote:

>> I am still unclear what should we do with testing though.
>
> We probably don't want to hold up this patch until we have more thorough 
> testing in place, since we don't even have any concrete plans for such 
> testing at the moment.

Well accepting such a significant change that implements the entire 
functionality of a new standard without a single test doesn't sound a workable 
approach. We should at least add some amount of testing in existing 
lib/Headers/opencl-c.h. However, instead of fighting with ad-hoc testing, it 
might save us time if we agree on a strategy and just do the testing properly. 
After all it is relatively clear what is to be tested. The issue is mainly with 
the testing time and amount of overloads to be tested. It should not be too 
difficult to guard such testing by a cmake option to avoid long execution time 
of default testing target of clang. However, it might be time-consuming to list 
all overloads. If we could use C++ templates it would help.

> Perhaps we should just land the patch once it's ready and if any problems are 
> reported then we can always temporarily revert?

I am not sure what do you mean by revert temporarily. What if other commits lie 
on top or depend on this change. Not to say that other repositories might start 
building functionality on top e.g. SPIRV-LLVM Translator or clspv. I can 
imagine how this could become a burden for the community. From my experience 
reverting only works if done within a few days after the commit, then it 
becomes much harder if not impossible. Another aspect we should consider - how 
it can impact releasing Clang. What if someone discovers the bug in this header 
when the release is in preparation? We won't even be able to test the fix 
adequately especially within releasing time bounds. Or even worst what if  a 
release is out with that bug? Will it be expected from us that a new release 
will be created with a fix?

It becomes evident that this is no longer experimental functionality and 
therefore saving on testing will sooner or later lead to a maintenance 
nightmare. I am not saying that this patch introduced the problems but it just 
highlights that functionality here is being used and released in products and 
not in the experimental phase anymore.

> One idea for getting some confidence of not breaking OpenCL 2.0 too much, is 
> to remove the `-fdeclare-opencl-builtins` flags from the SPIR-V LLVM 
> translator  test 
> suite and then run those tests to exercise `opencl-c.h` a bit.

Ok, I think it is generally acceptable. LLVM testing is already very 
heterogeneous. Would it mean we can test clang commits with such test regularly 
or would it be done once on this patch only? I anticipate it is likely we will 
have follow up fix ups.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92004/new/

https://reviews.llvm.org/D92004

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92004: [OpenCL] add CL 3.0 optional feature support to opencl-c.h

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh added a comment.

> I am still unclear what should we do with testing though.

We probably don't want to hold up this patch until we have more thorough 
testing in place, since we don't even have any concrete plans for such testing 
at the moment.  Perhaps we should just land the patch once it's ready and if 
any problems are reported then we can always temporarily revert?

One idea for getting some confidence of not breaking OpenCL 2.0 too much, is to 
remove the `-fdeclare-opencl-builtins` flags from the SPIR-V LLVM translator 
 test suite and then 
run those tests to exercise `opencl-c.h` a bit.




Comment at: clang/lib/Headers/opencl-c-base.h:284
   memory_scope_work_group = __OPENCL_MEMORY_SCOPE_WORK_GROUP,
+#if defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ >= 
CL_VERSION_2_0) || (__OPENCL_C_VERSION__ >= CL_VERSION_3_0 && 
defined(__opencl_c_atomic_scope_device))
   memory_scope_device = __OPENCL_MEMORY_SCOPE_DEVICE,

Anastasia wrote:
> I feel `__OPENCL_C_VERSION__ >= CL_VERSION_3_0` is redundant. Perhaps we 
> don't need `__OPENCL_C_VERSION__ >= CL_VERSION_2_0` either if we define the 
> feature macros above. Btw @azabaznov  is planning to define those macros in 
> this header in https://reviews.llvm.org/D89869#change-kc4kXsHko6uZ. I guess 
> some rebase will be necessary at some point depending on which commit will be 
> ready first.
I also think it makes more sense to define the feature macros in 
`opencl-c-base.h`, unless there is a reason that I missed?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92004/new/

https://reviews.llvm.org/D92004

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92004: [OpenCL] add CL 3.0 optional feature support to opencl-c.h

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

Thanks! This looks much cleaner.

I am still unclear what should we do with testing though.




Comment at: clang/lib/Headers/opencl-c-base.h:284
   memory_scope_work_group = __OPENCL_MEMORY_SCOPE_WORK_GROUP,
+#if defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ >= 
CL_VERSION_2_0) || (__OPENCL_C_VERSION__ >= CL_VERSION_3_0 && 
defined(__opencl_c_atomic_scope_device))
   memory_scope_device = __OPENCL_MEMORY_SCOPE_DEVICE,

I feel `__OPENCL_C_VERSION__ >= CL_VERSION_3_0` is redundant. Perhaps we don't 
need `__OPENCL_C_VERSION__ >= CL_VERSION_2_0` either if we define the feature 
macros above. Btw @azabaznov  is planning to define those macros in this header 
in https://reviews.llvm.org/D89869#change-kc4kXsHko6uZ. I guess some rebase 
will be necessary at some point depending on which commit will be ready first.



Comment at: clang/lib/Headers/opencl-c.h:37
 
+#if defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ >= CL_VERSION_2_0 
&& __OPENCL_C_VERSION__ < CL_VERSION_3_0)
+#ifndef __opencl_c_generic_address_space

I guess here it should be:

```
defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ == CL_VERSION_2_0)
```

There are no OpenCL C language versions between 2.0 and 3.0.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92004/new/

https://reviews.llvm.org/D92004

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92004: [OpenCL] add CL 3.0 optional feature support to opencl-c.h

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In D92004#2414692 , @airlied wrote:

> In D92004#2413560 , @Anastasia wrote:
>
>> Btw how about making some checks simpler. We could always define feature 
>> macros `__opencl_c_atomic_scope_device`, `__opencl_c_generic_address_space` 
>> for  OpenCL 2.0 or C++ for OpenCL . Then everywhere we would only need to 
>> check feature macros instead of language versions and feature macros 
>> together.
>>
>>   #if !(defined()) && (defined(__OPENCL_CPP_VERSION__) || 
>> (__OPENCL_C_VERSION__ == CL_VERSION_2_0))
>>   #define __opencl_c_atomic_scope_device  1
>>   #define __opencl_c_generic_address_space   1
>>   ...
>>   #endif
>
> I like this idea but my only worry was about leaking those definitions to the 
> user source when the system hasn't defined them.
>
> i.e. a CL2.0 user now sees __opencl_c_generic_address_space, writes code to 
> use that macro, but it fails on other OpenCL C implementations.
>
> But if we are going to have feature code that works like that then I'm fine 
> with going ahead and changing things to look like this.

Identifiers that start with a double underscore are reserved so there is no 
real harm in defining them for earlier OpenCL C versions. FYI there is this PR 
https://github.com/KhronosGroup/OpenCL-Docs/pull/477 where we intend to clarify 
that the earlier OpenCL C version might have the macro defined too.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92004/new/

https://reviews.llvm.org/D92004

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92004: [OpenCL] add CL 3.0 optional feature support to opencl-c.h

[Dave Airlie via Phabricator via cfe-commits]

airlied added a comment.

In D92004#2413560 , @Anastasia wrote:

> Btw how about making some checks simpler. We could always define feature 
> macros `__opencl_c_atomic_scope_device`, `__opencl_c_generic_address_space` 
> for  OpenCL 2.0 or C++ for OpenCL . Then everywhere we would only need to 
> check feature macros instead of language versions and feature macros together.
>
>   #if !(defined()) && (defined(__OPENCL_CPP_VERSION__) || 
> (__OPENCL_C_VERSION__ == CL_VERSION_2_0))
>   #define __opencl_c_atomic_scope_device  1
>   #define __opencl_c_generic_address_space   1
>   ...
>   #endif

I like this idea but my only worry was about leaking those definitions to the 
user source when the system hasn't defined them.

i.e. a CL2.0 user now sees __opencl_c_generic_address_space, writes code to use 
that macro, but it fails on other OpenCL C implementations.

But if we are going to have feature code that works like that then I'm fine 
with going ahead and changing things to look like this.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92004/new/

https://reviews.llvm.org/D92004

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92004: [OpenCL] add CL 3.0 optional feature support to opencl-c.h

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh added a comment.

In D92004#2413603 , @Anastasia wrote:

> @svenvh I imagine the testing functionality between the two headers will be 
> identical?

Mostly, assuming you intend to test in the conventional way of feeding a .cl 
file to Clang.  Although currently `-fdeclare-opencl-builtins` only provides a 
subset of the builtins in `opencl-c.h`.

In the past I did have a look at the possibility of automated equivalence 
checking of declarations provided by both `-fdeclare-opencl-builtins` and 
`opencl-c.h`.  It shouldn't be impossible, but it's not trivial (one reason 
being that gentype-expansion is currently done in Sema).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92004/new/

https://reviews.llvm.org/D92004

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92004: [OpenCL] add CL 3.0 optional feature support to opencl-c.h

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a subscriber: svenvh.
Anastasia added a comment.

> It will need some decent review and maybe some testing by other CL users to 
> make sure I've haven't messed up when used with a fully featured CL 3.0 stack.

Clang functionality is expected to be tested in llvm-project and with OpenCL 
headers we are violating the process to be honest. Aside from that it slows 
down the development in my opinion because making larger changes is extremely 
hard without the testing. Relying on reviews is not great for various reasons. 
We constantly fix bugs related to the headers.

The biggest issue with testing that header originally was that the parsing of 
it is so slow - with only one test we have exceeded the acceptable limits 
immediately. However, to mitigate this problem there is no reason we couldn't 
add header testing conditioned on a CMake option passed during the build 
configuration. So by default only minimal functionality could be tested as it 
is done now but if some option is being passed to cmake (say 
`ENABLE_FULL_OPENCL_BIFS_TESTS`) we could do comprehensive testing of the 
header. If there is interest in such an approach we could start from just 
adding the tests related to OpenCL 3.0 changes and then slowly build more tests 
in the future.

Another area where such tests would be needed is for Tablegen header enabled by 
`-fdeclare-opencl-builtins` that is developed to replace the regular header 
eventually because it is much quicker to parse. @svenvh I imagine the testing 
functionality between the two headers will be identical?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92004/new/

https://reviews.llvm.org/D92004

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92004: [OpenCL] add CL 3.0 optional feature support to opencl-c.h

[Anton Zabaznov via Phabricator via cfe-commits]

azabaznov added a comment.

Hi! Great to see the interest in OpenCL C 3.0!

I'm being working already at a proper feature macros definition to unify both 
clang and header OpenCL C 3.0 related changes, here is the change: 
https://reviews.llvm.org/D89869. So as this patch will be merged then the check 
for supported feature can be done uniformly in all OpenCL versions:

Use:

  #ifdef __opencl_c_pipes

instead of

  #if defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ >= 
CL_VERSION_2_0 && __OPENCL_C_VERSION__ < CL_VERSION_3_0) || 
(__OPENCL_C_VERSION__ >= CL_VERSION_3_0 && defined(__opencl_c_pipes))

The main concepts are described in RFC thread here: 
https://lists.llvm.org/pipermail/cfe-dev/2020-September/066883.html. Will be 
glad to discuss any technical details and concerns. Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92004/new/

https://reviews.llvm.org/D92004

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92004: [OpenCL] add CL 3.0 optional feature support to opencl-c.h

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In D92004#2412953 , @airlied wrote:

> this file has never been clang-format clean, happy to make changes if 
> reviewer decides they are needed.

This has not been considered a conventional clang source so to say, so the 
coding style has not been followed. But it is quite annoying now with 
formatting hooks being installed. Ideally, it would be good to format this 
properly but this makes retrieving the history harder and it is extremely 
valuable. So I suggest not to reformat the old code but for the new code I 
leave it up to you.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92004/new/

https://reviews.llvm.org/D92004

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92004: [OpenCL] add CL 3.0 optional feature support to opencl-c.h

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a subscriber: azabaznov.
Anastasia added a comment.

I assume your change depends on some other changes that add the feature macro 
definitions. So I think we should link the other changes in at some point. 
CCing to @azabaznov here who is working on the features too.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92004/new/

https://reviews.llvm.org/D92004

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92004: [OpenCL] add CL 3.0 optional feature support to opencl-c.h

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

Btw how about making some checks simpler. We could always define feature macros 
`__opencl_c_atomic_scope_device`, `__opencl_c_generic_address_space` for  
OpenCL 2.0 or C++ for OpenCL . Then everywhere we would only need to check 
feature macros instead of language versions and feature macros together.

  #if !(defined()) && (defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ 
== CL_VERSION_2_0))
  #define __opencl_c_atomic_scope_device  1
  #define __opencl_c_generic_address_space   1
  ...
  #endif


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92004/new/

https://reviews.llvm.org/D92004

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92004: [OpenCL] add CL 3.0 optional feature support to opencl-c.h

[Dave Airlie via Phabricator via cfe-commits]

airlied added a comment.

this file has never been clang-format clean, happy to make changes if reviewer 
decides they are needed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92004/new/

https://reviews.llvm.org/D92004

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits