Anastasia added inline comments. ================ Comment at: test/SemaOpenCL/extension-version.cl:11 @@ +10,3 @@ +#endif +#pragma OPENCL EXTENSION cl_clang_storage_class_specifiers: enable + ---------------- jvesely wrote: > Anastasia wrote: > > jvesely wrote: > > > Anastasia wrote: > > > > Could you use standard diagnostic check please: > > > > expected-warning{{unknown OpenCL extension ... > > > > > > > > Similarly to SemaOpenCL/extensions.cl > > > not sure I follow, the test does not trigger any diagnostics (by design). > > > are you saying that I should introduce negative checks to make sure > > > extensions are not available outside of their respective context? > > > Is there a way to filter verifier tags based on clang invocation? > > > (something like FileCheck prefix) > > Exactly, you should check that the extensions are enabled correctly based > > on CL versions. > > > > For example if you compile this without passing -cl-std=CL1.2: > > #pragma OPENCL EXTENSION cl_khr_gl_msaa_sharing: enable > > the following error is produced: > > unsupported OpenCL extension 'cl_khr_gl_msaa_sharing' - ignoring > > > > You can condition error directives on CL version passed as it's done in the > > example test SemaOpenCL/extensions.cl. > > > > So what is the original intension of this tests? Not sure I understand what > > you are trying to test. > it's a positive test that checks that extensions are available (both that the > define is present, and that #pragma passes without error). > > I did not include negative tests (check that extension is not available > outside of its respective context), because I think it's a bit overzealous > reading of the specs. > For example cl_khr_d3d10_sharing is first mentioned in OpenCL 1.1 specs, but > the text of the extension says that it is written against OpenCL 1.0.48 spec. > (I moved cl_khr_icd to 1.0 for the same reason). I think if a vendor can add > vendor specific extensions to the list of supported extensions, it should be > possible to add extensions from higher CL versions. > > similarly, I would argue against warnings for extensions promoted to core > features (or at least hide the warning behind -pedantic). they are listed in > CL_DEVICE_EXTENSIONS for backwards compatibility so I'd say it is OK to allow > pragmas in higher CLC versions for backward compatibility. I agree with this: "similarly, I would argue against warnings for extensions promoted to core features (or at least hide the warning behind -pedantic). they are listed in CL_DEVICE_EXTENSIONS for backwards compatibility so I'd say it is OK to allow pragmas in higher CLC versions for backward compatibility."
@yaxunl, what's your opinion here? Regarding the test, I think we should still check the diagnostics being given correctly especially for the extensions unavailable in the earlier versions. It should be quite straight forward to extend this test. Repository: rL LLVM http://reviews.llvm.org/D20447 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits