[libcxx-commits] [PATCH] D144362: [libcxx] <experimental/simd> Add ABI tags, class template simd/simd_mask implementations. Add related simd traits and tests.

[Yin Zhang via Phabricator via libcxx-commits]

Joy12138 added inline comments.


================
Comment at: libcxx/include/experimental/simd:96-99
+  template<class _Tp, class = void> struct __is_abi_tag_impl : false_type {};
+  template<class _Tp> struct __is_abi_tag_impl<_Tp, enable_if_t<_Tp::__is_abi_tag>> : bool_constant<_Tp::__is_abi_tag> {};
+  template<class _Tp> struct is_abi_tag : __is_abi_tag_impl<_Tp> {};
+  template<class _Tp> inline constexpr bool is_abi_tag_v = is_abi_tag<_Tp>::value;
----------------
philnik wrote:
> Joy12138 wrote:
> > Joy12138 wrote:
> > > philnik wrote:
> > > > Joy12138 wrote:
> > > > > philnik wrote:
> > > > > > I think it would be easier to move this to it's own header, and then specialize `is_abi_tag` per ABI tag, i.e.
> > > > > > ```lang=c++
> > > > > > // traits.h
> > > > > > template <class _Tp>
> > > > > > inline constexpr bool is_abi_tag_v = false;
> > > > > > 
> > > > > > template <class _Tp>
> > > > > > struct is_abi_tag : bool_constant<is_abi_tag_v<_Tp>> {};
> > > > > > 
> > > > > > // scalar.h
> > > > > > template <>
> > > > > > inline constexpr bool is_abi_tag_v<__scalar> = true;
> > > > > > 
> > > > > > // vec_ext.h
> > > > > > template <int _Np>
> > > > > > inline constexpr bool is_abi_tag_v<__vec_ext<_Np>> = true;
> > > > > > ```
> > > > > > Note that this also implements `is_abi_tag` in terms of `is_abi_tag_v` and not the other way around. Most people just use the latter, so we should avoid instantiating a class every time. This part also applies to `is_simd` and `is_simd_mask`.
> > > > > This looks great, but there are often references to other interfaces in the implementation of traits. If traits are to be implemented in different ABI tag headers, we may need to add some declarations to the outer layer of these headers. For example, class `simd`/`simd_mask` will be referenced in the `is_simd`/`is_simd_mask` implementation. Should I use a new header (`declaration. h`) to add these declarations?
> > > > You just specialize the traits in the header where the class is defined. No need to forward-declare anything.
> > > Sorry, my previous description was not clear enough. The main issue is that in our design, for more code reuse and more flexible use of internal interfaces, the structure of the external interface layer and internal implementation layer is not consistent. And template specialization does not directly occur on external interfaces such as `simd/simd_mask` classes.
> > > 
> > > We have designed four modules, **SIMD/MASK storage type** and **SIMD/MASK operations**, for different ABI tags at the implementation layer, which will be used by various external interfaces, including `simd/simd_mask`, `where_expression` and non-member functions.
> > > 
> > > i.e.
> > > 
> > > ```lang=c++
> > > // simd
> > > template <class _Tp, class _Abi>
> > > class simd {
> > >   using _Impl    = __simd_operations<_Tp, _Abi>;
> > >   using _Storage = __simd_storage<_Tp, _Abi>;
> > >   
> > >   _Storage __s_;
> > >   
> > >   simd operator+(const simd& __lhs, const simd& __rhs) {
> > >     return _Impl::__plus(__lhs.__s_, __rhs.__s_);
> > >   }
> > > }
> > > 
> > > // scalar.h
> > > template <class _Tp>
> > > struct __simd_storage<_Tp, simd_abi::__scalar> {
> > > 	......
> > > };
> > > 
> > > template <class _Tp>
> > > struct __simd_operations<_Tp, simd_abi::__scalar> {
> > > 	......
> > > };
> > > 
> > > // vec_ext.h
> > > template <class _Tp, int _Np>
> > > struct __simd_storage<_Tp, simd_abi::__vec_ext<_Np>> {
> > > 	......
> > > };
> > > 
> > > template <class _Tp, int _Np>
> > > struct __simd_operations<_Tp, simd_abi::__vec_ext<_Np>> {
> > > 	......
> > > };
> > > ```
> > > 
> > > Since the specialization does not occur on external interfaces, the external interfaces is now directly placed in the `simd` header. if the implementation of simd traits is to be placed in a separate header, a forward declaration is required, and if it is placed together with the external interfaces, it will also be placed in the `simd` header.
> > > 
> > > Do you have any better suggestions for the current code structure?
> > > 
> > In addition, most simd traits are platform independent, is it not necessary to specialize for all traits?
> I'm a bit confused what exactly you are talking about. Was this meant for a different thread?
I'm sorry my description confused you. Maybe I shouldn't have put this content in this thread. I just want to discuss the overall structure and the use of template specialization. And they have some impact on the implementation of simd traits.

What I mainly want to express is that since we do not directly specialize templates on external user interfaces, there will not be directly definitions and implementations of such as 
`simd<_Tp, __vec_ext<_Np>>`. It is basically composed of `simd_storage<_Tp, __vec_ext<_Np>>` and `simd_operations<_Tp, __vec_ext<_Np>>`. So we cannot directly define `is_simd<simd<_Tp, __vec_ext<_Np>>>` without forward-declare of simd class.





Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144362