From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ej1-x631.google.com (mail-ej1-x631.google.com [IPv6:2a00:1450:4864:20::631]) by sourceware.org (Postfix) with ESMTPS id E38653857828; Mon, 7 Jun 2021 04:25:24 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org E38653857828 Received: by mail-ej1-x631.google.com with SMTP id h24so24447632ejy.2; Sun, 06 Jun 2021 21:25:24 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding :content-language; bh=gmrhBPgoHgK86CbdkMKie8BbU1u5FdzakEnkQvMLSlA=; b=CBkLC/I9F5ElFbCGZ82JLgExz14Y8mgSoFNNpdQ6e8ht30tuI8VWmjSTrXZYbhuQxx IHByAmru+DzMhrJtfzN9/3vwTtREkS5RP1D3UIQMV7VyZIQKLEoHmLM8FYHsludKea8+ /UHC+xsPhUEbMoFl+nUcFpCJ9BZMu7ZA6htUecJIpAwlwlHi0GcqVjIaFWgbgBgzf7t0 dBnaZp7my51Wbs3IoYXYO5hW3ofXEXeBC7CGFwGtysnxalAjBbC3M0FomwlHtt/W3Dd8 f0v43lUUY1Q2Z8016QkzQzM8569omSHYSuFAUmVohjvLpyOUD+XcFlb1VUazZN1MZacg 3fAA== X-Gm-Message-State: AOAM53298/stLoQtttf6d+HHZ15PPSMbeEAUTBYwvGORbHLtPwBH1Fqt bbpDhdJ6a1HbuVPS0vCmZ0ry2aoUK+VFfA== X-Google-Smtp-Source: ABdhPJxOdrM4WZWOAANdfBl1yksxkjg52sbLSve/ffIqBEZeiB6nqVldp0w0kfotwXLaNxmt/uz26Q== X-Received: by 2002:a17:906:490:: with SMTP id f16mr16399021eja.541.1623039923521; Sun, 06 Jun 2021 21:25:23 -0700 (PDT) Received: from [10.58.4.115] ([109.190.253.15]) by smtp.googlemail.com with ESMTPSA id v23sm7220228eds.25.2021.06.06.21.25.22 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sun, 06 Jun 2021 21:25:22 -0700 (PDT) Subject: Re: [PATCH] Use _GLIBCXX_ASSERTIONS as _GLIBCXX_DEBUG light To: Jonathan Wakely Cc: "libstdc++@gcc.gnu.org" , gcc-patches References: <6ccc9e9e-af8b-fd06-4ad3-4bc021aa364e@gmail.com> From: =?UTF-8?Q?Fran=c3=a7ois_Dumont?= Message-ID: Date: Mon, 7 Jun 2021 06:25:21 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: fr X-Spam-Status: No, score=-8.3 required=5.0 tests=BAYES_00, BODY_8BITS, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, NICE_REPLY_A, RCVD_IN_BARRACUDACENTRAL, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham 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: Mon, 07 Jun 2021 04:25:26 -0000 On 03/06/21 2:31 pm, Jonathan Wakely wrote: > 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. > _GLIBCXX_DEBUG is already rarely used, so will be yet another mode. So let's forget about all this, thanks. François