[libcxx-commits] [PATCH] D144363: [libcxx] <experimental/simd> Added simd width functions, simd_size traits and related tests

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added a comment.

In D144363#4504748 <https://reviews.llvm.org/D144363#4504748>, @Joy12138 wrote:

> @philnik Hi! We found that the current implementation might have an unexpected behavior. Because we do not check the template parameter of `simd_size_v`, it gives value with invalid first parameter.
>
> https://godbolt.org/z/o8o4fWe7n
>
> In the document, `simd_size` is implemented first and using `simd_size_v` as value of `simd_size`. Therefore, the template parameters of `simd_size_v` are checked by `simd_size`. Here is the behavior with GCC implementation: https://godbolt.org/z/1vYTsnbsd
>
> I implement in the current form because you told me earlier that implementing variable first can avoid instantiating a class every time. But it seems to have some minor issues at the moment.
>
> By the way, `memory_alignment_v` will also have the same issue. Do you have any suggestions for solving this?

Good catch! I'm actually learning some really interesting things about template variables and how they interact with things. I think the easiest solution for now is to use the class to implement the `_v` version. It's probably not worth the effort to try to avoid the instantiation. That's more of a nice to have than an absolute need to have. Please also add a test for this. Something like

name=simd_size_v.verify.cpp
  static_assert(!std::experimental::simd_size_v<...>); // expected-error {{no member named 'value'}}

should be good enough.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144363