yaxunl 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:
> > > 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.
The warning is a reminder that this is no longer an extension and the user
should remove that. However I do not have strong opinion on that.
Repository:
rL LLVM
http://reviews.llvm.org/D20447
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
