From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 90050 invoked by alias); 1 Aug 2018 15:05:08 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 90030 invoked by uid 89); 1 Aug 2018 15:05:07 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.0 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY autolearn=no version=3.3.2 spammy= X-HELO: mail2-relais-roc.national.inria.fr Received: from mail2-relais-roc.national.inria.fr (HELO mail2-relais-roc.national.inria.fr) (192.134.164.83) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 01 Aug 2018 15:05:02 +0000 Received: from ip-209.net-89-2-64.rev.numericable.fr (HELO stedding) ([89.2.64.209]) by mail2-relais-roc.national.inria.fr with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 01 Aug 2018 17:04:59 +0200 Date: Wed, 01 Aug 2018 15:05:00 -0000 From: Marc Glisse To: =?ISO-8859-15?Q?Martin_Li=A8ka?= cc: gcc-patches@gcc.gnu.org, Jan Hubicka , Nathan Sidwell Subject: Re: [PATCH] Add malloc predictor (PR middle-end/83023). In-Reply-To: Message-ID: References: User-Agent: Alpine 2.21 (DEB 202 2017-01-01) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8BIT X-SW-Source: 2018-08/txt/msg00106.txt.bz2 On Wed, 1 Aug 2018, Martin Liška wrote: > On 08/01/2018 02:25 PM, Marc Glisse wrote: >> On Wed, 1 Aug 2018, Martin Liška wrote: >> >>> On 07/27/2018 02:38 PM, Marc Glisse wrote: >>>> On Fri, 27 Jul 2018, Martin Liška wrote: >>>> >>>>> So answer is yes, the builtin can be then removed. >>>> >>>> Good, thanks. While looking at how widely it is going to apply, I noticed that the default, throwing operator new has attribute malloc and everything, but the non-throwing variant declared in doesn't, so it won't benefit from the new predictor. I don't know if there is a good reason for this disparity... >>>> >>> >>> Well in case somebody uses operator new: >>> >>>     int* p1 = new int; >>>     if (p1) >>>      delete p1; >>> >>> we optimize out that to if (true), even when one has used defined >>> operator new. Thus it's probably OK. >> >> Throwing new is returns_nonnull (errors are reported with exceptions) so that's fine, but non-throwing new is not: >> >> int* p1 = new(std::nothrow) int; >> >> Here errors are reported by returning 0, so it is common to test if p1 is 0 and this is precisely the case that could benefit from a predictor but does not have the attribute to do so (there are also consequences on aliasing). > > Then it can be handled with DECL_IS_OPERATOR_NEW, for those we can also set the newly introduced predictor. Independently of whether you extend the predictor to DECL_IS_OPERATOR_NEW, it would be good for this nothrow operator new to get the aliasing benefits of attribute malloc. I'll open a PR. >> (Jan's remark about functions with an inferred malloc attribute reminds me that at some point, the code was adding attribute malloc for functions that always return 0...) > > By inferred do you mean function that are marked as malloc in IPA pure-const (propagate_malloc)? Yes. > Example would be appreciated. I used the past tense, I am not claiming this still happens. -- Marc Glisse