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 E2D0D3873CEE for ; Tue, 13 Dec 2022 12:53:07 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org E2D0D3873CEE 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 D282F1F45E; Tue, 13 Dec 2022 12:53:06 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1670935986; 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=e+iPFgof9UN1tPyOyx5/6opxp098wd7tjOZfw9EbkE0=; b=OQczStYz3nOmxauw3SZfo6pLOFylHEW6uizujNRhBkPowHtXwZdbm4/Vi5FjU882g/jDIp nKwWzha5Y9joFMYz0vdIYJOIwA4P5b+fhX9maY5hng+GvF5GCAx+DNM88md4RXkQrFq2oL TrhU6K77Xz/cLFstcz6wHPFCMF4eTAQ= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1670935986; 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=e+iPFgof9UN1tPyOyx5/6opxp098wd7tjOZfw9EbkE0=; b=8XtfYKCT34Zhyozxm6ll0ubr9e+0n3pqww8ikpyQmLLq6UUaPxQApP2+hbFLQkYAUbSi1m T5yf3AtAkPXzEmAw== 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 B5ADF2C141; Tue, 13 Dec 2022 12:53:06 +0000 (UTC) Date: Tue, 13 Dec 2022 12:53:06 +0000 (UTC) From: Richard Biener To: Jan Hubicka cc: Martin Jambor , GCC Patches Subject: Re: [PATCH 8/9] ipa-sra: Make scan_expr_access bail out on uninteresting expressions In-Reply-To: Message-ID: References: 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=-11.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,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 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? > Looking at: > > static inline bool > gimple_assign_load_p (const gimple *gs) > { > tree rhs; > if (!gimple_assign_single_p (gs)) > return false; > rhs = gimple_assign_rhs1 (gs); > if (TREE_CODE (rhs) == WITH_SIZE_EXPR) > return true; > rhs = get_base_address (rhs); > return (DECL_P (rhs) > || TREE_CODE (rhs) == MEM_REF || TREE_CODE (rhs) == TARGET_MEM_REF); > } > > I wonder if we don't want to avoid get_base_address (which is loopy) and > use same check and move it into a new predicate that is more convenient > to use? > > Honza > > > > Honza > > > + > > > base = get_ref_base_and_extent (expr, &poffset, &psize, &pmax_size, &reverse); > > > > > > if (TREE_CODE (base) == MEM_REF) > > > -- > > > 2.38.1 > > > > -- Richard Biener SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman; HRB 36809 (AG Nuernberg)