[RFC] Added type trait to remove address space qualifier from type

[Anastasia]

Summary

As I am not developing libcxx, I am not familiar with the process. However, I was wondering if it makes sense to add the following feature.

Clang provides experimental support for address spaces in C++ mode that are represented as type qualifiers (following ISO/IEC JTC1 SC22 WG14 N1169 Section 5.1.2). It would be convenient to have a utility to remove address space from the type just like other qualifiers e.g. remove_cv.

It is not part of any official spec and therefore I am adding double underscore prefix to the name. Also the feature is guarded on the compilation with Clang since address spaces in C++ is currently just a Clang extension.

The need for this feature has been discussed in:
https://llvm.org/PR42033
https://reviews.llvm.org/D69938#1745766

Any feedback is valuable!

[jeroen.dobbelaere]

libcxx/test/libcxx/type_traits/address_space.pass.cpp

15

Maybe a test that the address space was indeed removed ? (by looking at pointer-compatibility or so ?)

[rjmccall]

libcxx/include/type_traits

713

The precedent in C++17 is that you should provide both __remove_address_space and __remove_address_space_t.

Please make the argument an unsigned long long, we don't want to artificially restrict the range here in case the attribute accepts something wider in the future.

libcxx/test/libcxx/type_traits/address_space.pass.cpp

15

It also needs to be std::__remove_address_space<intAS2>::type, but yeah, I think something like:

static_assert(!std::is_same<intAS2, int>::value, "address-space qualifier not applied");
static_assert(std::is_same<std::__remove_address_space<intAS2>::type>::value, int>, "address-space qualifier not removed");
static_assert(std::is_same<std::__remove_address_space<const volatile intAS2>::type>::value, const volatile int>, "other qualifiers inappropriately removed");

And you should test __remove_address_space_t the same way.

[EricWF]

We don't implement non-standard extensions in libc++.
Only when this trait is proposed for standardization can we consider adding it to the library

[rjmccall]

Address spaces are actually defined in a number of standardized C extensions that are compatible with C++, such as the Embedded C specification, OpenCL, and so on. There's precedent for libc++ accepting work that supports this kind of C++-compatible extension in the past; for example, std::is_pointer looks through Objective-C's ARC type qualifiers. That said, (1) libc++ hasn't actually added new traits for such things before, just taught existing traits to handle them, and (2) I think this definition wouldn't actually work for cases like the OpenCL address spaces because they're not expressible as __attribute__((address_space(N))). (2) is fixable; (1) may be a reasonable place to draw the line on policy.

[ldionne]

I agree with @EricWF here. Libc++ shouldn't become a repository of useful but non-standard utilities. It's useful to sometimes extend standard tools to non-standard concepts (like handling some Objective-C qualifiers when we can), but adding something new that hasn't been standardized is not really the business we want to be into.

However, @Anastasia , you could consider proposing the attribute for standardization, and if that makes the cut, then we'll be happy to implement it.

Cheers!

[Anastasia]

In D76636#1940086, @rjmccall wrote:

Address spaces are actually defined in a number of standardized C extensions that are compatible with C++, such as the Embedded C specification, OpenCL, and so on. There's precedent for libc++ accepting work that supports this kind of C++-compatible extension in the past; for example, std::is_pointer looks through Objective-C's ARC type qualifiers. That said, (1) libc++ hasn't actually added new traits for such things before, just taught existing traits to handle them, and (2) I think this definition wouldn't actually work for cases like the OpenCL address spaces because they're not expressible as __attribute__((address_space(N))). (2) is fixable; (1) may be a reasonable place to draw the line on policy.

Regarding (2) (although it's probably not changing the major concerns) this exact implementation also works for OpenCL because we map the address space keywords to the attributes during the parsing. However, OpenCL currently doesn't support libcxx. :(

[Anastasia]

In D76636#1941260, @ldionne wrote:

I agree with @EricWF here. Libc++ shouldn't become a repository of useful but non-standard utilities. It's useful to sometimes extend standard tools to non-standard concepts (like handling some Objective-C qualifiers when we can), but adding something new that hasn't been standardized is not really the business we want to be into.

However, @Anastasia , you could consider proposing the attribute for standardization, and if that makes the cut, then we'll be happy to implement it.

Cheers!

Ok, thanks for the feedback. I will certainly look into this however it may take long time. :)

Btw could we consider adding some sort of Clang specific C++ libs that would leverage the extensions that Clang supports in addition to the standard libraries? Wondering if it is something of interest to other use cases?

For address spaces, in particular, we would probably just need an additional header. We already have a header for OpenCL C that clang adds implicitly... so theoretically the same can be added for C++ for OpenCL. However, I was trying to avoid doing something that is only specific to OpenCL because I do believe a wider community can benefit from such functionality.

[rjmccall]

In D76636#1941575, @Anastasia wrote:

In D76636#1940086, @rjmccall wrote:

Address spaces are actually defined in a number of standardized C extensions that are compatible with C++, such as the Embedded C specification, OpenCL, and so on. There's precedent for libc++ accepting work that supports this kind of C++-compatible extension in the past; for example, std::is_pointer looks through Objective-C's ARC type qualifiers. That said, (1) libc++ hasn't actually added new traits for such things before, just taught existing traits to handle them, and (2) I think this definition wouldn't actually work for cases like the OpenCL address spaces because they're not expressible as __attribute__((address_space(N))). (2) is fixable; (1) may be a reasonable place to draw the line on policy.

Regarding (2) (although it's probably not changing the major concerns) this exact implementation also works for OpenCL because we map the address space keywords to the attributes during the parsing. However, OpenCL currently doesn't support libcxx. :(

Ah, okay. Maybe it's one of the other languages that keeps its address spaces separate from address_space.

Regardless, if libc++ wants to have a policy of not adding new features that cover non-C++ features, that's a reasonable stance. libc++ should consider whether is_pointer needs a workaround for stripping address-space qualifiers, though.

[Anastasia]

In D76636#1941590, @rjmccall wrote:

In D76636#1941575, @Anastasia wrote:

In D76636#1940086, @rjmccall wrote:

Address spaces are actually defined in a number of standardized C extensions that are compatible with C++, such as the Embedded C specification, OpenCL, and so on. There's precedent for libc++ accepting work that supports this kind of C++-compatible extension in the past; for example, std::is_pointer looks through Objective-C's ARC type qualifiers. That said, (1) libc++ hasn't actually added new traits for such things before, just taught existing traits to handle them, and (2) I think this definition wouldn't actually work for cases like the OpenCL address spaces because they're not expressible as __attribute__((address_space(N))). (2) is fixable; (1) may be a reasonable place to draw the line on policy.

Regarding (2) (although it's probably not changing the major concerns) this exact implementation also works for OpenCL because we map the address space keywords to the attributes during the parsing. However, OpenCL currently doesn't support libcxx. :(

Ah, okay. Maybe it's one of the other languages that keeps its address spaces separate from address_space.

Regardless, if libc++ wants to have a policy of not adding new features that cover non-C++ features, that's a reasonable stance.

This perfectly makes sense. However, we do have useful Clang extensions that need corresponding support in the libraries too. Particularly for address spaces we could add such functionality into an OpenCL specific headers and libs. It feels a bit unfortunate because the implementation in Clang is generic and therefore more areas could benefit from this feature. I don't know if we could consider to add some Clang extensions specific C++ libs... I understand that standardisation with ISO is valuabloe but it is not often a reality in the embedded world.

libc++ should consider whether is_pointer needs a workaround for stripping address-space qualifiers, though.

Yes, I see. I will look into this. Thanks!

[Anastasia]

libcxx/include/type_traits

713

The pattern is incorrect!

[ldionne]

Resigning to clean up my review queue.

[rjmccall]

Anastasia, are you still looking at this? It looks like you have a fix for your bug, but the patch never got updated.

[Anastasia]

In D76636#2835000, @rjmccall wrote:

Anastasia, are you still looking at this? It looks like you have a fix for your bug, but the patch never got updated.

It would be good to have this functionality but it seems that libc++ currently doesn't accept functionality that is not in standard C++ yet.

I am now looking at adding this pattern in the Clang headers for OpenCL. Alternatively, we can add a clang builtin for this if you think it is useful to the wider community? Other options could include adding a specific header or libs to implement address space-specific logic in C++.

[rjmccall]

In D76636#2835529, @Anastasia wrote:

In D76636#2835000, @rjmccall wrote:

Anastasia, are you still looking at this? It looks like you have a fix for your bug, but the patch never got updated.

It would be good to have this functionality but it seems that libc++ currently doesn't accept functionality that is not in standard C++ yet.

Ah, right, I'd forgotten. In that case, it would probably be best to just close this, since it has no path to acceptance.

libcxx/include/type_traits

namespace std
     template <class T> struct is_volatile;
     template <class T> struct remove_const;
     template <class T> struct remove_volatile;
     template <class T> struct remove_cv;
     template <class T> struct add_const;
     template <class T> struct add_volatile;
     template <class T> struct add_cv;
 
+#ifdef __clang__
+    // Remove address space from type.
+    template <class T> struct __remove_address_space;
+#endif
+
     // Reference transformations:
     template <class T> struct remove_reference;
     template <class T> struct add_lvalue_reference;
     template <class T> struct add_rvalue_reference;
 
     // Pointer transformations:
     template <class T> struct remove_pointer;
     template <class T> struct add_pointer;

 // remove_cv
 
 template <class _Tp> struct _LIBCPP_TEMPLATE_VIS remove_cv
 {typedef typename remove_volatile<typename remove_const<_Tp>::type>::type type;};
 #if _LIBCPP_STD_VER > 11
 template <class _Tp> using remove_cv_t = typename remove_cv<_Tp>::type;
 #endif
 
+#ifdef __clang__
+// __remove_address_space
+
+template<class... T> struct __remove_address_space;
+template <class _Tp> struct __remove_address_space<_Tp> { typedef _Tp type; };
+template <class _Tp, int I> struct __remove_address_space<_Tp __attribute__((address_space(I)))> { typedef _Tp __attribute__((address_space(I))) type; };
+#endif
+
 // is_void
 
 template <class _Tp> struct __libcpp_is_void       : public false_type {};
 template <>          struct __libcpp_is_void<void> : public true_type {};
 
 template <class _Tp> struct _LIBCPP_TEMPLATE_VIS is_void
     : public __libcpp_is_void<typename remove_cv<_Tp>::type> {};
 

libcxx/test/libcxx/type_traits/address_space.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+
+#include <type_traits>
+#ifdef __clang__
+typedef int __attribute__((address_space(2))) intAS2;
+
+void foo(){
+  std::__remove_address_space<intAS2> i;
+  (void)i;
+}
+#endif
+
+int main(int, char**) {
+  return 0;
+}