This feature requires support of opencl_c_generic_address_space and
opencl_c_program_scope_global_variables so diagnostics for that is provided as well.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
This feature requires support of opencl_c_generic_address_space and
opencl_c_program_scope_global_variables so diagnostics for that is provided as well.
Do we have a spec issue for it or a PR? It would be good to list it here since it is not yet published in the registry.
clang/lib/Sema/Sema.cpp | ||
---|---|---|
337 | Here it should be sufficient to just check getLangOpts().Blocks only wothout the versions. | |
clang/test/Misc/opencl-c-3.0.incorrect_options.cl | ||
0–1 | I can't remember if we have discussed this already, but could we use -verify for these errors? | |
clang/test/SemaOpenCL/invalid-device-enqueue-types-cl3.0.cl | ||
6 | I know that many test have prefix "invalid" but I feel we have failed to establish the meaning for it because most of the tests in Sema are testing some sort of invalid behavior. But also here I feel that we should test that enqueue_kernel is not supported? Do you think we chould merge this testing together with SemaOpenCL/cl20-device-side-enqueue.cl with some filename renaming? Technically we should do the same testing even for CL1.x versions... | |
clang/test/SemaOpenCL/storageclass.cl | ||
1–2 | These lines are getting a bit longer. Do we actually need to set -__opencl_c_device_enqueue for this test? Same for some other tests... |
Simplified running lines in tests, use 'verify' when validating features, simplify condition in Sema
clang/test/SemaOpenCL/invalid-device-enqueue-types-cl3.0.cl | ||
---|---|---|
6 | I think it can. But enqueu_kernel is a LangBuiltin and still not supported yet. Support for LangBuiltins is going to be added in a separate patch: with this patch all of the features that affect language built-ins are supported. |
clang/test/Misc/opencl-c-3.0.incorrect_options.cl | ||
---|---|---|
0–1 | We should be able to remove FileCheck and replace CHECK directives with something like: | |
clang/test/SemaOpenCL/invalid-device-enqueue-types-cl3.0.cl | ||
6 | Ok, then would it still work if we merge this functionality SemaOpenCL/cl20-device-side-enqueue.cl and the rest can be added later on... | |
clang/test/SemaOpenCL/storageclass.cl | ||
1–2 | ping |
clang/test/Misc/opencl-c-3.0.incorrect_options.cl | ||
---|---|---|
0–1 | I don't think we can test it like this because there is different output for each invalid combination, so we need to check them with label. | |
clang/test/SemaOpenCL/invalid-device-enqueue-types-cl3.0.cl | ||
6 | It's difficult to refactor that test, since it relies on the fact that device enqueue feature is supported and checks for incorrect constructs. We can't enable it for 3.0 now since language built-ins (enqueue_kernel etc.) are not supported yet. | |
clang/test/SemaOpenCL/storageclass.cl | ||
1–2 | Yeah, we need that here because we are turning off generic AS and PSV in this test. Note that __opencl_c_device_enqueue is turned off with -cl-ext=-all. |
clang/test/Misc/opencl-c-3.0.incorrect_options.cl | ||
---|---|---|
0–1 | We normally use macros to guard the expected errors but however, this is a better fit for a refactoring patch... so I am ok if it's done separately... However, don't you need to add //expected-no-diagnostics directive? | |
clang/test/SemaOpenCL/storageclass.cl | ||
1–2 | since this test doesn't check anything for __opencl_c_device_enqueue why do we need to switch this off explicitly? |
clang/test/SemaOpenCL/storageclass.cl | ||
---|---|---|
1–2 | In this test PSV and generic address space features are alternately turned off. Since they are required to by device enqueue, it's required to turn device enqueue as well. |
Here it should be sufficient to just check getLangOpts().Blocks only wothout the versions.