On 3/29/19 9:23 AM, Jakub Jelinek wrote:
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.

memmove needs to be able to work with __last < __first also.

I was getting negative __num and when passed to __builtin_memmove which takes size_t got blowups.

I'm not sure why it worked before. __builtin_memmove will work with __last < __first and sensible positive __num.

When I tried to do what __builtin_memmove or ::memmove must do with unsigned num I would need to branch on __last < __first

and copy backwards.  But pointer comparisons were getting caught as non-constexpr.

I'll look at the __num==0 (noop) testing.


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

I am trying for a C++20-only patch (hoping to get it in for 9) so I used the library function and tested __cplusplus > 201703L.

We could do _GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED and then we could use these for lower version. Maybe stage 1?


        Jakub


Reply via email to