Re: Basic infrastructure for substitution tracking

[Richard Guenther <rguenther at suse dot de>]

On Thu, 24 Sep 2009, Jan Hubicka wrote:

> > On Sep 20, 2009, Jan Hubicka <hubicka@ucw.cz> wrote:
> > 
> > > Most interesting part is handling in inliner - basically when VAR is getting
> > > replaced by expression, simplify_value is called first to reconstruct (if
> > > possible) simple generic expression that reffers to user vars instead of
> > > temporaries so there is good chance we will be able to work out the value
> > > of exrpession when outputting debug info.
> > 
> > I'm a bit concerned with the apparent confusion between compiler and
> > source concepts.
> > 
> > Consider, for example, a function like this:
> > 
> > int f(int i) {
> >   int j = i+1;
> >   ...
> >   i = j;
> >   ...
> > }
> > 
> > This could very well be optimized to code that does modify i, say:
> > 
> > int f(int i) {
> >   int j;
> > 
> >   j_2 = i_1(D) + 1;
> >   # DEBUG j => j_2
> >   ...
> >   # DEBUG i => j_2
> >   ...
> > }
> > 
> > and then, AFAICT, your patch would decide that user variable i is never
> > modified, just because the ???implementation??? variable i isn't.  It would
> > then proceed to nonlocalize i.  Oops.
> 
> In my current code DEBUG counts as a set, so we won't nonlocalize here.  Well,
> there is problem that we will nonlocalize more with -no-g than with -g that
> will lead to different DECL_UIDs and different optimization decisions.  But
> this seems to be same in current tree (i.e. if debug statements hold decl live,
> we will copy it when inlining but we will prune it early).  How do you handle
> this problem?  Did you made passes independent on DECL_UID numbers?
> > 
> > 
> > Conversely, the VAR_VALUE expressions refer to names in the
> > ???implementation??? namespace, rather than in the ???source??? namespace.  But
> > then, IIUC, at the time of emitting debug information, you'd interpret
> > them as if they referred to ???source??? concepts.  This would lead to other
> > kinds of errors.  Consider:
> > 
> > int g(int a) {
> >   int b = a + 1;
> >   k(b);
> >   b = a++;
> >   f(a);
> > }
> > 
> > Early (pre-inline) optimizations could turn this into:
> > 
> > int g(int a) {
> >   int b;
> > 
> >   b_2 = a_1(D);
> >   # DEBUG b => b_2
> >   k(b_2);
> >   # DEBUG b => a_1(D)
> >   # DEBUG a => b_2
> >   f(b_2);
> > }
> > 
> > Now, if you inline f and decide you should map f's i to g's b, as it
> > might seem like you should do, you lose, because the implementation name
> > b_2 at that point holds the value for user variable a, not for b.
> 
> Hmm, I guess we would lose here indeed.
> > 
> > As such, I don't see much benefit for nonlocalizing VTA-trackable
> > variables.  For variables that are correctly amenable to this kind of
> > mapping, we should have no more than the inlined-entry-point debug stmt
> > for the parameter, which should end up as a single const_value or
> > location expression, applicable to the entire range, and without mixing
> > up and confusing source concepts with implementation concepts that could
> > result in incorrect debug information.
> 
> Yep, my main concern is about costs in inline heavy code.  We do have problems
> to scale on our default pooma testcases and I guess things will get worse over
> years.  The nonlocalizing change itself caused _a lot_ of memory usage
> improvement since the duplicated declarations are about main memory consumers
> when compiling such codebases.  Now debug stuff makes this worse not only
> requiring declarations, but also debug statements, expressions to track, value
> ranges etc.
> 
> We can limit tracking to function-wide constants (i.e. addresses and such).  I
> can give it a try if this has enough of coverage to save something in pracice,
> or we can just give up on the idea.
> > 
> > As for variables that currently aren't tracked by VTA...
> > 
> > For fully-scalarized variables (i.e., those for which a structure does
> > not survive), each component variable could in in theory be handled as a
> > VTA-trackable (one-part) variable, getting its own debug stmts and all.
> > Nothing actually prevents SR decls from showing up in debug stmts, and
> > all we have to do to deal with them properly at debug info generation
> > time is to consolidate the location lists for each part of the aggregate
> > source variable into a single location list.  AFAIK there's no reason to
> 
> Yes, that is rougly idea of how the code works.  
> Having struct a a;
> we will get variables a$a and a$b.  Those gets down the usual way getting location
> list.  Now the a's declaration has VALUE_EXPR with CONSTRUCTOR {a&a, a&b}.
> While expanding debug info for struct a, we call add_value on that constructor
> that takes the location lists of separate components and combines them back to
> what you write bellow.
> 
> > actually *combine* the separate parts into location list entries.  I.e.,
> > for a variable of type struct { int a, b; }, we could emit location
> > lists such as:
> > 
> > [PC1, PC2) <location for a> DW_OP_piece 4 [missing] DW_OP_piece 4
> > [PC2, PC3) <location for a> DW_OP_piece 4 [missing] DW_OP_piece 4
> > ...
> > [PC1, PC3) [missing] DW_OP_piece 4 <location for b> DW_OP_piece 4
> > [PC3, PC4) [missing] DW_OP_piece 4 <location for b> DW_OP_piece 4
> > ...
> > 
> > Partially-scalarized variables (i.e., those that retain an aggregate
> > variable, and fields are extracted from or stored in it), similar
> > arrangements could work, at least for the scalarized variables, that
> > should get debug stmts when they get extracted or stored in the
> > structure.  However, non-scalarized components might require more than
> > an initial bind statement, for aliasing memory stores may cause the
> > value binding to be invalidated.  To deal properly with aliased memory
> > locations, we'd need a location bind, rather than a value bind, so that
> > memory stores don't invalidate the association between source name and
> > memory location.  The alternative would be to emit additional debug
> > stmts at every aliasing store, which would be wasteful.
> 
> Yep, the partly scalarized variables are ugly, moreover our current SRA
> implementation sort of don't really know if it is scalarizing completely or
> not.  My implementation handles those same route as fully scalarized that
> should match the fact that SRA basically packs+unpacks on every occurence
> of full access to the whole aggregate.
> > 
> > As for non-scalarizable variables, these are always (?) addressable (no
> > benefit from var-tracking, because their location is constant) or fully
> > removed (could be handled just like fully-scalarized variables).  I can
> > see how NONLOCALIZED vars could help improve debug info for the latter
> > case, at least for as long as VTA can't deal with the scalarized cases.
> 
> The formely non-gimple regs seems to be quite common where we lose
> optimization.  I.e. variables that are passed by reference to function and thus
> not turned into SSA form early but after early inlining they become normal
> vars.
> 
> Simple testcase would be:
> 
> static inline
> t(int *a)
> {
> }
> 
> main()
> {
>   int ssa=1;
>   int nonssa=1;
>   t(&nonssa);
>   return ssa+nonssa;
> }
> 
> We end up with:
> 
> main ()
> {
> <bb 2>:
>   # DEBUG ssa => 1
>   # DEBUG a => &nonssa
>   return 2;
> 
> }
> 
> I.e. somehow we miss DEBUG nonssa => 1 though I don't quite see why (nonssa disappears in CCP
> after it is resolved to following form by ealias):
> 
> main ()
> { 
>   int nonssa;
>   int nonssa.0;
>   int D.2703;
> 
> <bb 2>:
>   # DEBUG ssa => 1
>   nonssa_5 = 1;
>   # DEBUG a => &nonssa
>   D.2703_3 = nonssa_5 + 1;
>   return D.2703_3;
> }
> 
> looks like ealias should emit new DEBUG statements when turning new symbol into SSA form?

No, in fact the above is invalid IL as you have still the address taken
of nonssa in the DEBUG insn.  The update_address_taken pass needs to
drop DEBUG insns still refering to addresses of variables we are going
to rewrite into SSA onto the floor.  And yes, the into-ssa should
generate new DEBUG insns - but I was assuming it already did that, or
how do we generate the initial set of DEBUG insns?

Thus, I'd have expected

 <bb 2>:
   # DEBUG ssa => 1
   nonssa_5 = 1;
   # DEBUG nonssa => nonssa_5
   # DEBUG a => NULL
   D.2703_3 = nonssa_5 + 1;
   return D.2703_3;

thus, new DEBUG insn for nonssa and the DEBUG insn for a killed
(the memory is gone now).

Richard.