mclow.lists added a comment. I'm going to stop here, because all the things I've noted are ticky-tack; formatting and minor changes. More substantial comments coming soon.
================ Comment at: libcxx/include/experimental/simd:40 +template <class T> inline constexpr bool is_abi_tag_v = is_abi_tag<T>::value; +template <class T> struct is_simd; +template <class T> inline constexpr bool is_simd_v = is_simd<T>::value; ---------------- A blank line between the separate groups (as in the paper) will make this much easier to read ================ Comment at: libcxx/include/experimental/simd:67 +template <class T, class U, class Abi> see below simd_cast(const simd<U, Abi>&); + +template <class T, class U, class Abi> see below static_simd_cast(const simd<U, Abi>&); ---------------- And here you have an extra line - go figure. ================ Comment at: libcxx/include/experimental/simd:113 +template <class T, class Abi> int find_last_set(const simd_mask<T, Abi>&); +bool all_of(see below ) noexcept; +bool any_of(see below ) noexcept; ---------------- blank line, extra space `(see below )` should be `(see below)` ================ Comment at: libcxx/include/experimental/simd:144 +const_where_expression<simd_mask<T, Abi>, const simd_mask<T, Abi>> +where(const nodeduce_t<simd_mask<T, Abi>>&, const simd_mask<T, Abi>&) noexcept; + ---------------- This is better formatting than in the paper :-) ================ Comment at: libcxx/include/experimental/simd:316 +template <class T, class V> using samesize = fixed_size_simd<T, V::size()>; // exposition only +template <class Abi> floatv<Abi> acos(floatv<Abi> x); +template <class Abi> doublev<Abi> acos(doublev<Abi> x); ---------------- These would be much easier to read lined up, and with blank lines. ``` template <class Abi> floatv<Abi> acos(floatv<Abi> x); template <class Abi> doublev<Abi> acos(doublev<Abi> x); template <class Abi> ldoublev<Abi> acos(ldoublev<Abi> x); template <class Abi> floatv<Abi> asin(floatv<Abi> x); template <class Abi> doublev<Abi> asin(doublev<Abi> x); template <class Abi> ldoublev<Abi> asin(ldoublev<Abi> x); ``` ================ Comment at: libcxx/include/experimental/simd:626 +constexpr auto __is_non_narrowing_convertible_impl(_From a [[gnu::unused]]) + -> decltype(_To{a}, true) { + return true; ---------------- Do you need the trailing return type here? instead of something like `decltype(_to{declval<_From>()})` Then you don't need to name `a`, and can get rid of the (gcc-specific) attribute. ================ Comment at: libcxx/include/experimental/simd:653 +constexpr _Tp __variadic_sum() { + return {}; +} ---------------- This won't work if `_Tp` has an explicit default ctor (which is probably not true for anything in SIMD). Do you lose anything by writing `return _Tp{}` ? ================ Comment at: libcxx/include/experimental/simd:669 + +#if _LIBCPP_STD_VER > 14 && !defined(_LIBCPP_HAS_NO_VARIABLE_TEMPLATES) +template <class _Tp> ---------------- Isn't the parallelism TS based on C++17? https://reviews.llvm.org/D41148 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits