From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out2.suse.de (smtp-out2.suse.de [IPv6:2001:67c:2178:6::1d]) by sourceware.org (Postfix) with ESMTPS id 600C43844695 for ; Wed, 14 Dec 2022 13:20:22 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 600C43844695 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.cz Authentication-Results: sourceware.org; spf=fail smtp.mailfrom=suse.cz Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id 98D3C1FFEC; Wed, 14 Dec 2022 13:20:21 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1671024021; 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=jQCWY8hzYfI5Rwyvuzajud+p6v87h+TnBPSdjOuAOGg=; b=pLD3Jp15di4Re5okryXiUQblsw/hcHaUNGJwVQwJt1cktlYEV52saaOioGEcQGGSrGGCDY 0iXpSPPgt7OauDX8Z+C534266wbmXw0NYGPfmx0r6ut5s0w/1JIzgnBG45mukDOaOxU3qb UsMiELa8JaW61lM+aShUb+NiA+8Nm78= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1671024021; 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=jQCWY8hzYfI5Rwyvuzajud+p6v87h+TnBPSdjOuAOGg=; b=TUquibaqFTohygYGnZ4TvVn18cc75WbTYNtFdLbPD4yQNzimHE38wrgrJhzp+RISQwvmdd OMRO7JommXVQyCDQ== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 8C67C1333E; Wed, 14 Dec 2022 13:20:21 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id 3Zc9IpXNmWOUAQAAMHmgww (envelope-from ); Wed, 14 Dec 2022 13:20:21 +0000 From: Martin Jambor To: Richard Biener , Jan Hubicka Cc: GCC Patches Subject: Re: [PATCH 8/9] ipa-sra: Make scan_expr_access bail out on uninteresting expressions In-Reply-To: References: User-Agent: Notmuch/0.37 (https://notmuchmail.org) Emacs/28.1 (x86_64-suse-linux-gnu) Date: Wed, 14 Dec 2022 14:20:21 +0100 Message-ID: MIME-Version: 1.0 Content-Type: text/plain X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,SPF_HELO_NONE,SPF_SOFTFAIL,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: Hi, On Tue, Dec 13 2022, Richard Biener wrote: > On Mon, 12 Dec 2022, Jan Hubicka wrote: > >> > > Hi, >> > > >> > > I'm re-posting patches which I have posted at the end of stage 1 but >> > > which have not passed review yet. >> > > >> > > 8<-------------------------------------------------------------------- >> > > >> > > I have noticed that scan_expr_access passes all the expressions it >> > > gets to get_ref_base_and_extent even when we are really only >> > > interested in memory accesses. So bail out when the expression is >> > > something clearly uninteresting. >> > > >> > > Bootstrapped and tested individually when I originally posted it and >> > > now bootstrapped and LTO-bootstrapped and tested as part of the whole >> > > series. OK for master? >> > > >> > > >> > > gcc/ChangeLog: >> > > >> > > 2021-12-14 Martin Jambor >> > > >> > > * ipa-sra.c (scan_expr_access): Bail out early if expr is something we >> > > clearly do not need to pass to get_ref_base_and_extent. >> > > --- >> > > gcc/ipa-sra.cc | 5 +++++ >> > > 1 file changed, 5 insertions(+) >> > > >> > > diff --git a/gcc/ipa-sra.cc b/gcc/ipa-sra.cc >> > > index 93fceeafc73..3646d71468c 100644 >> > > --- a/gcc/ipa-sra.cc >> > > +++ b/gcc/ipa-sra.cc >> > > @@ -1748,6 +1748,11 @@ scan_expr_access (tree expr, gimple *stmt, isra_scan_context ctx, >> > > || TREE_CODE (expr) == REALPART_EXPR) >> > > expr = TREE_OPERAND (expr, 0); >> > > >> > > + if (!handled_component_p (expr) >> > > + && !DECL_P (expr) >> > > + && TREE_CODE (expr) != MEM_REF) >> > > + return; >> > Is this needed because get_ref_base_and_extend crashes if given SSA_NAME >> > or something else or is it just optimization? >> > Perhaps Richi will know if there is better test for this. > > Also the code preceeding the above > > if (TREE_CODE (expr) == BIT_FIELD_REF > || TREE_CODE (expr) == IMAGPART_EXPR > || TREE_CODE (expr) == REALPART_EXPR) > expr = TREE_OPERAND (expr, 0); > > but get_ref_base_and_extent shouldn't crash on anything here. The > question is what you want 'expr' to be? The comment of the function > says CTX specifies that, but doesn't constrain the CALL case (does > it have to be a memory argument)? > > With allowing handled_component_p but above not handling > VIEW_CONVERT_EXPR you leave the possibility of VIEW_CONVERT_EXPR (d_1) > slipping through. Since the non-memory cases will have at most > one wrapping handled_component get_ref_base_and_extent should be > reasonably cheap, so maybe just cut off SSA_NAME, ADDR_EXPR and > CONSTANT_CLASS_P at the start of the function? > The patch was intended just as a simple optimization in order not to run get_ref_base_and_extent on stuff where one can see from the top-most tree they the result won't be interesting. Indeed it looks like get_ref_base_and_extent does not really need this when run on non-loads. I'll think about the function a bit more but it seems like the patch just is not really necessary. Thanks, Martin