[Bug tree-optimization/55920] New: ICE in expand_debug_locations, at cfgexpand.c:3753

[Bug tree-optimization/55920] New: ICE in expand_debug_locations, at cfgexpand.c:3753

[rmansfield at qnx dot com]

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

             Bug #: 55920
           Summary: ICE in expand_debug_locations, at cfgexpand.c:3753
    Classification: Unclassified
           Product: gcc
           Version: 4.8.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: rmansfield@qnx.com
                CC: mjambor@suse.cz


Created attachment 29125
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=29125
preprocessed src

gcc version 4.8.0 20130109 (experimental) [trunk revision 195054] (GCC) 

Using arm-unknown-linux-gnueabi

$ ./xgcc -B. -O2 -g ~/ice.i -c
/home/ryan/ice.i: In function 'state_panic':
/home/ryan/ice.i:32:6: warning: incompatible implicit declaration of built-in
function 'memcpy' [enabled by default]
      memcpy (&from, client->active->address.iabuf, sizeof from);
      ^
/home/ryan/ice.i:25:7: internal compiler error: in expand_debug_locations, at
cfgexpand.c:3753
  void state_panic (cpp)      void *cpp;
       ^
0x5e3584 expand_debug_locations
    ../../gcc/cfgexpand.c:3749
0x5e3584 gimple_expand_cfg
    ../../gcc/cfgexpand.c:4606
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <http://gcc.gnu.org/bugs.html> for instructions.

Seems it was introduced in rev195015

http://gcc.gnu.org/viewcvs?view=revision&revision=195015

[Bug tree-optimization/55920] ICE in expand_debug_locations, at cfgexpand.c:3753

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-01-09 17:47:08 UTC ---
Seems the bug is that the DEBUG stmt created by SRA has
 <var_decl 0x7ffff19a4e40 from$s_addr
    type <integer_type 0x7ffff1996690 unsigned int sizes-gimplified asm_written
public unsigned SI
as the first operand, but
 <mem_ref 0x7ffff17e0f78
    type <record_type 0x7ffff17df1f8 in_addr sizes-gimplified asm_written
no-force-blk packed type_0 BLK
as the second operand (note the first one is SImode, the latter BLKmode).
Perhaps the packed attribute is what causes this, dunno.  But we probably just
shouldn't emit a debug stmt if the mode is different, unless we can e.g. use a
COMPONENT_REF on it to get at the right mode.

Also, as the aggregate is actually used (in the call stmt a few stmts later),
I'd say SRA shouldn't emit the debug stmts for it at all, ideally for PR55579
we'd emit those only either if we'd SRA it anyway in the code (disregarding
debug), or if we have just stores but no uses of the aggregate.
Right now on trunk every SRA pass adds another set of debug stmts, because the
aggregate assignments aren't DCEd (as they are used).

Looking at this, perhaps we should defer PR55579 resolution for stage1 of 4.9,
so we have more time to e.g. think about some size limits what we still emit as
debug stmts and what not, etc.
Martin, what are your thoughts?

[Bug tree-optimization/55920] [4.8 Regression] ICE in expand_debug_locations, at cfgexpand.c:3753

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.8.0
            Summary|ICE in                      |[4.8 Regression] ICE in
                   |expand_debug_locations, at  |expand_debug_locations, at
                   |cfgexpand.c:3753            |cfgexpand.c:3753

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> 2013-01-09 18:22:07 UTC ---
Yeah, I agree - this looked not like stage3 material ...

[Bug tree-optimization/55920] [4.8 Regression] ICE in expand_debug_locations, at cfgexpand.c:3753

[jamborm at gcc dot gnu.org]

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

Martin Jambor <jamborm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jamborm at gcc dot gnu.org

--- Comment #3 from Martin Jambor <jamborm at gcc dot gnu.org> 2013-01-11 16:53:08 UTC ---
As far as the ICE is concerned, I think that if we want to fix it by
reverting patches, we need to revert both the patch for PR 55579 and
PR 54971 (which introduced the generation of the debug statement in
question).

Alternatively, we can punt and put NULL on the right side of the debug
statement when types do not match.  I looked at all places where SRA
generates them and only in sra_modify_assign we can have this problem.
Yes, in 4.9 we can even create a well-typed MEM_REF.  Meanwhile, the
minimal patch (that I am about to bootstrap and test) would be:


2013-01-11  Martin Jambor  <mjambor@suse.cz>

    PR tree-optimization/55920
    * tree-sra.c (sra_modify_assign): Put NULL RHS into debug statement if
    it would have incompatible types.

Index: src/gcc/tree-sra.c
===================================================================
--- src.orig/gcc/tree-sra.c
+++ src/gcc/tree-sra.c
@@ -3108,8 +3108,15 @@ sra_modify_assign (gimple *stmt, gimple_

   if (lacc && lacc->grp_to_be_debug_replaced)
     {
-      gimple ds = gimple_build_debug_bind (get_access_replacement (lacc),
-                       unshare_expr (rhs), *stmt);
+      tree dbg_rhs;
+      gimple ds;
+
+      if (useless_type_conversion_p (lacc->type, TREE_TYPE (rhs)))
+    dbg_rhs = unshare_expr (rhs);
+      else
+    dbg_rhs = NULL_TREE;
+      ds = gimple_build_debug_bind (get_access_replacement (lacc), dbg_rhs,
+                    *stmt);
       gsi_insert_before (gsi, ds, GSI_SAME_STMT);
     }

[Bug tree-optimization/55920] [4.8 Regression] ICE in expand_debug_locations, at cfgexpand.c:3753

[jamborm at gcc dot gnu.org]

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

--- Comment #4 from Martin Jambor <jamborm at gcc dot gnu.org> 2013-01-11 16:55:54 UTC ---
The debug statements for non-DCEable variables can be easily disabled
by the following (also, yet untested) patch:


2013-01-11  Martin Jambor  <mjambor@suse.cz>

    * tree-sra.c (analyze_access_subtree): Do not mark non-removable
    accesses as grp_to_be_debug_replaced.

Index: src/gcc/tree-sra.c
===================================================================
--- src.orig/gcc/tree-sra.c
+++ src/gcc/tree-sra.c
@@ -2199,7 +2199,9 @@ analyze_access_subtree (struct access *r
     {
       if (allow_replacements
       && scalar && !root->first_child
-      && (root->grp_scalar_write || root->grp_assignment_write))
+      && (root->grp_scalar_write || root->grp_assignment_write)
+      && !bitmap_bit_p (cannot_scalarize_away_bitmap,
+                DECL_UID (root->base)))
     {
       gcc_checking_assert (!root->grp_scalar_read
                    && !root->grp_assignment_read);

[Bug tree-optimization/55920] [4.8 Regression] ICE in expand_debug_locations, at cfgexpand.c:3753

[jakub at gcc dot gnu.org]

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

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-01-11 17:04:45 UTC ---
I'd very much prefer to keep PR54971 fix in.

As for #c3, I believe it would be enough to test the mode, so perhaps
              tree repl = get_access_replacement (lacc);
              enum machine_mode mode = DECL_MODE (repl);
and compare that to TYPE_MODE (TREE_TYPE (rhs)).  At least that would match
what cfgexpand.c is actually checking, otherwise it looks reasonable.

And if #c4 works, that would be even better.

[Bug tree-optimization/55920] [4.8 Regression] ICE in expand_debug_locations, at cfgexpand.c:3753

[jakub at gcc dot gnu.org]

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

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-01-15 12:04:13 UTC ---
Created attachment 29168
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=29168
gcc48-pr55920.patch

Looking at the #c3 patch, I wonder if this wouldn't be more appropriate
(untested so far).  It tries to match roughly what the modify_this_stmt
case does a few lines before.

[Bug tree-optimization/55920] [4.8 Regression] ICE in expand_debug_locations, at cfgexpand.c:3753

[jamborm at gcc dot gnu.org]

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

--- Comment #7 from Martin Jambor <jamborm at gcc dot gnu.org> 2013-01-15 12:18:40 UTC ---
(In reply to comment #6)
> Created attachment 29168 [details]
> gcc48-pr55920.patch
> 
> Looking at the #c3 patch, I wonder if this wouldn't be more appropriate
> (untested so far).  It tries to match roughly what the modify_this_stmt
> case does a few lines before.

Yes, that's what I had in mind for 4.9.  I wanted the #c3 patch to be
minimal, given that we are now in stage 4 and we were even considering
reverting patches.  But if it is OK now, I'll be more than happy to
test it and commit it.

[Bug tree-optimization/55920] [4.8 Regression] ICE in expand_debug_locations, at cfgexpand.c:3753

[jakub at gcc dot gnu.org]

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

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-01-15 16:33:38 UTC ---
Author: jakub
Date: Tue Jan 15 16:33:24 2013
New Revision: 195209

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=195209
Log:
    PR tree-optimization/55920
    * tree-sra.c (sra_modify_assign): If for lacc->grp_to_be_debug_replaced
    there is non-useless type conversion needed from debug rhs to lhs,
    use build_debug_ref_for_model and/or VIEW_CONVERT_EXPR.

    * gcc.c-torture/compile/pr55920.c: New test.

Added:
    trunk/gcc/testsuite/gcc.c-torture/compile/pr55920.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-sra.c

[Bug tree-optimization/55920] [4.8 Regression] ICE in expand_debug_locations, at cfgexpand.c:3753

[jamborm at gcc dot gnu.org]

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

--- Comment #9 from Martin Jambor <jamborm at gcc dot gnu.org> 2013-01-15 16:43:14 UTC ---
Author: jamborm
Date: Tue Jan 15 16:43:05 2013
New Revision: 195210

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=195210
Log:
2013-01-15  Martin Jambor  <mjambor@suse.cz>

    PR tree-optimization/55920
    * tree-sra.c (analyze_access_subtree): Do not mark non-removable
    accesses as grp_to_be_debug_replaced.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/tree-sra.c

[Bug tree-optimization/55920] [4.8 Regression] ICE in expand_debug_locations, at cfgexpand.c:3753

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |FIXED

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-01-15 18:36:36 UTC ---
Fixed.