[PATCH] D97058: [OpenCL] Refactor diagnostic for OpenCL extension/feature

azabaznov added a comment. > Do you also get fatal error: 'opencl-c-base.h' file not found? If so, we > might need at least to file a clang bug that we should look at before the > next release. I'm able to reproduce it when not setting `-target` option explicitly: $ ./build_release/bin/c-ind

[PATCH] D97058: [OpenCL] Refactor diagnostic for OpenCL extension/feature

Anastasia added a comment. In D97058#2623160 , @thakis wrote: > I'm going to revert this in 30 minutes. The tree has been red for 12h due to > this then. I have committed the fix mentioned above. Apology for the delay. Repository: rG LLVM Github Mono

[PATCH] D97058: [OpenCL] Refactor diagnostic for OpenCL extension/feature

thakis added a comment. I'm going to revert this in 30 minutes. The tree has been red for 12h due to this then. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97058/new/ https://reviews.llvm.org/D97058 _

[PATCH] D97058: [OpenCL] Refactor diagnostic for OpenCL extension/feature

Anastasia added a comment. In D97058#2622883 , @azabaznov wrote: > Yes, I was able to reproduce the failure with `-target arm64-apple-macosx` > flag, I provided the fix to set explicit target: > https://reviews.llvm.org/D98539. I have approved it. Thank

[PATCH] D97058: [OpenCL] Refactor diagnostic for OpenCL extension/feature

azabaznov added a comment. Yes, I was able to reproduce the failure with `-target arm64-apple-macosx` flag, I provided the fix to set explicit target: https://reviews.llvm.org/D98539. @Anastasia, I tried to cherry-pick https://reviews.llvm.org/D92244, but the error is still reproducible. Rep

[PATCH] D97058: [OpenCL] Refactor diagnostic for OpenCL extension/feature

Anastasia added a comment. I can't find why and how `opencl-c-base.h` is used for the test. But it feels it is adding the random noise to the testing as it is not expected to be used. I don't feel that this commit is affecting the header though, so it is something that might have already been b

[PATCH] D97058: [OpenCL] Refactor diagnostic for OpenCL extension/feature

Anastasia added a comment. In D97058#2622750 , @Anastasia wrote: > In D97058#2622717 , @thakis wrote: > >> In case it's useful, here's the output of the RUN line on an m1 mac with >> this patch locally reverted (wh

[PATCH] D97058: [OpenCL] Refactor diagnostic for OpenCL extension/feature

Anastasia added a comment. In D97058#2622717 , @thakis wrote: > In case it's useful, here's the output of the RUN line on an m1 mac with this > patch locally reverted (where the test passes): > http://pastie.org/p/2F3Y0xUMtH5RP9TVRzG4LI Thanks, this see

[PATCH] D97058: [OpenCL] Refactor diagnostic for OpenCL extension/feature

thakis added a comment. In case it's useful, here's the output of the RUN line on an m1 mac with this patch locally reverted (where the test passes): http://pastie.org/p/2F3Y0xUMtH5RP9TVRzG4LI Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97058/ne

[PATCH] D97058: [OpenCL] Refactor diagnostic for OpenCL extension/feature

Anastasia added a comment. > (The `fatal error: 'opencl-c-base.h' file not found` bit is printed when the > test passes too, and is unrelated.) I can't reproduce this locally neither with nor without this commit. I think there is something wrong with the way this test runs, this might explai

[PATCH] D97058: [OpenCL] Refactor diagnostic for OpenCL extension/feature

azabaznov added a comment. Thanks guys, I'm on way to trying that, just building clang from scratch. >   If it is, then just adding a fixed triple is fine. Yeah, it is expected as this change removes types which require extension from parsers state. X86 and SPIR support all extensions by defau

[PATCH] D97058: [OpenCL] Refactor diagnostic for OpenCL extension/feature

Anastasia added a comment. In D97058#2622681 , @svenvh wrote: > In D97058#2622669 , @thakis wrote: > >> Does it repro if you add `-target arm64-apple-macosx` as arg to c-index-test >> on the RUN line of that test?

[PATCH] D97058: [OpenCL] Refactor diagnostic for OpenCL extension/feature

thakis added a comment. It repros for me with this local diff % git diff diff --git a/clang/test/Index/opencl-types.cl b/clang/test/Index/opencl-types.cl index 496f38752fa2..13f6058864b5 100644 --- a/clang/test/Index/opencl-types.cl +++ b/clang/test/Index/opencl-types.cl @@ -1,4 +1,4

[PATCH] D97058: [OpenCL] Refactor diagnostic for OpenCL extension/feature

svenvh added a comment. In D97058#2622669 , @thakis wrote: > Does it repro if you add `-target arm64-apple-macosx` as arg to c-index-test > on the RUN line of that test? I also had trouble reproducing the failure locally, but by specifying this target I

[PATCH] D97058: [OpenCL] Refactor diagnostic for OpenCL extension/feature

thakis added a comment. Does it repro if you add `-target arm64-apple-macosx` as arg to c-index-test on the RUN line of that test? Here's the output of that RUN line on an arm mac when the test fails: http://pastie.org/p/1go1Y9WXjs5T371RtyJ3Gi Let's revert for now, things have been broken for

[PATCH] D97058: [OpenCL] Refactor diagnostic for OpenCL extension/feature

azabaznov added a comment. Hi! Yeah,  I'm looking into it, but I can't reproduce it locally: the test passes at x86_64 linux system. I'll revert the change if it takes too much time to investigate what's going on. Thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION http

[PATCH] D97058: [OpenCL] Refactor diagnostic for OpenCL extension/feature

thakis added a comment. Hi, this breaks check-clang on arm macs: http://45.33.8.238/macm1/5421/step_6.txt (The `fatal error: 'opencl-c-base.h' file not found` bit is printed when the test passes too, and is unrelated.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:/

[PATCH] D97058: [OpenCL] Refactor diagnostic for OpenCL extension/feature

RKSimon added a comment. @azabaznov This has broken some buildbots http://lab.llvm.org:8011/#/builders/105/builds/6855 - can you take a look please? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97058/new/ https://reviews.llvm.org/D97058 ___

[PATCH] D97058: [OpenCL] Refactor diagnostic for OpenCL extension/feature

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG840643bbe1d2: [OpenCL] Refactor diagnostic for OpenCL extension/feature (authored by azabaznov). Changed prior to commit: https://reviews.llvm.org

[PATCH] D97058: [OpenCL] Refactor diagnostic for OpenCL extension/feature

azabaznov added a comment. > Ok, addressing in a separate patch is reasonable, but why do you think that > we will break backward compatibility? My current worry is that the > implementation is so messy and inconsistent that it will take us longer time > if we do the incremental steps. Also, we

[PATCH] D97058: [OpenCL] Refactor diagnostic for OpenCL extension/feature

Anastasia accepted this revision. Anastasia added a comment. This revision is now accepted and ready to land. LGTM! Although some further improvements seem to be necessary they can be done separately. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D9

[PATCH] D97058: [OpenCL] Refactor diagnostic for OpenCL extension/feature

Anastasia added a comment. In D97058#2615926 , @azabaznov wrote: > Corrected some mistakes, added a test for diagnosing undeclared identifiers > when an extension is unsupported. Generally leaving the change as it is as > completely removing pragma may b

[PATCH] D97058: [OpenCL] Refactor diagnostic for OpenCL extension/feature

azabaznov updated this revision to Diff 329560. azabaznov added a comment. Replaced atomic_double implicit definition Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97058/new/ https://reviews.llvm.org/D97058 Files: clang/include/clang/Basic/Diagn

[PATCH] D97058: [OpenCL] Refactor diagnostic for OpenCL extension/feature

azabaznov updated this revision to Diff 329554. azabaznov added a comment. Corrected some mistakes, added a test for diagnosing undeclared identifiers when a extension is unsupported. Generally leaving the change as it is as completely removing pragma may break backward compatibility now: let's

[PATCH] D97058: [OpenCL] Refactor diagnostic for OpenCL extension/feature

Anastasia added inline comments. Comment at: clang/lib/Sema/Sema.cpp:364 for (auto &I : Atomic64BitTypes) setOpenCLExtensionForType(I, "cl_khr_int64_base_atomics cl_khr_int64_extended_atomics"); azabaznov wrote: > Anastasia wrote: > >

[PATCH] D97058: [OpenCL] Refactor diagnostic for OpenCL extension/feature

azabaznov added inline comments. Comment at: clang/lib/Sema/Sema.cpp:364 for (auto &I : Atomic64BitTypes) setOpenCLExtensionForType(I, "cl_khr_int64_base_atomics cl_khr_int64_extended_atomics"); Anastasia wrote: > I think this should

[PATCH] D97058: [OpenCL] Refactor diagnostic for OpenCL extension/feature

Anastasia added a comment. In D97058#2600160 , @azabaznov wrote: > Check 'isEnabled' is now private: it is used only for non-core or > non-optional core features; > creation of implicit type definitions is guarder with extension support > check; minor re

[PATCH] D97058: [OpenCL] Refactor diagnostic for OpenCL extension/feature

azabaznov added inline comments. Comment at: clang/lib/Basic/OpenCLOptions.cpp:22-26 + auto &OptInfo = OptMap.find(Ext)->getValue(); + if (OptInfo.isCoreIn(LO) || OptInfo.isOptionalCoreIn(LO)) +return isSupported(Ext, LO); + + return isEnabled(Ext); Note t

[PATCH] D97058: [OpenCL] Refactor diagnostic for OpenCL extension/feature

azabaznov updated this revision to Diff 327767. azabaznov added a comment. Check 'isEnabled' is now private: it is used only for non-core or non-optional core features; creation of implicit type definitions is guarder with extension support check; minor refactoring Repository: rG LLVM Github

[PATCH] D97058: [OpenCL] Refactor diagnostic for OpenCL extension/feature

Anastasia added a comment. In D97058#2587033 , @azabaznov wrote: >> FYI, I would even be ok if we remove the need for enabling non-core > > AFAIU this can be done not for every extension to maintain backward > compatibility. This should not result in any

[PATCH] D97058: [OpenCL] Refactor diagnostic for OpenCL extension/feature

azabaznov added a comment. > FYI, I would even be ok if we remove the need for enabling non-core AFAIU this can be done not for every extension to maintain backward compatibility. In this case I think https://reviews.llvm.org/D97052 can be useful. Also, I imagine that implicit type definition

[PATCH] D97058: [OpenCL] Refactor diagnostic for OpenCL extension/feature

Anastasia added a comment. FYI, I would even be ok if we remove the need for enabling non-core features too but it is also fine to start from just core features. Thanks! Comment at: clang/lib/Sema/Sema.cpp:360 setOpenCLExtensionForType(Context.DoubleTy, "cl_khr_fp64");

[PATCH] D97058: [OpenCL] Refactor diagnostic for OpenCL extension/feature

azabaznov created this revision. azabaznov added reviewers: Anastasia, svenvh. Herald added subscribers: jfb, yaxunl. azabaznov requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. There is no need to check for enabled pragma for core or optiona