jvesely added inline comments.

================
Comment at: test/SemaOpenCL/extension-version.cl:11
@@ +10,3 @@
+#endif
+#pragma OPENCL EXTENSION cl_clang_storage_class_specifiers: enable
+
----------------
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.


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

Reply via email to