KhronosGroup / OpenCL-Docs

OpenCL API, OpenCL C, Extensions, SPIR-V Environment Specs, Ref page, and C++ for OpenCL doc sources.
Other
359 stars 113 forks source link

Prevent clashing with user identifiers in extensions and future kernel language versions #547

Open AnastasiaStulova opened 3 years ago

AnastasiaStulova commented 3 years ago

In OpenCL C new identifiers are being added through extensions and new language versions even if they have not been reserved, an example is sub_group_* functions in OpenCL 3.0. While the clashes are unlikely this still leaves concerns from the developers regarding the actual backward compatibility of the new standards.

Technically if an old kernel source is not guaranteed to be compiled by the conformant compiler for a new OpenCL standard we can't claim backward compatibility.

I was wondering if the cl_ prefix was supposed to be used for that purpose but it hasn't been used consistently. Many vendor extensions solved the issue by adding a vendor prefix to the identifiers e.g. arm_, intel, etc.

Should something be done for the future common extensions or even standard?

keryell commented 3 years ago

You could adapt the way to add new features in newer versions of C.

For example having all the OpenCL 3 extensions or keywords named with a user-forbidden prefix like _CL_ext1 or __cl_ext2, equivalent of C _Bool, _Complex...

A real OpenCL 3 kernel would have a #include <opencl3.h> that would map _CL_ext1 to ext1 like in C you have _Complex mapped also to complex when #include <complex.h>.

bashbaug commented 3 years ago

I think the original idea was that the identifiers added by extensions would only become available when the extension was enabled using the corresponding pragma, but as we have discovered this didn't work out particularly well in practice (see e.g. https://github.com/KhronosGroup/OpenCL-Docs/issues/82).

IIRC the vendor extension prefix was designed to anticipate that either a) multiple vendors may want to support similar functionality without name conflicts, and b) a feature may change slightly during the standardization process. It wasn't done necessarily to prevent naming conflicts with user code, though that turned out to be a nice side benefit.

I'm not sure how much of this we can retrofit into an existing version of the standard, but I think this would be a great improvement in a future version.

keryell commented 3 years ago

The important part in the way it is done in C is that it is forbidden for normal user code to use identifiers starting with _ + uppercase letter or starting with double _, so this is where are defined all the new features. It prevents conflict with conformant C/C++ user code. I propose to specialize this approach by combining the C standard way + some OpenCL infix.

AnastasiaStulova commented 3 years ago

Summary from the teleconference, Feb 11, 2021

Two approaches have been discussed:

However, it is likely that such significant changes are to be added in the next versions. More trade offs are to be explored:

AnastasiaStulova commented 3 years ago

Regarding the two approaches highlighted earlier I would like to clarify while (1) is simpler (2) could be better solution to address scalability issues in tooling (especially in the open-source implementations where all extensions/features are being implemented).

The open-source compiler won't scale if we keep adding new features in non-modular way.

I would like to highlight the following recent discussion that provides more details of the implementation of builtin function declarations (https://lists.llvm.org/pipermail/cfe-dev/2021-February/067610.html). This shows that only now after having OpenCL for 10+ years we are able to provide support for the builtin functions directly in clang. Another issue that we have to deal with is a test time https://reviews.llvm.org/D97869.

Not sure if we should suggest increasing the priority of this issue and whether perhaps we should also file a separate issue for the scalability aspects in tooling too?