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 5C72F385840F for ; Thu, 14 Oct 2021 09:06:15 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 5C72F385840F 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-594-0NA_9AQOOC660Ol4KPfy9w-1; Thu, 14 Oct 2021 05:06:12 -0400 X-MC-Unique: 0NA_9AQOOC660Ol4KPfy9w-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 215C6100CFAF; Thu, 14 Oct 2021 09:06:11 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.39.193.172]) by smtp.corp.redhat.com (Postfix) with ESMTPS id AFCA619724; Thu, 14 Oct 2021 09:06:10 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.16.1/8.16.1) with ESMTPS id 19E967Yr2769767 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Thu, 14 Oct 2021 11:06:08 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.16.1/8.16.1/Submit) id 19E9660b2769766; Thu, 14 Oct 2021 11:06:06 +0200 Date: Thu, 14 Oct 2021 11:06:06 +0200 From: Jakub Jelinek To: Qing Zhao Cc: richard Biener , gcc-patches Nick Alcock via Subject: Re: [RFC][patch][PR102281]Clear padding for variables that are in registers Message-ID: <20211014090606.GS304296@tucnak> Reply-To: Jakub Jelinek References: <354CF088-F596-4824-BB2D-62FB2D123D50@oracle.com> MIME-Version: 1.0 In-Reply-To: <354CF088-F596-4824-BB2D-62FB2D123D50@oracle.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-5.5 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, KAM_SHORT, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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: Thu, 14 Oct 2021 09:06:16 -0000 On Tue, Oct 12, 2021 at 10:51:45PM +0000, Qing Zhao wrote: > PR10228 1https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102281 > Exposed an issue in the current implementation of the padding initialization of -ftrivial-auto-var-init. > > Currently, we add __builtin_clear_padding call _AFTER_ every explicit initialization of an auto variable: > > var_decl = {init_constructor}; > __builtin_clear_padding (&var_decl, 0B, 1); > > the reason I added the call to EVERY auto variable that has explicit initialization is, the folding of __builtin_clear_padding will automatically turn this call to a NOP when there is no padding in the variable. So, we don't need to check whether the variable has padding explicitly. > > However, always adding the call to __builtin_clear_padding (&var_decl,…) might introduce invalid IR when VAR_DECL cannot be addressable. > > In order to resolve this issue, I propose the following solution: > > Instead of adding the call to __builtin_clear_padding _AFTER_ the explicit initialization, Using zero to initialize the whole variable BEFORE explicit fields initialization when VAR_DECL has padding, i.e: > > If (had_padding_p (var_decl)) > var_decl = ZERO; > var_decl = {init_constructor}; > > This should resolve the invalid IR issue. However, there might be more run time overhead from such padding initialization since the whole variable is set to zero instead of only the paddings. > > Please let me know you comments on this. As I've tried to explain in the PR, this is wrong. It makes no sense whatsoever to clear padding on is_gimple_reg variables even if they do have padding (e.g. long double/_Complex long double), because all the GIMPLE passes will just not care about it. If you want to clear padding for those, it needs to be done where it is forced into memory (e.g. expansion for returning or passing such types in memory if they aren't passed in registers, or RA when the register allocator decides to spill them into memory if even that is what you want to cover). For !is_gimple_reg vars, yes, something like clear_padding_type_has_padding_p could be useful to avoid unnecessary IL growth, but it should be implemented more efficiently, in particular for the answer to a question will __builtin_clear_padding emit any code on this type at least for non-unions there is no buffer covering the whole type needed, just next to clear_in_mask bool add another bool for the new mode and in clear_padding_flush in that mode just set another bool if clear_in_mask mode would clear any bit in the mask if it was there. For unions, I'm afraid it needs to do it the clear_in_mask way and at the end analyze the buf. Also, besides answer to the question "would __builtin_clear_padding emit any code on this type" we should have another function/another mode which returns true if any padding whatsoever is found, including unions. For non-union it would be exactly the same thing, but in unions it would keep using the same mode too, even if just one union member has any padding return it. And, for these modes, there could be shortcuts, once we find some padding, it doesn't make sense to walk further fields. Jakub