[libcxx-commits] [PATCH] D153319: [libcxx] <experimental/simd> Added aliagned flag types, traits is_simd_flag_type[_v], memory_alignment[_v] and related tests

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added inline comments.


================
Comment at: libcxx/include/experimental/__simd/traits.h:88
+    class _Up = typename _Tp::value_type,
+    class = enable_if_t<(is_simd_v<_Tp> && __is_vectorizable_v<_Up>) || (is_simd_mask_v<_Tp> && is_same_v<_Up, bool>)>>
+struct memory_alignment : integral_constant<size_t, memory_alignment_v<_Tp, _Up>> {};
----------------
Joy12138 wrote:
> philnik wrote:
> > I think this might actually be wrong. The wording says that `memory_alignment` shouldn't contain `value`, not that the class should fail to instantiate.
> I'm a bit confuse about this. If no `class = enable_if_t<...>' here, there will be errors coming out. You can see the same situation in D144363#4497271 with 'simd_size'.
I think we have to do something like
```lang=c++
template <
    class _Tp,
    class _Up = typename _Tp::value_type,
    bool = (is_simd_v<_Tp> && __is_vectorizable_v<_Up>) || (is_simd_mask_v<_Tp> && is_same_v<_Up, bool>)>
struct memory_alignment : integral_constant<size_t, memory_alignment_v<_Tp, _Up>> {};

template <class _Tp, class _Up>
struct memory_alignment<Tp, _Up, false> {};
```

And yeah, this also applies to `simd_size`. The SFINAE tests you added are correct in that they ensure that there is no `value` member. I think the `enable_if` and test for `simd_size_v` can be removed, since they aren't required.

Sorry for not spotting this earlier.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153319