[Bug debug/47858] [4.5/4.6/4.7 Regression] IPA-SRA decreases quality of debug info

["jakub at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47858

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|unassigned at gcc dot       |jakub at gcc dot gnu.org
                   |gnu.org                     |

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> 2011-06-02 12:21:51 UTC ---
Created attachment 24416
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=24416
gcc47-pr47858.patch

Work in progress patch, so far just for the callee side only and not hooked
into IPA-SRA etc. too much.

main testcase used is:
volatile int vv;

static __attribute__((noinline)) int
f1 (int x, int y, int z)
{
  int a = x * 2;
  int b = y * 2;
  int c = z * 2;
  vv++;
  return x + z;
}

int
u1 (int x)
{
  return f1 (x, 2, 3) + f1 (x, 4, 3) + f1 (x + 6, x, 3) + x;
}

The main changes of this patch which I'd like to discuss are:
1) introduction of new debug stmt kind, DEBUG_SOURCE_BIND (something like
   we talked about on IRC, except due to use of the subcode we can't have flags
   and thus it is a new kind).  It has the source mapping semantics,
   so for GIMPLE POV is considered as a black box statement, which doesn't use
   anything.
   Alternatively, we could have a new tree which would embed a decl in it,
stand
   for black box and not be considered any kind of use.
2) remap_ssa_name creates these s=> debug stmts if possible instead of
resetting
   (replaces with D#N temporary set by s=>)
These two things alone fix the testcase in this comment with -g -O2
-fno-ipa-sra, during expansion it is expanded as ENTRY_VALUE and var-tracking
figures out it is even still available in a register.
For -O2 -g -fipa-sra, for b variable this offers a part of solution, in
particular:
3) add DW_OP_GNU_parameter_ref op, which references the artificial
DW_TAG_formal_parameter and corresponding DEBUG_PARAMETER_REF rtl
We should emit DEBUG y s=> y too when optimizing away the parameter so that the
parameter gets DW_OP_GNU_parameter_ref too (or alternatively expansion could
add those automatically for optimized away parameters directly in the form of
debug insn).

The original testcase from kernel needs __attribute__((noinline)) now on
may_create, otherwise it is inlined and doesn't test the interesting case.
And, for better coverage there should be struct dentry *child2 = child; or
similar at the start of may_create too, to also test vars whose value is
related to the optimized away parameter.  Again, some more work is needed not
to reset the # DEBUG child2 => child_2(D) stmt during eipa_sra, but instead add
# DEBUG D#N s=> child
# DEBUG child2 => D#N
and either again add it for the child parameter too
# DEBUG child s=> child
or handle it during expansion.

That is only the first part of the solution, there needs to be matching
DW_TAG_GNU_call_site_parameter with DW_AT_abstract_origin of the decl which
DW_OP_GNU_parameter_def will reference, with the right value.

This could be either again some special debug stmt kind before the call, or
perhaps could be just the call stmt itself taking DEBUG_EXPR_DECLs as extra
arguments (either normal args, but there is some risk of breaking various
passes), or say setting a flag in GIMPLE_CALL's subcode that it has
DEBUG_EXPR_DECL arguments and if it does, call some slower gimple_call_num_args
variant which would omit from the call DEBUG_EXPR_DECLs at the end.
At RTL level this could be e.g. represented by having DEBUG_EXPRs in
CALL_INSN_FUNCTION_USAGE.  So, we would have say:
  # DEBUG D#7 => 2
  D.2696_2 = f1.isra.0 (x_1(D), 3 [, D#7]);
  # DEBUG D#8 => 4
  D.2697_3 = f1.isra.0 (x_1(D), 3 [, D#8]);
  D.2698_4 = D.2696_2 + D.2697_3;
  D.2699_5 = x_1(D) + 6;
  # DEBUG D#9 => x_1(D)
  D.2700_6 = f1.isra.0 (D.2699_5, 3 [, D#9]);