From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-il1-x144.google.com (mail-il1-x144.google.com [IPv6:2607:f8b0:4864:20::144]) by sourceware.org (Postfix) with ESMTPS id 0741C385DC0F for ; Tue, 7 Apr 2020 15:17:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 0741C385DC0F Received: by mail-il1-x144.google.com with SMTP id t6so3535429ilj.8 for ; Tue, 07 Apr 2020 08:17:08 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=gU0hQbzRI4gthAywX00Qx8m5C4rAAIl+8o4bjG08ZVE=; b=HWnzUD4n+jxUKGOk76OBsunSf/Z6iu2vI+EGDJ66W6OsucNxaeLXMx8J37/MPXNgRK Pixa+NT0Ti3Z9xwDEdO2MtGSzXgL4ETznLq9nchHq+WD0MpWvLkzk2lcVuCcQUcbiHEq 7Kln9K+X6XWdbB3T8pcXsYpDt/avwjjxM8KuHvSuDHT0iMYJu1v4CA/wu8pZTN6J72/c C2iJTFIhLwp7DJJkYRS74WComnwSTORN82cT2TRiy1Cgsdvb5W9R2Jc9bcACX55If6nq mT+9n9nBhMxbZvjmFchpEY+XMf9fdgOKqA4L7MxSfDIeudFvxhPRZ65ESLbo6KifW/65 UTIQ== X-Gm-Message-State: AGi0Pua1KCO+wnq1imTmvnScQx4dOZndp1n0DEe/8coGxm7lShEQ1cGk +O0scvVSWATnYrR/7DJB04i5WUoOd2+hGhDzPIQ= X-Google-Smtp-Source: APiQypI9cd2brno+GRBXYfIdc0ECa+sOBQoTxHN+VTpacZp0duyrENds8yHreKaAz/BbsvLp7j2rP7txL84OHCYtG4s= X-Received: by 2002:a92:5ccd:: with SMTP id d74mr2770987ilg.59.1586272626498; Tue, 07 Apr 2020 08:17:06 -0700 (PDT) MIME-Version: 1.0 References: <20200331122907.GB62067@kam.mff.cuni.cz> <65230a52-c025-a6e3-0d31-409d37e9b2c9@suse.cz> <20200403152609.GA35629@kam.mff.cuni.cz> <0dbc191e-66f7-9878-956d-96149f20f5bf@suse.cz> In-Reply-To: From: Jonathan Wakely Date: Tue, 7 Apr 2020 15:16:55 +0000 Message-ID: Subject: Re: [PATCH] Check DECL_CONTEXT of new/delete operators. To: Richard Biener Cc: Nathan Sidwell , =?UTF-8?Q?Martin_Li=C5=A1ka?= , Jan Hubicka , GCC Patches , Marc Glisse , Jason Merrill Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-2.2 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, 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: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 07 Apr 2020 15:17:09 -0000 On Tue, 7 Apr 2020 at 12:57, Richard Biener wrote: > > On Tue, Apr 7, 2020 at 1:46 PM Jonathan Wakely wrote: > > > > On Tue, 7 Apr 2020 at 12:40, Richard Biener wrote: > > > > > > On Tue, Apr 7, 2020 at 1:30 PM Jonathan Wakely wrote: > > > > > > > > On Mon, 6 Apr 2020 at 13:45, Nathan Sidwell wrote: > > > > > The both operator new and operator delete are looked up in the same > > > > > manner. The std does not require a 'matching pair' be found. but it is > > > > > extremely poor form for a class to declare exactly one of operator > > > > > {new,delete}. > > > > > > > > There are unfortunately several such example in the standard! > > > > > > > > I wonder how much benefit we will really get from trying to make this > > > > optimisation too general. > > > > > > > > Just eliding (or coalescing) implicit calls to ::operator new(size_t) > > > > and ::operator delete(void*, size_t) (and the [] and align_val_t forms > > > > of those) probably covers 99% of real cases. > > > > > > IIRC the only reason to have the optimization was to fully elide > > > STL containers when possible. That brings in allocators and > > > thus non ::new/::delete allocations. > > > > But the vast majority of containers are used with std::allocator, and > > we control that. > > > > Wouldn't it be simpler to add __builtin_operator_new and > > __builtin_operator_delete like clang has, then make std::allocator use > > those, and limit the optimizations to uses of those built-ins? > > Sure, that's a viable implementation strathegy. Another might be > > void delete (void *) __attribute__((matching_new(somewhere::new))); > > and thus allow the user to relate a new/delete pair (here the FE would > do lookup for 'new' and record for example the mangled assembler name). Does that attribute tell us anything useful? Given a pointer obtained from new and passed to delete, can't we assume they are a matching pair? If not, the behaviour would be undefined anyway. > That is, we usually try to design a mechanism that's more broadly usable. > But yes, we match malloc/free so matching ::new/::delete by aliasing them > to __builtin_operator_new and __builtin_operator_delete is fair enough. Would it make sense to annotate the actual calls to the allocation/deallocation functions, instead of the declarations of those functions? According to Richard Smith, we don't want to optimise away 'operator delete(operator new(1), 1)' because that's an explicit call, and the user might have replaced those functions and be relying on the side effects. But we can optimise away 'delete new char' and we can optimise away std::allocator().deallocate(std::allocator().allocate(1), 1). So what matters is not whether the functions being called match up (they have to anyway) or which functions are being called. What matters is whether they are called implicitly (by a new-expression or by std::allocator). So when the compiler expands 'new T' into a call to operator new followed by constructing a T (plus exception handling) the call to operator new would be marked as optimisable by the FE, and likewise when the compiler expands 'delete p' into a destructor followed by calling operator delete, the call to operator delete would be marked as optimisable. If a pointer is allocated by a call marked optimisable and deallocated by a call marked optimisable, it can be optimised away (or coalesced with another optimisation). Then for std::allocator we just want to be able to mark the explicit calls to operator new and operator delete as "optimisable", as the FE does for the implicit calls it generates. So if we want a general purpose utility, it would be an attribute to mark /calls/ as optimisable, not functions. Adding __builtin_operator_new would just be a different syntax for "call operator new but mark it optimisable". N.B. I am not a compiler dev and might be talking nonsense :-)