From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by sourceware.org (Postfix) with ESMTPS id 49E263858C5E for ; Thu, 23 Feb 2023 15:02:02 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 49E263858C5E Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out2.suse.de (Postfix) with ESMTP id 6101020395; Thu, 23 Feb 2023 15:02:01 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1677164521; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=4466Sa15DKuiJ4dqz8BWz0C4+giW1sugXs1ZdjL9ZFs=; b=t+CZkv95BmhHN1QZ8wFt77dbYMXKDJzZeQAyQh0M8r2ue/BcTeTsmj9Df0Dft2Fp7RWk5Q A8fm8H4RPzGXNO50Kepodep2V33ksgQvhAPr0vM1idik+V2htZ/8zZSsL1WKD0mYEgtGAL 5cC3BMVfPF3z8GGlnUfNlGE7Zlvw0gk= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1677164521; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=4466Sa15DKuiJ4dqz8BWz0C4+giW1sugXs1ZdjL9ZFs=; b=FVqFyXNozYyc9h+krbn1qaZG9IKsHZrscDFy8K7pgDfF1jYcvcp8tb3u+kObjmzx3Nm746 NnXEYbUHUyEiXFAA== Received: from wotan.suse.de (wotan.suse.de [10.160.0.1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id 2A2A62C141; Thu, 23 Feb 2023 15:02:01 +0000 (UTC) Date: Thu, 23 Feb 2023 15:02:01 +0000 (UTC) From: Richard Biener To: Jakub Jelinek cc: Jonathan Wakely , gcc-patches@gcc.gnu.org Subject: Re: [PATCH] Avoid default-initializing auto_vec storage In-Reply-To: Message-ID: References: <20230223125427.DA402139B5@imap2.suse-dmz.suse.de> User-Agent: Alpine 2.22 (LSU 394 2020-01-19) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-5.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Thu, 23 Feb 2023, Jakub Jelinek wrote: > On Thu, Feb 23, 2023 at 01:54:27PM +0100, Richard Biener wrote: > > The following avoids default-initializing auto_vec storage for > > non-POD T since that's not what the allocated storage fallback > > will do and it's also not expected for existing cases like > > > > auto_vec, 64> elts; > > > > which exist to optimize the allocation. > > > > Bootstrapped and tested on x86_64-unknown-linux-gnu. > > > > It saves around 1kB of text size for cc1. > > > > OK for trunk? > > > > Thanks, > > Richard. > > > > * vec.h (auto_vec): Turn m_data storage into > > uninitialized unsigned char. > > Given that we actually never reference the m_data array anywhere, > it is just to reserve space, I think even the alignas(T) there is > useless. The point is that m_auto has as data members: > vec_prefix m_vecpfx; > T m_vecdata[1]; > and we rely on it (admittedly -fstrict-flex-arrays{,=2,=3} or > -fsanitize=bound-sstrict incompatible) being treated as > flexible array member flowing into the m_data storage after it. Doesn't the array otherwise eventually overlap with tail padding in m_auto? Or does an array of T never produce tail padding? > Though, this shows we still have work to do. Because > 1) T m_vecdata[1]; still has element type T in m_auto and > so is actually constructed/destructed uselessly True. > 2) the non-POD support is certainly incomplete; we have > vec_default_construct and vec_copy_construct and use that > for say grow_cleared etc. or vector copies, but don't have > anything that would invoke the destructors and don't invoke > vec_copy_construct (aka placement new) when we just push > stuff into the vector; so, I bet what we do is invalid > and kind of works for non-PODs which have assignment operator > which overwrites everything, doesn't use anything from the > overwritten object and has the same overall effect > as copy constructor, and only support trivial dtors > For full non-POD support, I bet we'd need to vec_copy_construct (..., 1) > on quick_push rather than invoke operator= there, we'd need > to destruct stuff on pop/truncate etc., and quick_grow would need > to be effectively the same as quick_grow_cleared for non-PODs. > As for 1), if even m_vecdata was > alignas(T) unsigned char m_vecdata[sizeof (T)]; > we'll need casts in various spots and it will printing vectors > by hand in gdb when the .gdbinit printers don't work properly. Yes, I'm not proposing to fix non-POD support. I want to make as-if-POD stuff like std::pair to work like it was intended. > Oh, and perhaps we should start marking such spots in GCC with > strict_flex_array attribute to make it clear where we rely on the > non-strict behavior. I think we never access the array directly as array, do we? Richard.