In D142033#4062671, @bader wrote:

Should we generalize and rename clang/test/CodeGenOpenCL/convergent.cl to validate function attributes other than convergent? It's not obvious that presence of nounwind attribute is validated by clang/test/CodeGenOpenCL/convergent.cl.

In D141297#4043122, @Anastasia wrote:

Btw I wonder if in the future we could add some error or warning in case someone uses the same approach for frontend specific features, i.e.

#ifdef __undef___opencl_c_generic_address_space
#error "Feature __opencl_c_generic_address_space can only be disabled via -cl-ext flag"
#endif

it may be more appropriate to make these triggered off of a hidden option defaulted to off for now, or maybe based on whether or not opaque pointers are enabled

@svenvh I remember that we have also discussed the addition of a vendor specific header where such feature/extension macro definition can be added to avoid the macro pollution but I feel this is somewhat orthogonal i.e. the fine grained control of macro defines is still needed?

I have attempted to workaround this issue for the reported test cases, however it still doesn't quite work when the type is created from the template parameter (see FIXME test case in the patch). I presume we want to allow this? If so we might need to disable lazy template instantiation in this case. My guess is the only issue this this is that we will have performance penalty for the code of this format:

LGTM, thanks!

In D132473#3742688, @Anastasia wrote:

@svenvh as you made quite a lot of changes in the headers feel free to expand the description if you feel we should document some of those in more detail.

LGTM.

Added FIXME and updated CHECK lines to avoid CHECK-NOT.

In D126660#3550356, @Anastasia wrote:

Ok, makes sense! Thanks!

Btw I was thinking we should provide some way for developers to know what extensions are being supported either through documentation or by querying clang somehow? I am guessing documentation would be easier to implement but harder to keep in sync?

LGTM

Add test case.

In D125387#3508744, @arkangath wrote:

Please note that I do not have commit rights.

In D124256#3474043, @Anastasia wrote:

LGTM! I imagine tablegen side is being tested automatically?

Added macro and macro test. Added reference to Extension spec section.

LGTM; just a few minor suggestions that you can address at commit time.

I've submitted the fix in 4dfec37037f5.

In D120254#3419221, @hvdijk wrote:

This was worked around by modifying tests, but I believe this is a fundamental problem in this change and was able to reproduce the error with plain old clang:

$ cat test.cl
void sub_group_barrier();

$ bin/clang -cl-std=CL1.2 -S -o - test.cl
error: enum type memory_scope not found; include the base header with -finclude-default-header
1 error generated.

$ bin/clang --version
clang version 15.0.0 (git@github.com:llvm/llvm-project c204cee642ee794901d2e8a9819b52ac12f92bc9)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /home/harald/llvm-project/build/bin

The problem is that this change enables certain built-ins in OpenCL 1.2 that take a memory_scope argument, but the memory_scope type is not defined in OpenCL 1.2 mode. When we then process the function, sub_group_barrier in my example, things break when checking whether the declaration matches the built-in. I am not sure what the right fix here is. Can we just define the type if any extension is enabled that requires the type, or is that not allowed?

Rebased on latest main.

Since this is a simple test update I'll commit this now (before code review), to get affected CI back to green. Please let me know if there are any post-commit concerns.

After a bit of digging I realized we don't need the explicit include at all anymore.

Thanks, I could reproduce the problem with your cmake line. I have uploaded a fix for review in https://reviews.llvm.org/D120470

In D120254#3342551, @dyung wrote:

Hi, our internal release build bots are showing failures in two clang-tidy tests that I bisected back to your commit, clang-tidy/checkers/altera-id-dependent-backward-branch.cpp and clang-tidy/checkers/altera-single-work-item-barrier.cpp. After this change, both are exhibiting this error:

Error while processing /home/dyung/src/upstream/aa9c2d19d9b73589d72114d6e0a4fb4ce42b922b-linux/tools/clang/tools/extra/test/clang-tidy/checkers/Output/altera-single-work-item-barrier.cpp.tmp.cpp.
error: enum type memory_scope not found; include the base header with -finclude-default-header [clang-diagnostic-error]

Oddly, this only fails in a release configuration. Can you take a look?

Use StringRef and extend comment.