From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id 573833839C7C for ; Thu, 3 Jun 2021 12:31:33 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 573833839C7C Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-274-8raFg2YyPFaMKyxek_Kr3g-1; Thu, 03 Jun 2021 08:31:29 -0400 X-MC-Unique: 8raFg2YyPFaMKyxek_Kr3g-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id CE838802B78; Thu, 3 Jun 2021 12:31:28 +0000 (UTC) Received: from localhost (unknown [10.33.37.1]) by smtp.corp.redhat.com (Postfix) with ESMTP id 683415D9F0; Thu, 3 Jun 2021 12:31:28 +0000 (UTC) Date: Thu, 3 Jun 2021 13:31:27 +0100 From: Jonathan Wakely To: =?iso-8859-1?Q?Fran=E7ois?= Dumont Cc: "libstdc++@gcc.gnu.org" , gcc-patches Subject: Re: [PATCH] Use _GLIBCXX_ASSERTIONS as _GLIBCXX_DEBUG light Message-ID: References: <6ccc9e9e-af8b-fd06-4ad3-4bc021aa364e@gmail.com> MIME-Version: 1.0 In-Reply-To: <6ccc9e9e-af8b-fd06-4ad3-4bc021aa364e@gmail.com> X-Clacks-Overhead: GNU Terry Pratchett X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=iso-8859-1; format=flowed Content-Disposition: inline Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-12.5 required=5.0 tests=BAYES_00, BODY_8BITS, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=unavailable autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libstdc++@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libstdc++ mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 03 Jun 2021 12:31:34 -0000 On 27/05/21 19:37 +0200, François Dumont via Libstdc++ wrote: >We have been talking for a long time of a debug mode with less impact >on performances. We already have it, that's what _GLIBCXX_ASSERTIONS already is :-) >I propose to simply use the existing _GLIBCXX_ASSERTIONS macro. > >    libstdc++: [_GLIBCXX_ASSERTIONS] Activate basic debug checks > >    Use _GLIBCXX_ASSERTIONS as a _GLIBCXX_DEBUG light mode. When >defined it activates >    all _GLIBCXX_DEBUG checks but skipping those requiring to loop >through the iterator >    range unless in case of constexpr. > >    libstdc++-v3/ChangeLog: > >            * include/debug/debug.h [_GLIBCXX_ASSERTIONS]: Define >debug macros non-empty. >            * include/debug/helper_functions.h: Cleanup comment about >removed _Iter_base. >            * include/debug/functions.h (__skip_debug_runtime_check): >New, returns false if >            _GLIBCXX_DEBUG is defined or if constant evaluated. >            (__check_sorted, __check_partitioned_lower, >__check_partitioned_upper): Use latter. > >Tested under Linux x64. > >Ok to commit ? > >François > >diff --git a/libstdc++-v3/include/debug/debug.h b/libstdc++-v3/include/debug/debug.h >index 116f2f023e2..2e6ce1c8a93 100644 >--- a/libstdc++-v3/include/debug/debug.h >+++ b/libstdc++-v3/include/debug/debug.h >@@ -61,7 +61,7 @@ namespace __gnu_debug > struct _Safe_iterator; > } > >-#ifndef _GLIBCXX_DEBUG >+#ifndef _GLIBCXX_ASSERTIONS > > # define __glibcxx_requires_cond(_Cond,_Msg) > # define __glibcxx_requires_valid_range(_First,_Last) >diff --git a/libstdc++-v3/include/debug/functions.h b/libstdc++-v3/include/debug/functions.h >index 6cac11f2abd..ee0eb877568 100644 >--- a/libstdc++-v3/include/debug/functions.h >+++ b/libstdc++-v3/include/debug/functions.h >@@ -48,6 +48,25 @@ namespace __gnu_debug > template > struct _Is_contiguous_sequence : std::__false_type { }; > >+ _GLIBCXX20_CONSTEXPR Should this be simply _GLIBCXX_CONSTEXPR so that it can be constexpr in C++14 mode too? Or are there are never any debug checks in functions that are already constexpr in C++14 or C++17? >+ inline bool >+ __skip_debug_runtime_check() >+ { >+ // We could be here while only _GLIBCXX_ASSERTIONS has been defined. >+ // In this case we skip expensive runtime checks, constexpr will still >+ // be checked. >+ return >+#ifndef _GLIBCXX_DEBUG >+# if _GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED >+ !__builtin_is_constant_evaluated(); >+# else >+ true; >+# endif >+#else >+ false; >+#endif I think this would be simpler without the nesting, and without the preprocessor checks halfway through the return statement: #ifdef _GLIBCXX_DEBUG return false; #elif _GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED return !__builtin_is_constant_evaluated(); #else return true; #endif >+ } >+ > /* Checks that [first, last) is a valid range, and then returns > * __first. This routine is useful when we can't use a separate > * assertion statement because, e.g., we are in a constructor. >@@ -260,8 +279,9 @@ namespace __gnu_debug > inline bool > __check_sorted(const _InputIterator& __first, const _InputIterator& __last) > { >- return __check_sorted_aux(__first, __last, >- std::__iterator_category(__first)); >+ return __skip_debug_runtime_check() >+ || __check_sorted_aux(__first, __last, >+ std::__iterator_category(__first)); Currently this function is never called at all ifndef _GLIBCXX_DEBUG. With this change, it's going to be present for _GLIBCXX_ASSERTIONS, and if it isn't inlined it's going to explode the code size. Some linux distros are already building the entire distro with _GLIBCXX_ASSERTIONS so I think we need to be quite careful about this kind of large change affecting every algo. So maybe we shouldn't enable these checks via _GLIBCXX_ASSERTIONS, but a new macro.