This is an archive of the discontinued LLVM Phabricator instance.

Differential D61506

[OpenCL] Switch to C++17
ClosedPublic

Authored by Anastasia on May 3 2019, 7:02 AM.

Details

Reviewers
kpet
Commits
rZORG4ef89625cafe: [OpenCL] Switched CXX mode to be derived from C++17
rZORGbc5d0bb240e7: [OpenCL] Switched CXX mode to be derived from C++17
rG4ef89625cafe: [OpenCL] Switched CXX mode to be derived from C++17
rGbc5d0bb240e7: [OpenCL] Switched CXX mode to be derived from C++17
rG933e305ed909: [OpenCL] Switched CXX mode to be derived from C++17
rC360330: [OpenCL] Switched CXX mode to be derived from C++17
rL360330: [OpenCL] Switched CXX mode to be derived from C++17
Summary

Caused test update as frontend now produces better IR due to copy elision.

Diff Detail

Repository
rL LLVM

Event Timeline

Anastasia created this revision.May 3 2019, 7:02 AM
Herald added subscribers: ebevhan, yaxunl. · View Herald TranscriptMay 3 2019, 7:02 AM
kpet added inline comments.May 3 2019, 7:38 AM
include/clang/Frontend/LangStandards.def
162 ↗(On Diff #197996)

Suggest you add HexFloat as well. It is part of c++17 and all OpenCL versions.

keryell added a subscriber: keryell.May 3 2019, 2:33 PM
keryell added inline comments.
include/clang/Frontend/LangStandards.def
162 ↗(On Diff #197996)

Why only C++17?
I would love to have CPlusPlus2a here too...

rsmith added a subscriber: rsmith.May 3 2019, 3:54 PM

Per the OpenCL C++ 1.0 specification, section 2:

The OpenCL C++ programming language is based on the ISO/IEC JTC1 SC22 WG21 N 3690 language specification (a.k.a. C++14 specification).

I think it would be reasonable to permit changing the base C++ standard in OpenCL C++ mode, but we'd need a good reason to deviate from the behavior specified in the OpenCL C++ specification by default.

Anastasia marked an inline comment as done.May 7 2019, 2:56 AM
In D61506#1490555, @rsmith wrote:

Per the OpenCL C++ 1.0 specification, section 2:

The OpenCL C++ programming language is based on the ISO/IEC JTC1 SC22 WG21 N 3690 language specification (a.k.a. C++14 specification).

I think it would be reasonable to permit changing the base C++ standard in OpenCL C++ mode, but we'd need a good reason to deviate from the behavior specified in the OpenCL C++ specification by default.

I am in discussion with Khronos about changing the directions for this spec and I believe some announcements will appear hopefully soon.

include/clang/Frontend/LangStandards.def
162 ↗(On Diff #197996)

I would love to have C++2a too but it's currently an experimental feature only and I can't see any timeline about when this will become production quality. As we are planning to release C++ for OpenCL in production quality during this year I am afraid it won't be possible to include C++2a at this point .

Perhaps this is something we can do in the future.

keryell added a comment.May 7 2019, 12:32 PM
In D61506#1490555, @rsmith wrote:

Per the OpenCL C++ 1.0 specification, section 2:

The OpenCL C++ programming language is based on the ISO/IEC JTC1 SC22 WG21 N 3690 language specification (a.k.a. C++14 specification).

I think it would be reasonable to permit changing the base C++ standard in OpenCL C++ mode

Indeed! There should be an option to pick the version of C++ the user wants for OpenCL.

Anastasia added a comment.May 8 2019, 2:57 AM
In D61506#1493953, @keryell wrote:
In D61506#1490555, @rsmith wrote:

Per the OpenCL C++ 1.0 specification, section 2:

The OpenCL C++ programming language is based on the ISO/IEC JTC1 SC22 WG21 N 3690 language specification (a.k.a. C++14 specification).

I think it would be reasonable to permit changing the base C++ standard in OpenCL C++ mode

Indeed! There should be an option to pick the version of C++ the user wants for OpenCL.

Unfortunately, this is not easy to do. We document changes on top of other spec and we need to provide references. It would be a lot more work to refer to all different spec versions of C++. Also the development becomes more complicated as it needs testing and fixing against all different C++ versions. It makes sense to focus on one version and potentially provide an upgrade in the future. Also C++17 is a reasonable target most of developers should be happy with at this point.

Anastasia updated this revision to Diff 198627.May 8 2019, 4:57 AM

Added HexFloat!

kpet accepted this revision.May 8 2019, 5:27 AM

LGTM.

This revision is now accepted and ready to land.May 8 2019, 5:27 AM
Closed by commit rL360330: [OpenCL] Switched CXX mode to be derived from C++17 (authored by stulova). · Explain WhyMay 9 2019, 4:55 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMay 9 2019, 4:55 AM
Herald added a subscriber: llvm-commits. · View Herald Transcript

Revision Contents

PathSize
cfe/
trunk/
include/
clang/
Frontend/
3 lines
test/
CodeGenOpenCLCXX/
8 lines

Diff 198792

cfe/trunk/include/clang/Frontend/LangStandards.def

Show First 20 Lines • Show All 153 Lines • ▼ Show 20 Lines
LANGSTANDARD(opencl12, "cl1.2", LANGSTANDARD(opencl12, "cl1.2",
OpenCL, "OpenCL 1.2", OpenCL, "OpenCL 1.2",
LineComment | C99 | Digraphs | HexFloat | OpenCL) LineComment | C99 | Digraphs | HexFloat | OpenCL)
LANGSTANDARD(opencl20, "cl2.0", LANGSTANDARD(opencl20, "cl2.0",
OpenCL, "OpenCL 2.0", OpenCL, "OpenCL 2.0",
LineComment | C99 | Digraphs | HexFloat | OpenCL) LineComment | C99 | Digraphs | HexFloat | OpenCL)
LANGSTANDARD(openclcpp, "c++", LANGSTANDARD(openclcpp, "c++",
OpenCL, "OpenCL C++ 1.0", OpenCL, "OpenCL C++ 1.0",
LineComment | CPlusPlus | CPlusPlus11 | CPlusPlus14 | Digraphs | OpenCL) LineComment | CPlusPlus | CPlusPlus11 | CPlusPlus14 | CPlusPlus17 |
Digraphs | HexFloat | OpenCL)
LANGSTANDARD_ALIAS_DEPR(opencl10, "CL") LANGSTANDARD_ALIAS_DEPR(opencl10, "CL")
LANGSTANDARD_ALIAS_DEPR(opencl11, "CL1.1") LANGSTANDARD_ALIAS_DEPR(opencl11, "CL1.1")
LANGSTANDARD_ALIAS_DEPR(opencl12, "CL1.2") LANGSTANDARD_ALIAS_DEPR(opencl12, "CL1.2")
LANGSTANDARD_ALIAS_DEPR(opencl20, "CL2.0") LANGSTANDARD_ALIAS_DEPR(opencl20, "CL2.0")
// CUDA // CUDA
LANGSTANDARD(cuda, "cuda", CUDA, "NVIDIA CUDA(tm)", LANGSTANDARD(cuda, "cuda", CUDA, "NVIDIA CUDA(tm)",
Show All 9 Lines

cfe/trunk/test/CodeGenOpenCLCXX/addrspace-of-this.cl

Show First 20 Lines • Show All 106 Lines • ▼ Show 20 Lines
// COMMON: [[C1GEN:%[0-9]+]] = addrspacecast %class.C* %c1 to %class.C addrspace(4)* // COMMON: [[C1GEN:%[0-9]+]] = addrspacecast %class.C* %c1 to %class.C addrspace(4)*
// COMMON: [[C2GEN:%[0-9]+]] = addrspacecast %class.C* %c2 to %class.C addrspace(4)* // COMMON: [[C2GEN:%[0-9]+]] = addrspacecast %class.C* %c2 to %class.C addrspace(4)*
// EXPL: call dereferenceable(4) %class.C addrspace(4)* @_ZNU3AS41CaSERU3AS4KS_(%class.C addrspace(4)* [[C2GEN]], %class.C addrspace(4)* dereferenceable(4) [[C1GEN]]) // EXPL: call dereferenceable(4) %class.C addrspace(4)* @_ZNU3AS41CaSERU3AS4KS_(%class.C addrspace(4)* [[C2GEN]], %class.C addrspace(4)* dereferenceable(4) [[C1GEN]])
// IMPL: [[C2GENVOID:%[0-9]+]] = bitcast %class.C addrspace(4)* [[C2GEN]] to i8 addrspace(4)* // IMPL: [[C2GENVOID:%[0-9]+]] = bitcast %class.C addrspace(4)* [[C2GEN]] to i8 addrspace(4)*
// IMPL: [[C1GENVOID:%[0-9]+]] = bitcast %class.C addrspace(4)* [[C1GEN]] to i8 addrspace(4)* // IMPL: [[C1GENVOID:%[0-9]+]] = bitcast %class.C addrspace(4)* [[C1GEN]] to i8 addrspace(4)*
// IMPL: call void @llvm.memcpy.p4i8.p4i8.i32(i8 addrspace(4)* {{.*}}[[C2GENVOID]], i8 addrspace(4)* {{.*}}[[C1GENVOID]] // IMPL: call void @llvm.memcpy.p4i8.p4i8.i32(i8 addrspace(4)* {{.*}}[[C2GENVOID]], i8 addrspace(4)* {{.*}}[[C1GENVOID]]
// Test the address space of 'this' when invoking the operator+ // Test the address space of 'this' when invoking the operator+
// COMMON: [[C3GEN:%[0-9]+]] = addrspacecast %class.C* %c3 to %class.C addrspace(4)*
// COMMON: [[C1GEN:%[0-9]+]] = addrspacecast %class.C* %c1 to %class.C addrspace(4)* // COMMON: [[C1GEN:%[0-9]+]] = addrspacecast %class.C* %c1 to %class.C addrspace(4)*
// COMMON: [[C2GEN:%[0-9]+]] = addrspacecast %class.C* %c2 to %class.C addrspace(4)* // COMMON: [[C2GEN:%[0-9]+]] = addrspacecast %class.C* %c2 to %class.C addrspace(4)*
// COMMON: call void @_ZNU3AS41CplERU3AS4KS_(%class.C* sret %ref.tmp, %class.C addrspace(4)* [[C1GEN]], %class.C addrspace(4)* dereferenceable(4) [[C2GEN]]) // COMMON: call void @_ZNU3AS41CplERU3AS4KS_(%class.C* sret %c3, %class.C addrspace(4)* [[C1GEN]], %class.C addrspace(4)* dereferenceable(4) [[C2GEN]])
// COMMON: [[REFGEN:%[0-9]+]] = addrspacecast %class.C* %ref.tmp to %class.C addrspace(4)*
// EXPL: call void @_ZNU3AS41CC1EOU3AS4S_(%class.C addrspace(4)* [[C3GEN]], %class.C addrspace(4)* dereferenceable(4) [[REFGEN]])
// IMPL: [[C3VOID:%[0-9]+]] = bitcast %class.C* %c3 to i8*
// IMPL: [[REFGENVOID:%[0-9]+]] = bitcast %class.C addrspace(4)* [[REFGEN]] to i8 addrspace(4)*
// IMPL: call void @llvm.memcpy.p0i8.p4i8.i32(i8* {{.*}}[[C3VOID]], i8 addrspace(4)* {{.*}}[[REFGENVOID]]
// Test the address space of 'this' when invoking the move constructor // Test the address space of 'this' when invoking the move constructor
// COMMON: [[C4GEN:%[0-9]+]] = addrspacecast %class.C* %c4 to %class.C addrspace(4)* // COMMON: [[C4GEN:%[0-9]+]] = addrspacecast %class.C* %c4 to %class.C addrspace(4)*
// COMMON: [[CALL:%call[0-9]+]] = call spir_func dereferenceable(4) %class.C addrspace(4)* @_Z3foov() // COMMON: [[CALL:%call[0-9]+]] = call spir_func dereferenceable(4) %class.C addrspace(4)* @_Z3foov()
// EXPL: call void @_ZNU3AS41CC1EOU3AS4S_(%class.C addrspace(4)* [[C4GEN]], %class.C addrspace(4)* dereferenceable(4) [[CALL]]) // EXPL: call void @_ZNU3AS41CC1EOU3AS4S_(%class.C addrspace(4)* [[C4GEN]], %class.C addrspace(4)* dereferenceable(4) [[CALL]])
// IMPL: [[C4VOID:%[0-9]+]] = bitcast %class.C* %c4 to i8* // IMPL: [[C4VOID:%[0-9]+]] = bitcast %class.C* %c4 to i8*
// IMPL: [[CALLVOID:%[0-9]+]] = bitcast %class.C addrspace(4)* [[CALL]] to i8 addrspace(4)* // IMPL: [[CALLVOID:%[0-9]+]] = bitcast %class.C addrspace(4)* [[CALL]] to i8 addrspace(4)*
// IMPL: call void @llvm.memcpy.p0i8.p4i8.i32(i8* {{.*}}[[C4VOID]], i8 addrspace(4)* {{.*}}[[CALLVOID]] // IMPL: call void @llvm.memcpy.p0i8.p4i8.i32(i8* {{.*}}[[C4VOID]], i8 addrspace(4)* {{.*}}[[CALLVOID]]
▲ Show 20 Lines • Show All 80 Lines • Show Last 20 Lines