Re: [PATCH] Fix debug info for expr and jump stmt

[Dehao Chen <dehao at google dot com>]

On Sat, Oct 27, 2012 at 11:07 AM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Sat, Oct 27, 2012 at 12:53 AM, Dehao Chen <dehao@google.com> wrote:
>> Hi,
>>
>> I've updated the patch:
>>
>> 1. abandon the changes in cfgexpand.c
>> 2. set the block for trees when lowering gimple stmt.
>> 3. add a unittest.
>
> Index: gcc/gimple-low.c
> ===================================================================
> --- gcc/gimple-low.c    (revision 192809)
> +++ gcc/gimple-low.c    (working copy)
> @@ -331,7 +331,18 @@ lower_omp_directive (gimple_stmt_iterator *gsi, st
>    gsi_next (gsi);
>  }
>
> +/* Call back function to set the block for expr.  */
>
> +static tree
> +tree_set_block_r (tree *tp, int *walk_subtrees ATTRIBUTE_UNUSED,
> +                     void *data)
> +{
> +  tree block = (tree) data;
> +  if (CAN_HAVE_LOCATION_P (*tp))
> +    TREE_SET_BLOCK (*tp, block);
> +  return NULL_TREE;
> +}
> +
>  /* Lower statement GSI.  DATA is passed through the recursion.  We try to
>     track the fallthruness of statements and get rid of unreachable return
>     statements in order to prevent the EH lowering pass from adding useless
> @@ -343,8 +354,11 @@ static void
>  lower_stmt (gimple_stmt_iterator *gsi, struct lower_data *data)
>  {
>    gimple stmt = gsi_stmt (*gsi);
> +  unsigned i;
>
>    gimple_set_block (stmt, data->block);
> +  for (i = 0; i < gimple_num_ops (stmt); i++)
> +    walk_tree (gimple_op_ptr (stmt, i), tree_set_block_r, data->block, NULL);
>
>    switch (gimple_code (stmt))
>      {
>
>
> why do you need this?  The stmt location is taken from the operands and in fact
> they may have more precise locations.  So it seems completely pointless to me
> (wasting location entries where a NULL block is just fine (take the block from
> the stmt)).
>
> So, what is the issue you try to fix?  Yes, the stmt operands will
> keep the BLOCK
> live, but it is live if the frontend assigned it.

The issue I was trying to fix is that when expanding to rtl, the
operands's location is still gonna be used by set_curr_insn_location.
Thus we need to set the block info when we update the block info for
stmt. The unittest I added will have this problem.

Thanks,
Dehao

>
> Richard.
>
>> However, this patch will trigger two lto bug when asserting
>> LTO_NO_PREVAIL for TREE_CHAIN. After debugging for a while, I found
>> that the problem was also there even without the patch. This patch
>> just reveal the problem by moving a decl into cache so that it will be
>> checked. As I'm not familiar with LTO, not quite sure what the root
>> problem is. Can anyone help take a look?
>>
>> Thanks,
>> Dehao
>>
>> gcc/ChangeLog:
>> 2012-10-25  Dehao Chen  <dehao@google.com>
>>
>>         * tree-eh.c (do_return_redirection): Set location for jump statement.
>>         (do_goto_redirection): Likewise.
>>         (frob_into_branch_around): Likewise.
>>         (lower_try_finally_nofallthru): Likewise.
>>         (lower_try_finally_copy): Likewise.
>>         (lower_try_finally_switch): Likewise.
>>         * gimple-low.c (tree_set_block_r): New callback function.
>>         (lower_stmt): Set block for tested expr.
>>
>> gcc/testsuite/ChangeLog:
>> 2012-10-25  Dehao Chen  <dehao@google.com>
>>
>>         * g++.dg/debug/dwarf2/block.C: New testcase.