Re: Ping: [PATCH V6] Extend IPA-CP to support arithmetically-computed value-passing on by-ref argument (PR ipa/91682)

[Jan Hubicka <hubicka@ucw.cz>]

> 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;
> >
> >   <bb 2> [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;
> >
> >   <bb 2> [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.
> 
> > think correct answer here is to do no subtitution in in ipa-prop.c
> > transform function.  Rather note the known values for late passes and
> > let FRE do its job.
> 
> And where would you like to save it?   Do a load at the beginning of the
> function?  My thinking was that it is better to modify the IL rather
> than storing stuff to ad-hoc on-the-side data structures.

It is already saved in the ipcp transform summary. It is about keeping
it around while copmiling function and using it same way as we use, say
results of ipa-reference analysis.

Honza
> 
> Martin