From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 115716 invoked by alias); 13 Nov 2019 14:39:30 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 115706 invoked by uid 89); 13 Nov 2019 14:39:30 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-5.4 required=5.0 tests=AWL,BAYES_00 autolearn=ham version=3.3.1 spammy=H*i:sk:CAFiYyc, organized, H*f:sk:driQ@ma, H*i:sk:_-oZzXZ X-HELO: nikam.ms.mff.cuni.cz Received: from nikam.ms.mff.cuni.cz (HELO nikam.ms.mff.cuni.cz) (195.113.20.16) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 13 Nov 2019 14:39:28 +0000 Received: by nikam.ms.mff.cuni.cz (Postfix, from userid 16202) id 62B69280823; Wed, 13 Nov 2019 15:39:25 +0100 (CET) Date: Wed, 13 Nov 2019 14:41:00 -0000 From: Jan Hubicka To: Richard Biener Cc: Martin Jambor , Feng Xue OS , "gcc-patches@gcc.gnu.org" Subject: Re: Ping: [PATCH V6] Extend IPA-CP to support arithmetically-computed value-passing on by-ref argument (PR ipa/91682) Message-ID: <20191113143925.7pyf2akdxbtupupj@kam.mff.cuni.cz> References: <1dc2bc97-7cb7-fae6-f88f-26b256be3707@linux.ibm.com> <20191112121526.dbxmpjadlrf7vc46@kam.mff.cuni.cz> <20191112123418.huts5lnnxjaybjw6@kam.mff.cuni.cz> <20191112132720.r5ralkbc6se5lc6v@kam.mff.cuni.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) X-SW-Source: 2019-11/txt/msg01050.txt.bz2 > On Tue, Nov 12, 2019 at 2:27 PM Jan Hubicka wrote: > > > > > Hi, > > > > > > On Tue, Nov 12 2019, Jan Hubicka wrote: > > > > Also note that there is a long standing problem with inlining ipacp > > > > clones. This can be shown on the following example: > > > > > > > > struct a {int a;}; > > > > static int foo (struct a a) > > > > { > > > > return a.a; > > > > } > > > > __attribute__ ((noinline)) > > > > static int bar (struct a a) > > > > { > > > > return foo(a); > > > > } > > > > main() > > > > { > > > > struct a a={1}; > > > > return bar (a); > > > > } > > > > > > > > Now if you compile it with -O2 -fno-early-inlining ipacp correctly > > > > determines constants: > > > > > > > > Estimating effects for bar/1. > > > > Estimating body: bar/1 > > > > Known to be false: > > > > size:6 time:14.000000 nonspec time:14.000000 > > > > - context independent values, size: 6, time_benefit: 0.000000 > > > > Decided to specialize for all known contexts, code not going to grow. > > > > Setting dest_lattice to bottom, because type of param 0 of foo is NULL or unsuitable for bits propagation > > > > > > > > Estimating effects for foo/0. > > > > Estimating body: foo/0 > > > > Known to be false: op0[offset: 0] changed > > > > size:3 time:2.000000 nonspec time:3.000000 > > > > - context independent values, size: 3, time_benefit: 1.000000 > > > > Decided to specialize for all known contexts, code not going to grow. > > > > > > > > > > > > Yet the intended tranformation to "return 1" does not happen: > > > > > > > > __attribute__((noinline)) > > > > bar.constprop (struct a a) > > > > { > > > > int a$a; > > > > > > > > [local count: 1073741824]: > > > > a$a_5 = a.a; > > > > return a$a_5; > > > > > > > > } > > > > > > > > > > > > > > > > ;; Function main (main, funcdef_no=2, decl_uid=1937, cgraph_uid=3, symbol_order=2) (executed once) > > > > > > > > main () > > > > { > > > > struct a a; > > > > int _3; > > > > > > > > [local count: 1073741824]: > > > > a.a = 1; > > > > _3 = bar.constprop (a); [tail call] > > > > a ={v} {CLOBBER}; > > > > return _3; > > > > > > > > } > > > > > > > > The problem here is that foo get inlined into bar and we never apply > > > > ipcp transform on foo, so a.a never gets constant propagated. > > > > > > Ugh, we never... what? That is quite bad, how come we don't have PR > > > about this? > > > > I remember speaking about it with you few times years ago :) > > > > > > > > > > > For value ranges this works since late passes are able to propagate > > > > constants from value ranges we attach to the default def SSA names. I > > > > > > Well, there are no SSA names for parts of aggregates. > > > > I think all we need is to make FRE's alias oracle walker which is > > responsible for propagation of constants to see if it hits entry of > > function, check that base is a parameter and look into ipcp transform > > summary if known value is there. > > Why don't we instantiate IPA CP clones we want to inline and then > inline the instantiated function instead of the original? Isn't that simpler That is what we discussed originally ;) One problem is that as the code is organized currently applying tranform to a function will apply inline decisions. Inline decisions may differ in inline and offline version of the code. This can be fixed by arranging one additional offline copy (so having possibly two) which never gets inlined and call ipcp transform on it specially. However this does not solve problem fully - if I extended the testcase so the access goes thorugh pointer which becomes &a only during late optimization, we will still have missed optimization. The way value ranges happens to work by letting local optimizations to do the propagation into inline copies seems quite easy and robust. It seems to me that it is easiest to maintain variant here - think of ipcp as an analysis pass which does the propagation (and decides on cloning) and let late passes to update gimple bodies. > and not too bad since it shouldn't happen all so often? > > Or apply the IPA transform during inlining itself... Yep, that was other solution I was thinking about - it would be nice to not make tree-inline more complex though. Honza