[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 accepted this revision.
philnik added a comment.
This revision is now accepted and ready to land.

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

> In D144363#4491370 <https://reviews.llvm.org/D144363#4491370>, @philnik wrote:
>
>> I think you did something wrong. You shouldn't need a `.verify.cpp` for SFINAE testing, and looking at the error message the types don't SFINAE away.
>>
>> Something like
>>
>>   template <class T, class U, class = void>
>>   struct check_sfinae : false_type {};
>>   
>>   template <class T, class U>
>>   struct check_sfinae<void_t<ex::simd_size<T, U>>> : true_type {};
>>   
>>   static_assert(!check_sfinae<int , ex::native_simd<int>>::value);
>>
>> should work just fine in a normal `.pass.cpp`. If you get an error, either I did something wrong in the SFINAE test, or the implementation of `ex::simd_size` is wrong (because it doesn't SFINAE away).
>>
>> I'm pretty sure that variable templates can't SFINAE away.
>
> @philnik Hi, we rewrite the SFINAE test as you given style. It seems no errors come out. Does it mean our implementations right?

Looks correct to me. TIL that variable templates can SFINAE away. I think we are missing some test coverage elsewhere...

LGTM % comment. Thanks! I'm really looking forward to having a great `<experimental/simd>` implementation.



================
Comment at: libcxx/test/std/experimental/simd/simd.traits/simd_size.pass.cpp:58-62
+template <class T, class U, class = void>
+struct has_simd_size : std::false_type {};
+
+template <class T, class U>
+struct has_simd_size<T, U, std::void_t<decltype(ex::simd_size_v<T, U>)>> : std::true_type {};
----------------
Please also add tests for `simd_size`, and not just for `simd_size_v`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144363