[PATCH] D64418: [Docs][OpenCL] Documentation of C++ for OpenCL mode

Thu Aug 15 05:22:57 PDT 2019

svenvh added inline comments.

================
Comment at: docs/LanguageExtensions.rst:1561
+s3.1.3). For OpenCL it means that implicit conversions are allowed from
+named address space except for ``__constant`` to ``__generic`` address space
+that is a superset of all others except for ``__constant`` (OpenCL C v2.0
----------------
"For OpenCL it means that implicit conversions are allowed from a named address space except for conversion from the ``__constant`` to the ``__generic`` address space."

I would omit "that is a superset of all others except for ``__constant``" as that is already documented in the OpenCL spec and it might confuse readers here.  If you want to keep it, I suggest rephrasing it as a separate sentence.

================
Comment at: docs/LanguageExtensions.rst:1563
+that is a superset of all others except for ``__constant`` (OpenCL C v2.0
+s6.5.5). Reverse conversion is only allowed explicitly.  ``__constant``
+address space does not overlap with any other and therefore no valid conversion
----------------
+The+ ``__constant`` address space

================
Comment at: docs/LanguageExtensions.rst:1570
+
+C style cast will follow OpenCL C v2.0 rules (s6.5.5). All cast operators will
+permit conversion to ``__generic`` implicitly. However converting from
----------------
"C-style casts follow"

================
Comment at: docs/LanguageExtensions.rst:1630
+All non-static member functions take an implicit object parameter ``this`` that
+is a pointer type. By default this pointer parameter is in ``__generic`` address
+space. All concrete objects passed as an argument to ``this`` parameter will be
----------------
+the+ ``__generic`` address space

(also in the next few lines)

================
Comment at: docs/LanguageExtensions.rst:1647
+
+Clang allows specifying address space qualifier on member functions to signal that
+they are to be used with objects constructed in some specific address space. This
----------------
+an+ address space qualifier

================
Comment at: docs/LanguageExtensions.rst:1650
+works just the same as qualifying member functions with ``const`` or any other
+qualifiers. The overloading resolution will select ithe overload with most specific
+address space if multiple candidates are provided. If there is no conversion to
----------------
ithe -> the

================
Comment at: docs/LanguageExtensions.rst:1651
+qualifiers. The overloading resolution will select ithe overload with most specific
+address space if multiple candidates are provided. If there is no conversion to
+to an address space among existing overloads compilation will fail with a
----------------
Duplicate "to" (to to)

================
Comment at: docs/LanguageExtensions.rst:1652
+address space if multiple candidates are provided. If there is no conversion to
+to an address space among existing overloads compilation will fail with a
+diagnostic.
----------------
Add comma:
... overloads, compilation ...

================
Comment at: docs/LanguageExtensions.rst:1715
+definition and instantiation. If multiple different address spaces are specified in
+template definition and instantiation compilation of such a program will fail with a
+diagnostic.
----------------
Add comma:
... instantiation, compilation ...

================
Comment at: docs/LanguageExtensions.rst:1730
+
+Once template is instantiated regular restrictions for address spaces will apply.
+
----------------
Once a template has been instantiated, regular restrictions for address spaces will apply.

================
Comment at: docs/LanguageExtensions.rst:1746
+
+All temporaries are materialized in ``__private`` address space. If a reference
+with some other address space is bound to them, the conversion will be generated
----------------
the ``__private`` address space

================
Comment at: docs/LanguageExtensions.rst:1747
+All temporaries are materialized in ``__private`` address space. If a reference
+with some other address space is bound to them, the conversion will be generated
+in case it's valid otherwise compilation will fail with a diagnostic.
----------------
some other -> another

================
Comment at: docs/LanguageExtensions.rst:1748
+with some other address space is bound to them, the conversion will be generated
+in case it's valid otherwise compilation will fail with a diagnostic.
+
----------------
it's -> it is

add comma:
... valid, otherwise ...

================
Comment at: docs/LanguageExtensions.rst:1776
+API for invoking global constructors. However, an easy workaround would be
+to enqueue constructor initialization kernel that has a name
+``@_GLOBAL__sub_I_<compiled file name>``. This kernel is only present if there
----------------
enqueue *a* constructor

================
Comment at: docs/LanguageExtensions.rst:1782
+
+Note that if multiple files are compiled and linked into libraries multiple
+kernels that initialize global objects for multiple modules would have to be
----------------
Add comma:
... libraries, multiple ...

================
Comment at: docs/LanguageExtensions.rst:1793
+
+If there are any global objects to be initialized the final binary will
+contain ``@_GLOBAL__sub_I_test.cl`` kernel to be enqueued.
----------------
Add comma:
... initialized, the ...

================
Comment at: docs/LanguageExtensions.rst:1794
+If there are any global objects to be initialized the final binary will
+contain ``@_GLOBAL__sub_I_test.cl`` kernel to be enqueued.
+
----------------
contain +the+

================
Comment at: docs/UsersManual.rst:2769
+<https://www.khronos.org/registry/OpenCL/specs/2.2/pdf/OpenCL_Cxx.pdf>`_ and
+there is no plan to support it in clang in any new releases in the near future.
+
----------------
kpet wrote:
> No plans that we're aware of ;). For all we know, there could be a company/individual working on this. I think conjectures on people's plans don't belong in the documentation.
> I think conjectures on people's plans don't belong in the documentation.

Agreed.

================
Comment at: docs/UsersManual.rst:2771
+
+There are only a few restrictions on allowed C++ features. For detailed information
+please refer to documentation on Extensions (:doc:`LanguageExtensions`).
----------------
Anastasia wrote:
> kpet wrote:
> > Everything is relative. I would keep this 100% factual: "there are restrictions". This is not a sales pitch :).
> I think it's fair to say few restrictions because the majority of C++ just works in OpenCL.
I agree with @kpet and also prefer "There are restrictions" instead of an unquantified statement.

================
Comment at: docs/UsersManual.rst:2765
+
+Starting from Clang9 kernel code can contain C++17 features: classes, templates,
+function overloading, type deduction, etc. Please note that this is not an
----------------
Clang9 -> Clang 9 (with space).

================
Comment at: docs/UsersManual.rst:2776
+restrictions from OpenCL C v2.0 will inherently apply. All OpenCL C builtin types
+and function libraries are supported and can be used in the new mode.
+
----------------
I wouldn't say "new" mode as this is part of a persistent user manual.

================
Comment at: docs/UsersManual.rst:2778
+
+To enable the new mode pass the following command line option when compiling ``.cl``
+file ``-cl-std=clc++``, ``-cl-std=CLC++``, ``-std=clc++`` or ``-std=CLC++``.
----------------
Again I would avoid "new mode".

"To enable the C++ for OpenCL mode,"

Also: pass *one of* the following command line options when compiling a ``.cl``

So in total:
To enable the C++ for OpenCL mode, pass one of the following command line options when compiling a ``.cl`` file:

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64418/new/

https://reviews.llvm.org/D64418