Re: [patch] Add block debug info to phi_arg_d

[Dehao Chen <dehao at google dot com>]

On Thu, Jul 5, 2012 at 5:58 PM, Richard Guenther
<richard.guenther@gmail.com> wrote:
>
> On Thu, Jul 5, 2012 at 11:42 AM, Steven Bosscher <stevenb.gcc@gmail.com> wrote:
> > On Thu, Jul 5, 2012 at 11:23 AM, Richard Guenther
> > <richard.guenther@gmail.com> wrote:
> >> On Thu, Jul 5, 2012 at 11:08 AM, Dehao Chen <dehao@google.com> wrote:
> >>> Hi,
> >>>
> >>> This patch added block field to phi_arg_d to make sure the associated
> >>> source locus is consistent with its block info.
> >>>
> >>> Bootstrapped and passed gcc regression tests.
> >>>
> >>> OK for trunk?
> >>
> >> Hum - makes me want a location like we have on RTL (which maps to
> >> location plus block ...).  But oh well ...
> >>
> >> struct GTY(()) phi_arg_d {
> >>   /* imm_use MUST be the first element in struct because we do some
> >>      pointer arithmetic with it.  See phi_arg_index_from_use.  */
> >>   struct ssa_use_operand_d imm_use;
> >> location_t locus;
> >> +  tree block;
> >>  };
> >>
> >> please place block before locus though.
> >>
> >> Ok with that change.
> >
> > Ehm, wait a moment, please -- does this have to be a tree pointer? Why
> > not use BLOCK_NUMBER instead? Adding a pointer to such a
> > frequently-used structure should be avoided if an int suffices.
>
> Well, we re-number blocks frequently enough that this won't work.  In
> gimple stmts there is also a block tree pointer.  The real solution would
> of course be to associate location_t with a block, too, like the source locus
> on RTL does.  It will also be fun do deal with during inlining and LTO
> streaming.

Yeah, I agree that we should have a uniformed way to represent locus,
discriminator and block together. But as you mentioned, updating it
would be non-trivial since blocks are updated regularly (I'm wondering
why we need to eliminate redundant blocks all the time, just for
saving space?). But anyway, this patch can at least ensure consist
debug info. Shall we get it into trunk?

Thanks,
Dehao

>
> Richard.
>
> > Ciao!
> > Steven