[Bug debug/54519] New: [4.6/4.7/4.8 Regression] Debug info quality regression due to (pointless) partial inlining

[Bug debug/54519] New: [4.6/4.7/4.8 Regression] Debug info quality regression due to (pointless) partial inlining

[jakub at gcc dot gnu.org]

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

             Bug #: 54519
           Summary: [4.6/4.7/4.8 Regression] Debug info quality regression
                    due to (pointless) partial inlining
    Classification: Unclassified
           Product: gcc
           Version: 4.7.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: debug
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: jakub@gcc.gnu.org
                CC: aoliva@gcc.gnu.org, hubicka@gcc.gnu.org,
                    jan.kratochvil@redhat.com


extern void bar (int);

static int
foo (int x, int y)
{
  if (x)
    {
      bar (x);
      bar (x);
      bar (x);
      bar (x);
      bar (x);
    }
  return y;
}

#ifndef INL
void *p = (void *) foo;
#else
int
baz1 (int x, int y)
{
  return foo (x, y);
}

int
baz2 (int x, int y)
{
  return foo (x, y);
}
#endif

at -g -O2 results in partial inlining (even when without -DINL it has no
callers at all, can that be changed?), and immediately inlining it back.  In
4.7 (before
http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=188981
change) in debuginfo we actually end up with terribly confusing
DW_TAG_inlined_subroutine of the same abstract origin as the current function,
r188981 improves that slightly, so it is actually DW_TAG_lexical_block instead.
But still, for one of the parameters we end up with unnecessary another
location list for it (it would be nice if we could merge them) and more
importantly, for the second parameter we end up with another DW_TAG_variable
with abstract origin of its origin, but actually no location list at all.
This is the main reason why this PR is filed.
The VAR_DECL is created by copy_arguments_for_versioning, with copy_decl_to_var
there, just in case the var would be used as local temporary.  It isn't used in
this case, so it ends up with no location at all (we want to use the location
of the outer argument in that case), if it was used as a local temporary, we'd
still want to use the outer parameter location before it is initialized.

And completely another case is when foo.part.N isn't inlined back, -g -O2 -DINL
shows that case.  Then y still doesn't have any location info.

Either we could make inlining smarter about the case where fnsplitted function
is being inlined back into the main body (essentially try to remap the inline
fn parameter or these special outer block vars back to the outer function's
parameters or variables), or perhaps at least add a debug source stmt at the
beginning of the foo.part.* which would say that the special var created by
copy_arguments_for_versioning lives in the caller's parameter (like we do for
say IPA cp).

[Bug debug/54519] [4.6/4.7/4.8 Regression] Debug info quality regression due to (pointless) partial inlining

[rguenth at gcc dot gnu.org]

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

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2012-09-10
                 CC|                            |rguenth at gcc dot gnu.org
   Target Milestone|---                         |4.6.4
     Ever Confirmed|0                           |1

--- Comment #1 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-09-10 11:02:10 UTC ---
I wonder whether we can make the split function pieces "virtual clones", thus
drive all this from IPA inlining instead.  So in case we do not inline the
header keep the call to the unsplit function.  That would have been a better
overall design IMHO (not giving any early optimization benefits of course).
But even with the current design we might be able to simply fold back to
calling the original function at some point (we'd of course have to outline
both the header and the tail in this case to keep the original body).

[Bug debug/54519] [4.6/4.7/4.8 Regression] Debug info quality regression due to (pointless) partial inlining

[jakub at gcc dot gnu.org]

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

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-09-10 16:21:54 UTC ---
Created attachment 28161
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=28161
gcc48-pr54519.patch

Untested WIP patch which tries to handle at least the case where the fn.part.N
isn't inlined back.  Will need to do something at the inlining side too, to
change the source binds back to normal binds of the corresponding vars.

And of course, this doesn't handle non-gimple-reg arguments at all, so for the
case where we inline back the fn.part.N (either into original function or into
some function into which fn has been inlined already) it would still be nice to
special case that inlining.

[Bug debug/54519] [4.6/4.7/4.8 Regression] Debug info quality regression due to (pointless) partial inlining

[jakub at gcc dot gnu.org]

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

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 #3 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-09-11 13:26:54 UTC ---
Created attachment 28171
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=28171
gcc48-pr54519.patch

Generic solution patch.  This doesn't attempt to special case inlining of
FN.part.N back into FN or FN inlined into BAR (which is going to be harder than
I've initially thought, because both the (possibly inlined) FN and FN.part.N
have originally full copy of the BLOCK tree of FN, after some optimizations in
between fnsplit and inlining some of the BLOCKs or BLOCK_VARS in either or both
of them might be removed though.  So, expand_call_inline would probably need to
avoid attaching remap_blocks as children of new BLOCK it creates, instead it
should somehow merge the two BLOCK trees back into one (it could use
BLOCK_ABSTRACT_ORIGIN to find the matching blocks, and for the blocks that
already exist in FN just insert_decl_map from FN.part.N's BLOCK to
corresponding FN BLOCK).  Similarly BLOCK_VARS need to be handled by preferring
to remap to vars in the caller FN BLOCK_VARS (just insert_decl_map those), and
just re-add the rest that was dropped on the floor in the mean time.  And it
would need to drop some of the debug bind and debug source bind stmts for
parameters that this patch adds.

Several of the tests fail sometimes, I'm going to file a separate PR for that
because the problem is during the first df_analyze.

[Bug debug/54519] [4.6/4.7/4.8 Regression] Debug info quality regression due to (pointless) partial inlining

[jakub at gcc dot gnu.org]

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

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-10-05 19:24:41 UTC ---
Author: jakub
Date: Fri Oct  5 19:24:38 2012
New Revision: 192139

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=192139
Log:
    PR debug/54519
    * ipa-split.c (split_function): Add debug args and
    debug source and normal stmts for args_to_skip which are
    gimple regs.
    * tree-inline.c (copy_debug_stmt): When inlining, adjust
    source debug bind stmts to debug binds of corresponding
    DEBUG_EXPR_DECL.

    * gcc.dg/guality/pr54519-1.c: New test.
    * gcc.dg/guality/pr54519-2.c: New test.
    * gcc.dg/guality/pr54519-3.c: New test.
    * gcc.dg/guality/pr54519-4.c: New test.
    * gcc.dg/guality/pr54519-5.c: New test.
    * gcc.dg/guality/pr54519-6.c: New test.

Added:
    trunk/gcc/testsuite/gcc.dg/guality/pr54519-1.c
    trunk/gcc/testsuite/gcc.dg/guality/pr54519-2.c
    trunk/gcc/testsuite/gcc.dg/guality/pr54519-3.c
    trunk/gcc/testsuite/gcc.dg/guality/pr54519-4.c
    trunk/gcc/testsuite/gcc.dg/guality/pr54519-5.c
    trunk/gcc/testsuite/gcc.dg/guality/pr54519-6.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/ipa-split.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-inline.c

[Bug debug/54519] [4.6/4.7 Regression] Debug info quality regression due to (pointless) partial inlining

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED
            Summary|[4.6/4.7/4.8 Regression]    |[4.6/4.7 Regression] Debug
                   |Debug info quality          |info quality regression due
                   |regression due to           |to (pointless) partial
                   |(pointless) partial         |inlining
                   |inlining                    |

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-11-08 12:25:52 UTC ---
Fixed for trunk, not planning a backport.