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

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added a comment.

In D144362#4389779 <https://reviews.llvm.org/D144362#4389779>, @Joy12138 wrote:

> I encountered a problem in subsequent development that currently seems to need to be resolved in this patch.
>
> In the previous comment, you suggested that I place all simd traits in a separate header file `traits.h`. During the process of adding the rest traits implementations, I found that most of them require the use of other interfaces placed in the `simd` header, similar to `is_simd` requires the use of `simd` classes. Therefore, I added `declaration.h` as forward declarations. However, there are too many interfaces that need to be referenced in the future, which is not suitable to add forward declarations for all.
>
> I think there are two solutions. 
> One is to keep all external user interfaces under the `simd` header, just like our previous implementation. 
> Another  is to categorize all external user interfaces into separate header files. No implementations in the `simd` header and only including other header files in order. Then we may need to add `traits.h`, `simd.h`, `simd_mask.h`, `aligned_tag.h`, `abi_tag.h`, `simd_casts.h`, `where_expression.h`, etc.
> Which one do you prefer?

I think it's easier to gollow the code when splitting up the headers, so I'd go for option 2.



================
Comment at: libcxx/test/std/experimental/simd/test_utils.h:20-21
+
+using integral_no_bool_types =
+    types::concatenate_t<types::character_types, types::signed_integer_types, types::unsigned_integer_types>;
+using arithmetic_no_bool_types = types::concatenate_t<integral_no_bool_types, types::floating_point_types>;
----------------
Joy12138 wrote:
> philnik wrote:
> > This is just `integer_types`, right?
> No, it is not the same.
> 
> The first template parameter of the `simd` class does not accept input of bool type. And the vector type corresponding to bool type is defined by `simd_mask` class.
> 
> In "type_algorithms.h", `integer_types` has the following definition:
> 
> ```
> // libcxx/test/support/type_algorithms.h:98
> using integral_types = concatenate_t<character_types, signed_integer_types, unsigned_integer_types, type_list<bool> >;
> ```
> It contains type bool. And we need arithmetic types without bool to test class `simd`.
> 
You show the implementation of `integral_types`, not `integer_types` ;-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144362