[OpenCL][Docs] Add guidelines for adding new extensions and features

[Anastasia]

Summary

Add documentation that explains how to extend clang with new extensions/features.

The guidelines also detail clang's position about the extension pragmas for the new functionality and it is expected the following change to be added first: https://reviews.llvm.org/D97052.

For further clarification around the extension pragma - the most common interpretation of the pragma in clang right now is the diagnostic of the use of types or functions from the extensions:

def err_opencl_requires_extension : Error<
  "use of %select{type|declaration}0 %1 requires %2 extension to be enabled">;

This forces the developers to use the pragma, but it is not very clear why and it seems to violate the following statement in the spec:

disable - Behave (including issuing errors and warnings) as if the extension extension_name is not part of the language definition.

The diagnostic given indicates that the disabled extension is indeed part of the language.

However, I must admit it doesn't seem that implementing dynamic loading/unloading of the extension functionality implied by the spec would be possible in clang. It is not clear why such flexibility would be needed either - most of the language provide one way of importing/including functionality.

Additionally, many extensions add functionality via the implicitly included headers and for using those no pragmas is required as no diagnostic is given if the pragma is not used.

Based on the above the conclusion is that the functionality provided by clang is neither conformant nor consistent for the user.

[svenvh]

Some minor typos and requests for clarifications, looks like reasonable guidelines other than that.

clang/docs/OpenCLSupport.rst

234

Probably easier to keep this section singular (i.e. talk about a adding a single extension), instead of switching between extension and extensions all the time.

235

associated

236

Again, keep it singular.

238

254

255

This might need some rephrasing to take TableGen BIFs into account. I would consider clang/lib/Sema/OpenCLBuiltins.td an integral part of the clang source code.

Maybe rephrase "clang source code modifications" into "clang parser source code modifications"?

260

Should this mention clang/lib/Sema/OpenCLBuiltins.td too?

271

275

278

280

Remove "acceptable behavior of".

How do you define "useful functionality"?

285

[Anastasia]

Added suggestions from Sven.

[Anastasia]

clang/docs/OpenCLSupport.rst

255

I am trying to avoid using parsing in relation to clang since someone might understand it as clang Parser library functionality. I have added a note about this case explicitly

Note that new functionality in `OpenCLBuiltins.td detailed in :ref:<opencl_builtins>` belong to this category even if it is part of the clang source code.

Although we do refer to header functionality in the next paragraph anyway where this is referenced but indirectly. I hope it provides enough clarifications for now.

260

I think here I don't want to go to too many details of how headers are implemented. I refer to this: https://clang.llvm.org/docs/UsersManual.html#opencl-header that also refers to OpenCLSupport page where we explain details on Tablegen header. However, the header topic does require another round of clarifications and I do plan to improve it further iin the near future.

280

I think this is one of those situations that is hard to define unambiguously, so I am open to suggestions. However, I do want to prevent changes that are reinventing the wheel (i.e. C/C++ already had another way to do the same thing) or functionality that doesn't add any value (i.e. requiring pragma enable to use already added types or functions). This has happened in the past multiple times so I think it is good to be specific that when new functionality is added it should have a reason for it.

I think the other statement above:

New functionality is accepted as soon as the documentation is detailed to the level sufficient to be implemented.

is similar. It is not very easy to tell what is the detailed level of documentation. FYI it generally aligns with clang overall policy:

https://clang.llvm.org/get_involved.html

A specification: The specification must be sufficient to understand the design of the feature as well as interpret the meaning of specific examples. The specification should be detailed enough that another compiler vendor could implement the feature.

I am just emphasizing this item here.

Regarding 'usefulness' I would even suggest adding it as a general guideline for clang but I think this is not very common. So for now I want to make sure we have it in our guidelines at least since we have encountered this.

[mantognini]

Some minor comments.

clang/docs/OpenCLSupport.rst

256

278

"of their behavior", I think.

282

[svenvh]

clang/docs/OpenCLSupport.rst

255

This makes it very confusing for the reader, because you're referring to "this category" without defining what a category is.

How about removing that note, and appending to the line above instead:

If an extension adds functionality that does not modify standard language parsing it should not require modifying anything other than header files and `OpenCLBuiltins.td as detailed in :ref:<opencl_builtins>`

260

Fair enough; with the addition I'm suggesting above my concern would be addressed anyway.

278

Yes!

280

I see, though I'm a bit hesitant to accept the word "useful" without any further explanation. Would adding the following make sense?

... should provide useful functionality that cannot be achieved by other means to the user.

[Anastasia]

• Added suggestions from Sven and Marco.

[Anastasia]

clang/docs/OpenCLSupport.rst

280

I think this wording doesn't cover the case with pragmas though... where the behavior was not possible to achieve without them but it was not clear why it was needed to achieve what was achieved with the pragmas.

I find useful pretty clear here even though it is not precisely defined. I would not be able to define it precisely at this point because I don't even know all the possible ways the pragmas can be misused. I think we might have to accept the fact that human language is not as precise as a programming language. As a matter of fact, we are not publishing an algorithm but only the guidelines. Although of course, we don't want to misguide, this might not always be avoidable. For the same reasons Clang contribution policy has never defined what is a sufficiently precise documentation format for the new features. This will be left for the community to assess on a case by case basis.

[svenvh]

LGTM mostly, but would like to hear the opinion of others on the discussion about "useful".

clang/docs/OpenCLSupport.rst

280

this wording doesn't cover the case with pragmas though... where the behavior was not possible to achieve without them but it was not clear why it was needed to achieve what was achieved with the pragmas.

Yes, that's a good point.

I'm still not sure about specifying "useful" without any further clarification then, but if others are happy to accept this then that's fine by me.

[Anastasia]

Added sentence to elaborate the meaning of "useful".

[svenvh]

LGTM with the latest elaboration on "useful". It seems you accidentally added unrelated changes to Types.cpp to this review, so please take care not to commit those.

[Anastasia]

Fixed patch reupload issue.

clang/docs/OpenCLSupport.rst

 ``1-global``, ``2-constant``, ``3-local``, ``4-generic``. The private address
 space is represented by the absence of an address space attribute in the IR (see
 also :ref:`the section on the address space attribute <opencl_addrsp>`).
 
    .. code-block:: console
 
      $ clang -cc1 -ffake-address-space-map test.cl
 
+.. _opencl_builtins:
+
 OpenCL builtins
 ---------------
 
 **Clang builtins**
 
 There are some standard OpenCL functions that are implemented as Clang builtins:
 
 - All pipe functions from `section 6.13.16.2/6.13.16.3
- A Clang TableGen emitter defined in ``ClangOpenCLBuiltinEmitter.cpp``.  During
   definitions in a compact manner.
 
 - OpenCL specific code in ``SemaLookup.cpp``.  When ``Sema::LookupBuiltin``
   encounters a potential builtin function, it will check if the name corresponds
   to a valid OpenCL builtin function.  If so, all overloads of the function are
   inserted using ``InsertOCLBuiltinDeclarationsFromTable`` and overload
   resolution takes place.
 
+OpenCL Extensions and Features
+------------------------------
+
+Clang implements various extensions to OpenCL kernel languages.
+
+New functionality is accepted as soon as the documentation is detailed to the
+level sufficient to be implemented. There should be an evidence that the
+extension is designed with implementation feasibility in consideration and
+assessment of complexity for C/C++ based compilers. Alternatively, the
+documentation can be accepted in a format of a draft that can be further
+refined during the implementation.
+
+Implementation guidelines
+^^^^^^^^^^^^^^^^^^^^^^^^^
+
+This section explains how to extend clang with the new functionality.
+
+**Parsing functionality**
+
+If an extension modifies the standard parsing it needs to be added to
+the clang frontend source code. This also means that the associated macro
+indicating the presence of the extension should be added to clang.
+
+The default flow for adding a new extension into the frontend is to
+modify `OpenCLExtensions.def
+<https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Basic/OpenCLExtensions.def>`_
+
+This will add the macro automatically and also add a field in the target
+options ``clang::TargetOptions::OpenCLFeaturesMap`` to control the exposure
+of the new extension during the compilation.
+
+Note that by default targets like `SPIR` or `X86` expose all the OpenCL
+extensions. For all other targets the configuration has to be made explicitly.
+
+Note that the target extension support performed by clang can be overridden
+with :ref:`-cl-ext <opencl_cl_ext>` command-line flags.
+
+**Library functionality**
+
+If an extension adds functionality that does not modify standard language
+parsing it should not require modifying anything other than header files or
+``OpenCLBuiltins.td`` detailed in :ref:`OpenCL builtins <opencl_builtins>`.
+Most commonly such extensions add functionality via libraries (by adding
+non-native types or functions) parsed regularly. Similar to other languages this
+is the most common way to add new functionality.
+
+Clang has standard headers where new types and functions are being added,
+for more details refer to
+:ref:`the section on the OpenCL Header <opencl_header>`. The macros indicating
+the presence of such extensions can be added in the standard header files
+conditioned on target specific predefined macros or/and language version
+predefined macros.
+
+**Pragmas**
+
+Some extensions alter standard parsing dynamically via pragmas.
+
+Clang provides a mechanism to add the standard extension pragma
+``OPENCL EXTENSION`` by setting a dedicated flag in the extension list entry of
+``OpenCLExtensions.def``. Note that there is no default behavior for the
+standard extension pragmas as it is not specified (for the standards up to and
+including version 3.0) in a sufficient level of detail and, therefore,
+there is no default functionality provided by clang.
+
+Pragmas without detailed information of their behavior (e.g. an explanation of
+changes it triggers in the parsing) should not be added to clang. Moreover, the
+pragmas should provide useful functionality to the user. For example, such
+functionality should address a practical use case and not be redundant i.e.
+cannot be achieved using existing features.
+
+Note that some legacy extensions (published prior to OpenCL 3.0) still
+provide some non-conformant functionality for pragmas e.g. add diagnostics on
+the use of types or functions. This functionality is not guaranteed to remain in
+future releases. However, any future changes should not affect backward
+compatibility.
+
 .. _opencl_addrsp:
 
 Address spaces attribute
 ------------------------
 
 Clang has arbitrary address space support using the ``address_space(N)``
 attribute, where ``N`` is an integer number in the range specified in the
 Clang source code. This addresses spaces can be used along with the OpenCL