[libcxx-commits] [PATCH] D144364: [libcxx] <experimental/simd> Added internal storage type, constructors, subscript operators of class simd/simd_mask and related tests

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added a comment.

I mainly look quickly over the patch, can you get the patch to apply properly so the CI can test it?



================
Comment at: libcxx/include/experimental/__simd/utility.h:76
+template <class _Tp>
+auto __choose_mask_type() {
+  if constexpr (sizeof(_Tp) == 1) {
----------------
This change needs to be done at other functions too.


================
Comment at: libcxx/include/experimental/__simd/utility.h:90
+  }
+#endif
+}
----------------
Let's make sure unsupported sizes are ill-formed instead of triggering UB.


================
Comment at: libcxx/include/experimental/__simd/utility.h:98
+  for (unsigned long __i = 0; __i < __CHAR_BIT__ * sizeof(_Tp); __i++)
+    __res |= static_cast<_Up>(__v) << __i;
+  return __res;
----------------
Will the compiler optimize this?
Since a bool has two values it might otherwise be better to have a constexpr function to generate the result at compile time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144364