From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 33505 invoked by alias); 1 Oct 2019 20:05:16 -0000 Mailing-List: contact libstdc++-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libstdc++-owner@gcc.gnu.org Received: (qmail 33490 invoked by uid 89); 1 Oct 2019 20:05:15 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-11.1 required=5.0 tests=BAYES_00,FREEMAIL_FROM,GIT_PATCH_2,GIT_PATCH_3,KAM_ASCII_DIVIDERS,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy=copie X-HELO: mail-wr1-f68.google.com Received: from mail-wr1-f68.google.com (HELO mail-wr1-f68.google.com) (209.85.221.68) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 01 Oct 2019 20:05:12 +0000 Received: by mail-wr1-f68.google.com with SMTP id a11so17021951wrx.1; Tue, 01 Oct 2019 13:05:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:subject:to:cc:references:message-id:date:user-agent :mime-version:in-reply-to:content-language; bh=7YJUOrZtLLnWe/cHuI8QkfkDfxyOvDooxZgSVhpPufM=; b=g78xQ9XBREJh9Mmgs2KkmvO8txbiPQ3OXUujVem6Ggt5FPXtJ/P7YtR3NQ5ZWKoRwj DGeMEDHwt188srrZOCMnAu5KKzubWpGpJBTYk7jQADn4kSZ3YM3+KxYy41bEl/KHkfdI QyNMPueRIIClAZl5wiPL20ubeybOwWCfc+B3wfIzxjTX1n1fHC49MmQ4GXAxCXl97eUA cYvmKsmWkIcxkWXNi2sMaxfz9+ZISInXrOgmFAw89zEW6V27GflIIqVmIt83//sxhXny KIimM8ZLNUM8JQa2uE0OLmWMJ/Mz3HBX7sBhZwEs+mwsfzv5mhq9OLO3staihzYZT9+q mYEg== Return-Path: Received: from ?IPv6:2a01:e0a:1dc:b1c0:d51c:9fc2:5dd3:a2fd? ([2a01:e0a:1dc:b1c0:d51c:9fc2:5dd3:a2fd]) by smtp.googlemail.com with ESMTPSA id o188sm8634677wma.14.2019.10.01.13.05.07 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 01 Oct 2019 13:05:08 -0700 (PDT) From: =?UTF-8?Q?Fran=c3=a7ois_Dumont?= Subject: Re: [PATCH] Help compiler detect invalid code To: Jonathan Wakely Cc: "libstdc++@gcc.gnu.org" , gcc-patches References: <368143e7-e151-9e37-f055-065044a57e7a@gmail.com> <96b51704-67fe-3ac4-405c-6997151d926c@gmail.com> <20190927112424.GV9487@redhat.com> Message-ID: <829af710-d6e2-5a1f-e80f-dbf3e29d5c2a@gmail.com> Date: Tue, 01 Oct 2019 20:05:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 MIME-Version: 1.0 In-Reply-To: <20190927112424.GV9487@redhat.com> Content-Type: multipart/mixed; boundary="------------9603764D1DA75C6B347A85FB" X-IsSubscribed: yes X-SW-Source: 2019-10/txt/msg00002.txt.bz2 This is a multi-part message in MIME format. --------------9603764D1DA75C6B347A85FB Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-length: 2283 On 9/27/19 1:24 PM, Jonathan Wakely wrote: > On 20/09/19 07:08 +0200, François Dumont wrote: >> I already realized that previous patch will be too controversial to >> be accepted. >> >> In this new version I just implement a real memmove in __memmove so > > A real memmove doesn't just work backwards, it needs to detect any > overlaps and work forwards *or* backwards, as needed. ok, good to know, I understand now why using __builtin_memcopy didn't show any performance enhancement when I tested it ! > > I think your change will break this case: > > #include > > constexpr int f(int i, int j, int k) > { >  int arr[5] = { 0, 0, i, j, k }; >  std::move(arr+2, arr+5, arr); >  return arr[0] + arr[1] + arr[2]; > } > > static_assert( f(1, 2, 3) == 6 ); > > This is valid because std::move only requires that the result iterator > is not in the input range, but it's OK for the two ranges to overlap. > > I haven't tested it, but I think with your change the array will end > up containing {3, 2, 3, 2, 3} instead of {1, 2, 3, 2, 3}. > Indeed, I've added a std::move constexpr test in this new proposal which demonstrate that. C++ Standard clearly states that [copy|move]_backward is done backward. So in this new proposal I propose to add a __memcopy used in copy/move and keep __memmove for *_backward algos. Both are using __builtin_memmove as before.     * include/bits/stl_algobase.h (__memmove): Return void, loop as long as     __n != 0.     (__memcopy): New.     (__copy_move<_IsMove, true, std::random_access_iterator_tag>::__copy_m):     Adapt to use latter.     (__copy_move_backward_a): Remove std::is_constant_evaluated block.     * testsuite/25_algorithms/copy/constexpr.cc (test): Add check on copied     values.     * testsuite/25_algorithms/copy_backward/constexpr.cc (test): Likewise     and rename in test1.     (test2): New.     * testsuite/25_algorithms/copy/constexpr_neg.cc: New.     * testsuite/25_algorithms/copy_backward/constexpr.cc: New.     * testsuite/25_algorithms/equal/constexpr_neg.cc: New.     * testsuite/25_algorithms/move/constexpr.cc: New.     * testsuite/25_algorithms/move/constexpr_neg.cc: New. Tested under Linux x86_64. Ok to commit ? François --------------9603764D1DA75C6B347A85FB Content-Type: text/x-patch; name="constexpr_algo.patch" Content-Transfer-Encoding: 8bit Content-Disposition: attachment; filename="constexpr_algo.patch" Content-length: 13132 Index: include/bits/stl_algobase.h =================================================================== --- include/bits/stl_algobase.h (révision 276259) +++ include/bits/stl_algobase.h (copie de travail) @@ -78,18 +78,18 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION /* - * A constexpr wrapper for __builtin_memmove. + * A constexpr wrapper for memcopy. * @param __num The number of elements of type _Tp (not bytes). */ template _GLIBCXX14_CONSTEXPR - inline void* - __memmove(_Tp* __dst, const _Tp* __src, size_t __num) + inline void + __memcopy(_Tp* __dst, const _Tp* __src, ptrdiff_t __num) { #ifdef __cpp_lib_is_constant_evaluated if (std::is_constant_evaluated()) { - for(; __num > 0; --__num) + for (; __num != 0; --__num) { if constexpr (_IsMove) *__dst = std::move(*__src); @@ -98,15 +98,40 @@ ++__src; ++__dst; } - return __dst; } else #endif - return __builtin_memmove(__dst, __src, sizeof(_Tp) * __num); - return __dst; + __builtin_memmove(__dst, __src, sizeof(_Tp) * __num); } /* + * A constexpr wrapper for memmove. + * @param __num The number of elements of type _Tp (not bytes). + */ + template + _GLIBCXX14_CONSTEXPR + inline void + __memmove(_Tp* __dst, const _Tp* __src, ptrdiff_t __num) + { +#ifdef __cpp_lib_is_constant_evaluated + if (std::is_constant_evaluated()) + { + __dst += __num; + __src += __num; + for (; __num != 0; --__num) + { + if constexpr (_IsMove) + *--__dst = std::move(*--__src); + else + *--__dst = *--__src; + } + } + else +#endif + __builtin_memmove(__dst, __src, sizeof(_Tp) * __num); + } + + /* * A constexpr wrapper for __builtin_memcmp. * @param __num The number of elements of type _Tp (not bytes). */ @@ -446,7 +471,7 @@ #endif const ptrdiff_t _Num = __last - __first; if (_Num) - std::__memmove<_IsMove>(__result, __first, _Num); + std::__memcopy<_IsMove>(__result, __first, _Num); return __result + _Num; } }; @@ -676,12 +701,6 @@ && __is_pointer<_BI2>::__value && __are_same<_ValueType1, _ValueType2>::__value); -#ifdef __cpp_lib_is_constant_evaluated - if (std::is_constant_evaluated()) - return std::__copy_move_backward::__copy_move_b(__first, __last, - __result); -#endif return std::__copy_move_backward<_IsMove, __simple, _Category>::__copy_move_b(__first, __last, Index: testsuite/25_algorithms/copy/constexpr.cc =================================================================== --- testsuite/25_algorithms/copy/constexpr.cc (révision 276259) +++ testsuite/25_algorithms/copy/constexpr.cc (copie de travail) @@ -24,12 +24,12 @@ constexpr bool test() { - constexpr std::array ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}}; + constexpr std::array ca0{{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12}}; std::array ma0{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}}; const auto out6 = std::copy(ca0.begin(), ca0.begin() + 8, ma0.begin() + 2); - return out6 == ma0.begin() + 10; + return out6 == ma0.begin() + 10 && *(ma0.begin() + 2) == 1 && *out6 == 0; } static_assert(test()); Index: testsuite/25_algorithms/copy/constexpr_neg.cc =================================================================== --- testsuite/25_algorithms/copy/constexpr_neg.cc (nonexistent) +++ testsuite/25_algorithms/copy/constexpr_neg.cc (copie de travail) @@ -0,0 +1,38 @@ +// Copyright (C) 2019 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// . + +// { dg-options "-std=gnu++2a" } +// { dg-do compile { target c++2a xfail *-*-* } } + +#include +#include + +constexpr bool +test() +{ + constexpr std::array ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}}; + std::array ma0{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}}; + + const auto out6 = std::copy(ca0.begin() + 8, ca0.begin(), ma0.begin() + 2); + + return out6 == ma0.begin() + 10; +} + +static_assert(test()); // { dg-error "outside the bounds" } + +// { dg-prune-output "non-constant condition" } +// { dg-prune-output "in 'constexpr'" } Index: testsuite/25_algorithms/copy_backward/constexpr.cc =================================================================== --- testsuite/25_algorithms/copy_backward/constexpr.cc (révision 276259) +++ testsuite/25_algorithms/copy_backward/constexpr.cc (copie de travail) @@ -22,15 +22,27 @@ #include constexpr bool -test() +test1() { - constexpr std::array ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}}; + constexpr std::array ca0{{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12}}; std::array ma0{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}}; const auto out7 = std::copy_backward(ca0.begin(), ca0.begin() + 8, ma0.begin() + 10); - return true; + return out7 == ma0.begin() + 2 && *out7 == 1 && *(ma0.begin() + 10) == 0; } -static_assert(test()); +static_assert(test1()); + +constexpr bool +test2() +{ + std::array ma0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}}; + const auto out7 = std::copy_backward(ma0.begin(), ma0.begin() + 8, + ma0.begin() + 10); + + return out7 == ma0.begin() + 2 && *out7 == 0 && *(ma0.begin() + 9) == 7; +} + +static_assert(test2()); Index: testsuite/25_algorithms/copy_backward/constexpr_neg.cc =================================================================== --- testsuite/25_algorithms/copy_backward/constexpr_neg.cc (nonexistent) +++ testsuite/25_algorithms/copy_backward/constexpr_neg.cc (copie de travail) @@ -0,0 +1,39 @@ +// Copyright (C) 2019 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// . + +// { dg-options "-std=gnu++2a" } +// { dg-do compile { target c++2a xfail *-*-* } } + +#include +#include + +constexpr bool +test() +{ + constexpr std::array ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}}; + std::array ma0{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}}; + + const auto out7 = std::copy_backward(ca0.begin() + 8, ca0.begin(), + ma0.begin() + 10); + + return out7 == ma0.begin() + 2; +} + +static_assert(test()); // { dg-error "outside the bounds" } + +// { dg-prune-output "non-constant condition" } +// { dg-prune-output "in 'constexpr'" } Index: testsuite/25_algorithms/equal/constexpr_neg.cc =================================================================== --- testsuite/25_algorithms/equal/constexpr_neg.cc (nonexistent) +++ testsuite/25_algorithms/equal/constexpr_neg.cc (copie de travail) @@ -0,0 +1,49 @@ +// Copyright (C) 2019 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// . + +// { dg-options "-std=gnu++2a" } +// { dg-do compile { target c++2a xfail *-*-* } } + +#include +#include + +constexpr bool +test01() +{ + constexpr std::array ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}}; + constexpr std::array ca1{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}}; + + const auto outa = std::equal(ca0.end(), ca0.begin(), ca1.begin()); + return outa; +} + +static_assert(test01()); // { dg-error "outside the bounds" } + +constexpr bool +test02() +{ + constexpr std::array ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}}; + constexpr std::array ca1{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10}}; + + const auto outa = std::equal(ca0.begin(), ca0.end(), ca1.begin()); + return outa; +} + +static_assert(test02()); // { dg-error "outside the bounds" } + +// { dg-prune-output "non-constant condition" } +// { dg-prune-output "in 'constexpr'" } Index: testsuite/25_algorithms/move/constexpr.cc =================================================================== --- testsuite/25_algorithms/move/constexpr.cc (nonexistent) +++ testsuite/25_algorithms/move/constexpr.cc (copie de travail) @@ -0,0 +1,47 @@ +// Copyright (C) 2019 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// . + +// { dg-options "-std=gnu++2a" } +// { dg-do compile { target c++2a } } + +#include +#include + +constexpr bool +test1() +{ + constexpr std::array ca0{{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12}}; + std::array ma0{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}}; + + const auto out6 = std::move(ca0.begin(), ca0.begin() + 8, ma0.begin() + 2); + + return out6 == ma0.begin() + 10 && *(ma0.begin() + 2) == 1 && *out6 == 0; +} + +static_assert(test1()); + +constexpr bool +test2() +{ + std::array ca0{{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12}}; + + const auto out6 = std::move(ca0.begin() + 2, ca0.begin() + 8, ca0.begin()); + + return out6 == ca0.begin() + 6 && *(ca0.begin()) == 3 && *out6 == 7; +} + +static_assert(test2()); Index: testsuite/25_algorithms/move/constexpr_neg.cc =================================================================== --- testsuite/25_algorithms/move/constexpr_neg.cc (nonexistent) +++ testsuite/25_algorithms/move/constexpr_neg.cc (copie de travail) @@ -0,0 +1,38 @@ +// Copyright (C) 2019 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// . + +// { dg-options "-std=gnu++2a" } +// { dg-do compile { target c++2a xfail *-*-* } } + +#include +#include + +constexpr bool +test() +{ + constexpr std::array ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}}; + std::array ma0{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}}; + + const auto out6 = std::move(ca0.begin() + 8, ca0.begin(), ma0.begin() + 2); + + return out6 == ma0.begin() + 10; +} + +static_assert(test()); // { dg-error "outside the bounds" } + +// { dg-prune-output "non-constant condition" } +// { dg-prune-output "in 'constexpr'" } --------------9603764D1DA75C6B347A85FB--