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 A977A385801F for ; Tue, 11 Jan 2022 14:59:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org A977A385801F Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.cz Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.cz Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out2.suse.de (Postfix) with ESMTP id B19D81F3B1; Tue, 11 Jan 2022 14:59:27 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1641913167; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=a0JXTm7X1wL24Y4FMOr/u/xio3HR0KM30r5tetYJQuE=; b=NzObmfJsdXonEJglhFmr6+iLT075tH8xqDXawmImvae1pmQ+VtoCffWWSkPvqoNXv5la8B l75cj1R2DTBB06QpTUFLaBVZ6gpQLk1ug00jvTKcOtG6NC5m93XJqipZ6sypO3o46JwV2D 1k9Tki6IycDjMSZJl12zXFXNInENX9c= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1641913167; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=a0JXTm7X1wL24Y4FMOr/u/xio3HR0KM30r5tetYJQuE=; b=oFsTYhPMScKSRnFvX5PkPXB6fRHknWZ5DzyeOQr3aDTv5amVhZFDjn7556fJF5FAf/kywW huFf7Bar6priVyCw== Received: from suse.cz (virgil.suse.cz [10.100.13.50]) (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 94267A3B84; Tue, 11 Jan 2022 14:59:27 +0000 (UTC) From: Martin Jambor To: Eric Botcazou , gcc-patches@gcc.gnu.org Subject: Re: [patch] Fix reverse SSO issues in IPA-SRA In-Reply-To: <1805634.tdWV9SEqCh@fomalhaut> References: <1805634.tdWV9SEqCh@fomalhaut> User-Agent: Notmuch/0.34.1 (https://notmuchmail.org) Emacs/27.2 (x86_64-suse-linux-gnu) Date: Tue, 11 Jan 2022 15:59:27 +0100 Message-ID: MIME-Version: 1.0 Content-Type: text/plain X-Spam-Status: No, score=-5.2 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.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: Tue, 11 Jan 2022 14:59:29 -0000 Hi Eric, On Tue, Jan 11 2022, Eric Botcazou via Gcc-patches wrote: > Hi, > > we recently received the report that the IPA-SRA pass introduced in GCC 10 > does not always play nice with the reverse scalar storage order that can be > used in structures/records/unions. Reading the code, the pass apparently > correctly detects it but fails to propagate the information to the rewriting > phase in some cases and, in particular, does not stream it for LTO. > > The attached patch is a tentative fix for these issues spotted essentially by > code reading. It also contains various minor tweaks left and right. > > Bootstrapped/regtested on x86-64/Linux, OK for mainline, 11 and 10 branches? > > > 2022-01-11 Eric Botcazou > > * ipa-param-manipulation.c (ipa_dump_adjusted_parameters): Dump > reverse flag as "reverse" for the sake of consistency. > * ipa-sra.c: Fix copyright year. > (ipa_sra_function_summaries::duplicate): Copy the reverse flag. > (dump_isra_access): Remove confusing dump line. > (isra_write_node_summary): Write the reverse flag. > (isra_read_node_info): Read it. > (pull_accesses_from_callee): Copy it. Thanks for the fixes, the forgotten duplication and streaming were quite embarrassing, but one reason is that there are few reverse SSO testcases in the suite. Would it be difficult to add some covering the issues you fixed? Apart from that... > @@ -664,8 +663,6 @@ dump_isra_access (FILE *f, param_access *access) > print_generic_expr (f, access->alias_ptr_type); > if (access->certain) > fprintf (f, ", certain"); > - else > - fprintf (f, ", not-certain"); > if (access->reverse) > fprintf (f, ", reverse"); > fprintf (f, "\n"); ...is this strictly necessary? I know it is inconsistent but the certain flag is fairly special. More importantly... > @@ -3349,6 +3346,7 @@ pull_accesses_from_callee (cgraph_node *caller, isra_param_desc *param_desc, > copy->type = argacc->type; > copy->alias_ptr_type = argacc->alias_ptr_type; > copy->certain = true; > + copy->reverse = argacc->reverse; > vec_safe_push (param_desc->accesses, copy); > } > else if (prop_kinds[j] == ACC_PROP_CERTAIN) ...earlier in the function, there is a check doing: if (argacc->alias_ptr_type != pacc->alias_ptr_type || !types_compatible_p (argacc->type, pacc->type)) return "propagated access types would not match existing ones"; Will the types_compatible_p catch the case when one type is a reverse-SSO and the other is not? If not, we probably need to check that the flags are the same too. Thanks a gain for looking into this. Martin