Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
It seems like there is no test covering this?
Could you add a test in SemaOpenCL checking this code i.e. extension is allowed in CL>1.2 and disallowed if CL<1.2?
Thanks!
| test/SemaOpenCL/extension-version.cl | ||
|---|---|---|
| 12 | Could you use standard diagnostic check please: expected-warning{{unknown OpenCL extension ...Similarly to SemaOpenCL/extensions.cl | |
| test/SemaOpenCL/extension-version.cl | ||
|---|---|---|
| 12 | not sure I follow, the test does not trigger any diagnostics (by design). | |
| test/SemaOpenCL/extension-version.cl | ||
|---|---|---|
| 12 | 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. | |
| test/SemaOpenCL/extension-version.cl | ||
|---|---|---|
| 12 | 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. 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. | |
| test/SemaOpenCL/extension-version.cl | ||
|---|---|---|
| 12 | 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. | |
| test/SemaOpenCL/extension-version.cl | ||
|---|---|---|
| 12 | 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. | |
| test/SemaOpenCL/extension-version.cl | ||
|---|---|---|
| 13 | When the AMD compiler added these warnings a long time ago I found them obnoxious and required adding more complicated ifdef combinations to silence them | |
| test/SemaOpenCL/extension-version.cl | ||
|---|---|---|
| 42 | Actually looking at other comments -> Core | |
| test/SemaOpenCL/extension-version.cl | ||
|---|---|---|
| 74 | Ok, no problem. Let's just leave it as is. I guess the other change will be committed soon. :) | |
Could you use standard diagnostic check please:
expected-warning{{unknown OpenCL extension ...Similarly to SemaOpenCL/extensions.cl