OpenCL-Docs icon indicating copy to clipboard operation
OpenCL-Docs copied to clipboard

object in atomic_load should be declared as const

Open ltowarek opened this issue 5 years ago • 5 comments
trafficstars

OpenCL C atomic_load declaration does not contain const qualifier. As a result below kernel can't be compiled.

kernel void test(global const atomic_int* a, global int* b)
{
    b[0] = atomic_load(a);
}

C, C++ and OpenCL C++ specifications contain const qualifier, so it seems natural that OpenCL C should also follow this pattern:

  1. https://en.cppreference.com/w/c/atomic/atomic_load
  2. https://en.cppreference.com/w/cpp/atomic/atomic_load
  3. https://www.khronos.org/registry/OpenCL/specs/2.2/html/OpenCL_Cxx.html#atomic-operations-library

ltowarek avatar Nov 05 '20 10:11 ltowarek

Can we call this a bugfix and just add the const qualifier to the atomic_load functions?

I think that was almost certainly the original intent, although the const qualifier has been missing all the way back when atomic_load was added in OpenCL C 2.0.

If we fix this it's possible that code compiled against newer drivers with the const qualifier will fail on older (conformant!) drivers that did not include the const qualifier, but code that worked with older drivers without the const qualifier will continue to work with newer drivers that have the const qualifier.

bashbaug avatar Nov 06 '20 21:11 bashbaug

If we fix this it's possible that code compiled against newer drivers with the const qualifier will fail on older (conformant!) drivers that did not include the const qualifier, but code that worked with older drivers without the const qualifier will continue to work with newer drivers that have the const qualifier.

Agreed. We have a number of such language bugs that are stalled due to this problem. We had once agreed to solve this using compiler extnsions however I still feel small bugfixes like this don't seem to be a great fit for this conceptually plus they are technically not part of spec document so I am not sure how known those extension would be for the developers. New spec versions are perhaps a good opportunities to address those.

AnastasiaStulova avatar Nov 11 '20 11:11 AnastasiaStulova

If we applied this as a bug fix, it would mean that existing implementations that lack the const qualifier would not conform to the specification. No existing working OpenCL C code will expect a const qualifier (unless it's non-portable), but new code relying on the const qualifier would not work on old implementations. I agree that this is worth fixing, and in my view, it would be appropriate for implementations to add the const qualifier in preparation for this, as it would not break any existing OpenCL C code.

StuartDBrady avatar Dec 08 '20 17:12 StuartDBrady

For OpenCL 3.0 at least it might still be feasible to fix the implementations, but I am not sure we can force updating all OpenCL 2.0 drivers that exist.

AnastasiaStulova avatar Dec 08 '20 17:12 AnastasiaStulova

We have a number of such language bugs that are stalled due to this problem.

If we applied this as a bug fix, it would mean that existing implementations [...] would not conform to the specification.

As indeed this pattern maybe the answer is to take the same approach for them all, possible ideas:

  1. Make the change and accept that new code may not word on older implementations. A stronger variation of this is to CTS test the new behaviour to ensure that in development code-bases pick it up -- or maybe for only 3.0 onwards as Anastasia says.

  2. Have a list of changes to be made in the next specification major bump (i.e. not a maintenance release), this is safest, but very slow.

  3. Have a language super-extension that gathers together all language fixes such as this. Would make it clear to programmers that their opting in to a new thing. I suspect these changes are too trivial for anyone to bother though.

alycm avatar Feb 25 '21 14:02 alycm