Bug 30564 - [4.3 Regression] ice for legal code with -O3
Summary: [4.3 Regression] ice for legal code with -O3
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 4.3.0
: P1 normal
Target Milestone: 4.3.0
Assignee: Andrew Pinski
URL: http://gcc.gnu.org/ml/gcc-patches/200...
Keywords: ice-on-valid-code, patch
: 32033 (view as bug list)
Depends on:
Blocks: 32033
  Show dependency treegraph
 
Reported: 2007-01-23 20:58 UTC by David Binderman
Modified: 2007-08-20 07:43 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2007-01-23 21:55:31


Attachments
C source code (57.12 KB, text/plain)
2007-01-23 21:00 UTC, David Binderman
Details
somewhat reduced testcase (890 bytes, text/plain)
2007-01-23 21:56 UTC, Richard Biener
Details

Note You need to log in before you can comment on or make changes to this bug.
Description David Binderman 2007-01-23 20:58:30 UTC
I just tried to compile Suse Linux package lynx-2.8.6_rel2-23
with the GNU C compiler version 4.3 snapshot 20070119.

The compiler said

gcc -DLINUX  -D_GNU_SOURCE -DHAVE_CONFIG_H -I../.. -I../../src -I../../src/chrtrans -I../../WWW/Library/Implementation -I../../   -O3 -fmessage-length=0 -D_FORTIFY_SOURCE=2 -pipe   -c ./makeuctb.c
./makeuctb.c: In function 'main':
./makeuctb.c:253: internal compiler error: in extract_range_from_assert, at tree-vrp.c:820
Please submit a full bug report,
with preprocessed source if appropriate.
See <URL:http://gcc.gnu.org/bugs.html> for instructions.

Preprocessed source code attached. Flag -O3 required.
Comment 1 David Binderman 2007-01-23 21:00:07 UTC
Created attachment 12941 [details]
C source code
Comment 2 Richard Biener 2007-01-23 21:29:03 UTC
Nice one.

(gdb) up
#1  0x085aa055 in extract_range_from_assert (vr_p=0xbf806324, expr=0xb7ba1ee8)
    at /home/richard/src/trunk/gcc/tree-vrp.c:845
845       gcc_assert (limit != var);
(gdb) call debug_generic_expr (expr)
ASSERT_EXPR <i_657, i_657 != i_657>

reducing.
Comment 3 Richard Biener 2007-01-23 21:55:31 UTC
Honza, we're not folding the comparison on inlining and later ICE in tree-vrp because of this (from the einline dump):

  # fp1_5 = PHI <fp1_7(5), fp1_4(8), fp1_4(9)>
<L7>:;
  i_30 = fp0_6;
  goto <bb 14> (<L10>);

<L18>:;
  D.1689_35 = unicount[i_3];
  D.1690_36 = D.1689_35 == 0;
  D.1691_37 = i_3 != i_3;
  D.1692_38 = D.1690_36 && D.1691_37;
  if (D.1692_38) goto <L15>; else goto <L17>;

<L15>:;
  RawOrEnc = 1;


look at the i_3 != i_3
Comment 4 Richard Biener 2007-01-23 21:56:15 UTC
Created attachment 12942 [details]
somewhat reduced testcase
Comment 5 Andrew Pinski 2007-01-24 07:20:44 UTC
Reduced testcase:
static int RawOrEnc = 0;
static inline void addpair(int fp, int un)
{
  if (RawOrEnc == 0 && fp != un)
    RawOrEnc = 1;
}
int f(int un0, char *a, unsigned int __s2_len)
{
  addpair(un0, un0);
  __s2_len < 4 ? __builtin_strcmp (a, "-") : 0;
}
Comment 6 Andrew Pinski 2007-01-24 07:24:14 UTC
Remove the dead code and it works.
Comment 7 Andrew Pinski 2007-01-27 21:41:59 UTC
This works with "4.3.0 20070127" on powerpc-darwin with -O3 and -O3 -m64.
Comment 8 Andrew Pinski 2007-01-27 22:30:08 UTC
(In reply to comment #7)
> This works with "4.3.0 20070127" on powerpc-darwin with -O3 and -O3 -m64.
Ok, I had to change a paramater internal to GCC to get it to reproduce on ppc-darwin but I am able to with today's compiler.

Looking into it.
Comment 9 Andrew Pinski 2007-01-27 23:31:18 UTC
The problem is we are calling fold_marked_statements after renumbering the basic blocks and such which causes us to get the wrong starting point.
patch which I am tesing:
Index: ../../gcc/tree-inline.c
===================================================================
--- ../../gcc/tree-inline.c     (revision 121236)
+++ ../../gcc/tree-inline.c     (working copy)
@@ -2658,6 +2658,10 @@
     gimple_expand_calls_inline (bb, &id);
 
   pop_gimplify_context (NULL);
+  
+  fold_marked_statements (last, id.statements_to_fold);
+  pointer_set_destroy (id.statements_to_fold);
+  
   /* Renumber the (code) basic_blocks consecutively.  */
   compact_blocks ();
   /* Renumber the lexical scoping (non-code) blocks consecutively.  */
@@ -2683,8 +2687,6 @@
      Kill it so it won't confuse us.  */
   cgraph_node_remove_callees (id.dst_node);
 
-  fold_marked_statements (last, id.statements_to_fold);
-  pointer_set_destroy (id.statements_to_fold);
   fold_cond_expr_cond ();
   /* We make no attempts to keep dominance info up-to-date.  */
   free_dominance_info (CDI_DOMINATORS);
Comment 10 Andrew Pinski 2007-01-28 07:58:28 UTC
> The problem is we are calling fold_marked_statements after renumbering the
> basic blocks and such which causes us to get the wrong starting point.

We have to call verify_cgraph before calling fold_marked_statements, otherwise we get an ICE about an function (a builtin) which we no longer call.  This shows up in the Ada front-end in C code in which we call strlen in the function which gets inlined with a constant string.

The patch is able to pass bootstrap but I still have another regression I need to look into, dealing with an inline-asm and the testcase is x86 specific one at that. (it does #ifdef __i386__).
Comment 11 Andrew Pinski 2007-01-28 19:10:00 UTC
(In reply to comment #10)
> The patch is able to pass bootstrap but I still have another regression I need
> to look into, dealing with an inline-asm and the testcase is x86 specific one
> at that. (it does #ifdef __i386__).

SRA messes up somehow and it only happens with inlined functions so I am going to look more into it.
Comment 12 Andrew Pinski 2007-01-28 22:57:40 UTC
(In reply to comment #11)
> SRA messes up somehow and it only happens with inlined functions so I am going
> to look more into it.

I have a fix for the SRA issue now, SRA tries to look into 3rd/4th operand to decide if the array is a non constant stride but in_array_bounds_p already checks that so there is no need to check the 3rd/4th operand again.
Comment 13 Andrew Pinski 2007-05-22 23:10:56 UTC
*** Bug 32033 has been marked as a duplicate of this bug. ***
Comment 14 Andrew Pinski 2007-08-19 19:07:35 UTC
The testcases here don't crash anymore but the one from PR 32033 does:


static int spready[] = {
};
void explosion_map (int y)
{
  for (int i = 0; i < 4; i++)
    if (y * spready[i] < 0)
      break;
}
void explosion (void)
{
  explosion_map (0);
  for (int i = 0; i < 2; i++)
    continue;
}
Comment 15 Andrew Pinski 2007-08-19 19:19:45 UTC
Testing a slight modifed version of the patch.
Comment 16 Andrew Pinski 2007-08-20 07:43:10 UTC
Subject: Bug 30564

Author: pinskia
Date: Mon Aug 20 07:42:55 2007
New Revision: 127638

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=127638
Log:
2007-08-20  Andrew Pinski  <andrew_pinski@playstation.sony.com>

        PR middle-end/30564
        * tree-inline.c (optimize_inline_calls): Move the cgraph checking
        code in front of the compacting of basic blocks.
        Move the folding of statements inbetween the cgraph checking
        and compacting of basic blocks.

2007-08-20  Andrew Pinski  <andrew_pinski@playstation.sony.com>

        PR middle-end/30564
        * gcc.c-torture/compile/pr30564-1.c: New test.
        * gcc.c-torture/compile/pr30564-2.c: New test.


Added:
    trunk/gcc/testsuite/gcc.c-torture/compile/pr30564-1.c
    trunk/gcc/testsuite/gcc.c-torture/compile/pr30564-2.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-inline.c

Comment 17 Andrew Pinski 2007-08-20 07:43:42 UTC
Fixed, sorry it took me this long to test/submit/commit this patch, I had been busy with other patches and other work.