On Fri, Mar 29, 2019 at 09:10:26AM -0400, Ed Smith-Rowland via gcc-patches 
wrote:
> Greetings,
> 
> This patch implements C++20 constexpr for <algorithm>, <uility>, <array>.
> 
> It's a large patch but only affects C++20 and the volume is mostly test
> cases.
> 
> This differs from the previous patch in actually testing constexpr :-\ and
> in the addition of wrappers for __builtin_memmove and __builtin_memcmp that
> supply constexpr branches if C++20 and is_constant_evaluated().

+    void*
+    __memmove(_Tp* __dst, const _Tp* __src, ptrdiff_t __num)
+    {
+#if __cplusplus > 201703L
+      if (is_constant_evaluated())
+       {
+         for(; __num > 0; --__num)
+           {
+             *__dst = *__src;
+             ++__src;
+             ++__dst;
+           }
+         return __dst;
+       }
+      else if (__num)
+#endif
+       return __builtin_memmove(__dst, __src, sizeof(_Tp) * abs(__num));
+      return __dst;
...
          const ptrdiff_t _Num = __last - __first;
          if (_Num)
-           __builtin_memmove(__result, __first, sizeof(_Tp) * _Num);
+           __memmove(__result, __first, _Num);
...
          const ptrdiff_t _Num = __last - __first;
          if (_Num)
-           __builtin_memmove(__result - _Num, __first, sizeof(_Tp) * _Num);
+           __memmove(__result - _Num, __first, _Num);

Why the abs in there, that is something that wasn't previously there and
if the compiler doesn't figure out that __last >= __first, it would mean
larger emitted code for the non-constexpr case.  As memmove argument is
size_t, wouldn't it be better to make __num just size_t and remove this abs?
Also, wouldn't it be better to have on the other side the __num == 0
handling inside of __memmove, you already have it there for C++2a, but not
for older.  You could then drop the if (_Num) guards around __memmove.

Also, shouldn't the is_constant_evaluated() calls be guarded with
_GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED ?  Without that it won't be
defined...

        Jakub

Reply via email to