mantognini requested changes to this revision. mantognini added a comment. This revision now requires changes to proceed.
The overall structure seems alright. I'll let people more familiar with the code-base judge whether other important features should be added there as well. ================ Comment at: docs/ReleaseNotes.rst:49 +- Full experimental support of :ref:`C++ for OpenCL <openclcpp>` has + been added. ---------------- "Full experimental" feels like an oxymoron. I would drop the "full". ================ Comment at: docs/ReleaseNotes.rst:49 +- Full experimental support of :ref:`C++ for OpenCL <openclcpp>` has + been added. ---------------- mantognini wrote: > "Full experimental" feels like an oxymoron. I would drop the "full". "support of" -> "support for" ================ Comment at: docs/ReleaseNotes.rst:138 +- Support of address space attribute in various C++ features was improved, refer + to :ref:`C++ for OpenCL <openclcpp>` for more details). The following features ---------------- Support //for// address space attribute//s// [...] improved //(refer// [...] Alternatively, if you don't want to use parenthesis, drop the closing parenthesis on the next line. ================ Comment at: docs/ReleaseNotes.rst:142 + + (1) address spaces as method qualifiers are not accepted yet; + ---------------- Inconsistent sentence capitalization between the two bullet points. ================ Comment at: docs/ReleaseNotes.rst:173 + +- Added initial support for implicitly including OpenCL BIFs using + efficient trie lookup generated by TableGen. A corresponding ---------------- If the BIF acronym wasn't introduced before, it should be replaced with "builtin functions". It seems we don't have more file context in this review so I cannot tell. ================ Comment at: docs/ReleaseNotes.rst:175 + efficient trie lookup generated by TableGen. A corresponding + frontend only flag ``-fadd-opencl-builtins`` has been added to + enable trie during parsing. ---------------- I'm not 100% sure about the grammar rule in English, but shouldn't there be a "-" between "frontend" and "only" here to make it an adjective-ish? ================ Comment at: docs/ReleaseNotes.rst:183 + +- Simplified representation of blocks including their generation in + IR i.e. indirect calls to block function has been changed to ---------------- Simplified //the// representation of blocks//**,**// including their generation //into// IR. //Furthermore,// indirect calls [...] (I'm assuming here that the indirect calls to block function is not the only improvement. And even if it is, "i.e." is less impressive, isn't it?) ================ Comment at: docs/ReleaseNotes.rst:194 + +- Improved math builtin function of long long for x86. + ---------------- Maybe this would be better? Improved math builtin functions with parameters of type `long long` for x86. ================ Comment at: docs/ReleaseNotes.rst:201 + +Full experimental support for C++17 features in OpenCL has been +added and backwards compatibility to OpenCL C v2.0 was enabled. ---------------- Ditto, I would drop "full" here. ================ Comment at: docs/ReleaseNotes.rst:202 +Full experimental support for C++17 features in OpenCL has been +added and backwards compatibility to OpenCL C v2.0 was enabled. +The documentation has been added for supported language features ---------------- compatible //with// ================ Comment at: docs/ReleaseNotes.rst:209 + +- Templates parameters and arguments + ---------------- Did you meant to indent this bullet point as well? ================ Comment at: docs/ReleaseNotes.rst:215 + + - Objects and member functions including special member + functions; ---------------- Missing comma: "functions, including" ================ Comment at: docs/ReleaseNotes.rst:220 + + - Method qualifiers are allowed with address spaces; + ---------------- Maybe something along these line would be better? Methods can be overloaded for different address spaces. Or, if you want to emphasis the qualifiers, Method qualifiers now include address space. ================ Comment at: docs/ReleaseNotes.rst:222 + + - Address space deduction has been extended for C++ use cases; + ---------------- This seems to be already included in previous point. ================ Comment at: docs/ReleaseNotes.rst:226 + + - Cast operators are now preventing to converting address + spaces. They can still be cast using C style cast. ---------------- Which "cast" operators? ================ Comment at: docs/ReleaseNotes.rst:226 + + - Cast operators are now preventing to converting address + spaces. They can still be cast using C style cast. ---------------- mantognini wrote: > Which "cast" operators? [...] are now prevent//ed from// converting [...] ================ Comment at: docs/ReleaseNotes.rst:229 + +- Vector types as in OpenCL C including compound vector + initialization. ---------------- missing comma: "C, including" ================ Comment at: docs/ReleaseNotes.rst:232-233 + +- OpenCL specific type: images, samplers, events, pipes, except + for blocks. + ---------------- I would suggest this: OpenCL-specific types, except within blocks: [...] ================ Comment at: docs/ReleaseNotes.rst:237-238 + +- Global constructor stab is made an executable kernel to allow + invoking it from the host side. + ---------------- Stab? Stub? Global constructors can be invoked from the host side using a specific, compiler-generated kernel. ================ Comment at: docs/ReleaseNotes.rst:240-241 + +- Overloads with generic address space are added to all atomics + including the ones from prior to OpenCL v2.0. ---------------- [...] to all //atomic builtins//(?)**//,//** including the ones prior to [...] CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66294/new/ https://reviews.llvm.org/D66294 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
