Bug 52306 - [4.8 regression] ICE in cselib_record_set, at cselib.c:2158
Summary: [4.8 regression] ICE in cselib_record_set, at cselib.c:2158
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 4.6.2
: P2 major
Target Milestone: 4.8.5
Assignee: Jeffrey A. Law
URL:
Keywords: build
: 59536 (view as bug list)
Depends on:
Blocks:
 
Reported: 2012-02-18 23:25 UTC by Thorsten Glaser
Modified: 2015-01-19 22:33 UTC (History)
15 users (show)

See Also:
Host:
Target: m68k-*-*
Build:
Known to work: 4.4.6, 4.9.0
Known to fail: 4.6.2, 4.6.3, 4.8.1
Last reconfirmed: 2012-03-25 00:00:00


Attachments
preprocessed source of libxslt occurrence (50.33 KB, application/octet-stream)
2012-05-06 14:16 UTC, Thorsten Glaser
Details
preprocessed source of second kde4libs occurrence (184.20 KB, application/octet-stream)
2012-12-04 12:58 UTC, Thorsten Glaser
Details
preprocessed source of festival occurrence (12.76 KB, application/octet-stream)
2012-12-24 00:53 UTC, Thorsten Glaser
Details
reduced test case (592 bytes, text/plain)
2013-02-06 23:23 UTC, Mikael Pettersson
Details
Testcase from qtbase-opensource-src_5.1.0+dfsg-4 and g++ 4.8.1 (250.25 KB, application/octet-stream)
2013-08-17 17:31 UTC, Thorsten Glaser
Details
Testcase from jumpnbump (684 bytes, text/plain)
2014-02-19 22:02 UTC, Andreas Schwab
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Thorsten Glaser 2012-02-18 23:25:08 UTC
+++ This bug was initially created as a clone of Bug #43437 +++

I’m also getting one of these. […] Interestingly enough (this is a libtool build), the line with -fPIC -DPIC did not fail.

See Mikael’s explanations on #43437 for when/where the bug appears and a reduced testcase.
Comment 1 Alexandre Oliva 2012-03-25 12:00:34 UTC
The bug, duplicated by compiling attachment 26150 [details], from bug 43437 comment 16, with a cross-compiler to m68k-elf with -O -c, exposes a reload problem.  Before reload, we have this insn:

(insn 288 287 290 40 (set (reg/f:SI 124 [ D.1788 ])
        (mem/f:SI (post_inc:SI (reg:SI 145 [ ivtmp.76 ])) [5 MEM[base: D.1889_285, offset: 0B]+0 S4 A16])) pr52306.c:278 37 {*movsi_m68k2}
     (expr_list:REG_INC (reg:SI 145 [ ivtmp.76 ])
        (nil)))

reload turns this into:

(insn 639 287 640 40 (set (reg:SI 9 %a1)
        (mem/c:SI (plus:SI (reg/f:SI 14 %a6)
                (const_int -44 [0xffffffffffffffd4])) [13 %sfp+-44 S4 A16])) pr5
2306.c:278 36 {*movsi_m68k}
     (nil))

(insn 640 639 288 40 (set (mem/c:SI (plus:SI (reg/f:SI 14 %a6)
                (const_int -44 [0xffffffffffffffd4])) [13 %sfp+-44 S4 A16])
        (plus:SI (mem/c:SI (plus:SI (reg/f:SI 14 %a6)
                    (const_int -44 [0xffffffffffffffd4])) [13 %sfp+-44 S4 A16])
            (const_int 4 [0x4]))) pr52306.c:278 132 {*addsi3_internal}
     (nil))

(insn 288 640 290 40 (set (reg:SI 9 %a1)
        (mem/f:SI (post_inc:SI (reg:SI 9 %a1)) [5 MEM[base: D.1889_285, offset: 0B]+0 S4 A16])) pr52306.c:278 37 {*movsi_m68k2}
     (expr_list:REG_INC (reg:SI 9 %a1)
        (nil)))

reload correctly performs the post_inc in a separate insn, but it fails to remove the post_inc from the MEM that had it, so we end up with two concurrent modifications of the same register, which is not well-formed RTX.
Comment 2 Alexandre Oliva 2012-03-25 13:46:43 UTC
FWIW, the problem is AFAICT still present in mainline, but it is latent, for the testcase no longer triggers the problem that 4.6 did.  The only change I could find to relevant portions of the reload code was the fix for bug 47976, but it doesn't seem to fix this particular issue.  Regardless, maybe Bernd can offer some guidance since he's the reload expert ;-)

In GCC 4.6 r185770, the reloads for insn 288 are:

Reloads for insn # 288
Reload 0: reload_in (SI) = (post_inc:SI (reg:SI 145 [ ivtmp.76 ]))
        reload_out (SI) = (post_inc:SI (reg:SI 145 [ ivtmp.76 ]))
        ADDR_REGS, RELOAD_FOR_OPERAND_ADDRESS (opnum = 1), inc by 4
        reload_in_reg: (post_inc:SI (reg:SI 145 [ ivtmp.76 ]))
        reload_reg_rtx: (reg:SI 9 %a1)
Reload 1: reload_out (SI) = (reg/f:SI 124 [ D.1788 ])
        GENERAL_REGS, RELOAD_FOR_OUTPUT (opnum = 0), optional
        reload_out_reg: (reg/f:SI 124 [ D.1788 ])
Reload 2: reload_in (SI) = (mem/f:SI (post_inc:SI (reg:SI 145 [ ivtmp.76 ])) [5 MEM[base: D.1889_285, offset: 0B]+0 S4 A16])
        GENERAL_REGS, RELOAD_FOR_INPUT (opnum = 1), optional
        reload_in_reg: (mem/f:SI (post_inc:SI (reg:SI 145 [ ivtmp.76 ])) [5 MEM[base: D.1889_285, offset: 0B]+0 S4 A16])

in GCC trunk r185451, the same insn is numbered 296, and it has a single reload, for both pseudos already got REGs:

Reload 0: reload_in (SI) = (mem/f:SI (post_inc:SI (reg:SI 9 %a1 [orig:110 ivtmp.82 ] [110])) [4 MEM[base: D.1990_299, offset: 0B]+0 S4 A16])
        GENERAL_REGS, RELOAD_FOR_INPUT (opnum = 1), optional
        reload_in_reg: (mem/f:SI (post_inc:SI (reg:SI 9 %a1 [orig:110 ivtmp.82 ] [110])) [4 MEM[base: D.1990_299, offset: 0B]+0 S4 A16])
Comment 3 Thorsten Glaser 2012-05-06 14:16:07 UTC
Created attachment 27325 [details]
preprocessed source of libxslt occurrence

The same thing happens in libxslt-1.1.26 but even with PIC:

/bin/bash ../libtool --tag=CC   --mode=compile gcc -Wl,--as-needed -DHAVE_CONFIG_H -I. -I../../../libxslt -I.. -I../../.. -I../../../libxslt -I/usr/include/libxml2    -g -O2 -Wall -c -o xslt.lo ../../../libxslt/xslt.c
libtool: compile:  gcc -Wl,--as-needed -DHAVE_CONFIG_H -I. -I../../../libxslt -I.. -I../../.. -I../../../libxslt -I/usr/include/libxml2 -g -O2 -Wall -c ../../../libxslt/xslt.c  -fPIC -DPIC -o .libs/xslt.o
../../../libxslt/xslt.c: In function 'xsltParseStylesheetProcess':
../../../libxslt/xslt.c:6464:1: internal compiler error: in cselib_record_set, at cselib.c:2148

In the meantime, we’ve got 4.6.3 in Debian, and all patches recommended by Mikael Patterson are applied:

(pbuild11307)root@ara3:~/libxslt-1.1.26/build/main/libxslt # gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/m68k-linux-gnu/4.6/lto-wrapper
Target: m68k-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Debian 4.6.3-4+m68k.1' --with-bugurl=file:///usr/share/doc/gcc-4.6/README.Bugs --enable-languages=c,c++,fortran,objc,obj-c++ --prefix=/usr --program-suffix=-4.6 --enable-shared --enable-linker-build-id --with-system-zlib --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --with-gxx-include-dir=/usr/include/c++/4.6 --libdir=/usr/lib --enable-nls --with-sysroot=/ --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --enable-gnu-unique-object --disable-libssp --enable-plugin --enable-objc-gc --disable-werror --disable-multilib --enable-checking=release --build=m68k-linux-gnu --host=m68k-linux-gnu --target=m68k-linux-gnu
Thread model: posix
gcc version 4.6.3 (Debian 4.6.3-4+m68k.1) 

Lowering to -O1 allows the file to compile.
Comment 4 Thorsten Glaser 2012-05-06 17:04:12 UTC
Luckily, -Os works in the libxslt case (not in the libvirt case, though),
although, from the earlier comments, optimisation is just hiding the bug
by changing control flow.
Comment 5 Thorsten Glaser 2012-12-04 09:26:12 UTC
Also happens in src:kde4libs (= 4:4.8.4-4) on ../../kdecore/util/kpluginfactory.cpp – I tested -O1, which helped.

If there’s any need of _more_ preprocessed source… just shout, but seeing as we’ve got some already… the line is 2158 by now in Debian though, with all the changes. (gcc -O2 -g -fPIC -c x.c on Attachment 27325 [details] still triggers it.)

Though, with the three recent patches, my genattrtab time went down from six or so *hours* to two *minutes*, so I have much faster turnaround times now, thanks to Mikael Petterson who backported them ;)
Comment 6 Mikael Pettersson 2012-12-04 10:27:34 UTC
(In reply to comment #5)
> Also happens in src:kde4libs (= 4:4.8.4-4) on
> ../../kdecore/util/kpluginfactory.cpp – I tested -O1, which helped.

Just to clarify, compiling with -O1 avoids the ICE?  So you do have a workaround?

> If there’s any need of _more_ preprocessed source… just shout

Please upload the preprocessed source for this new test case.  I'd like to check if it too is silenced in 4.7 and trunk by the PR37273 fix.
Comment 7 Thorsten Glaser 2012-12-04 12:58:35 UTC
Created attachment 28876 [details]
preprocessed source of second kde4libs occurrence

OK. I found another one in the meantime, which I added. It can be triggered thus:

ara3:~# gcc -x c++ -g -fPIC -ansi -fno-exceptions -fno-check-new -fno-common -fno-threadsafe-statics -O2 -c kstatusnotifieritem.i
../../kdeui/notifications/kstatusnotifieritem.cpp: In member function 'KDbusImageVector KStatusNotifierItemPrivate::iconToVector(const QIcon&)':
../../kdeui/notifications/kstatusnotifieritem.cpp:1037:1: internal compiler error: in cselib_record_set, at cselib.c:2158
Please submit a full bug report,
with preprocessed source if appropriate.
See <file:///usr/share/doc/gcc-4.6/README.Bugs> for instructions.
Preprocessed source stored into /tmp/cc8gGDaq.out file, please attach this to your bugreport.

(I’ve just attached the .i file, they’re virtually identical save for newlines.)

The set of compiler options is not minimal, but just -O2 -g -fPIC wasn’t enough. Appending -O1 solves it.
Comment 8 Mikael Pettersson 2012-12-04 22:05:19 UTC
The new test case from kde4libs ICEs g++ 4.8-20121202 and 4.6-20121130, but not 4.7-20121201, when targeting x86-linux.  The 4.8 ICE looks as follows:

../../kdeui/notifications/kstatusnotifieritem.cpp: In member function 'KDbusImageVector KStatusNotifierItemPrivate::iconToVector(const QIcon&)':
../../kdeui/notifications/kstatusnotifieritem.cpp:1037:1: internal compiler error: in cselib_record_set, at cselib.c:2375
0x59fcc7 cselib_record_set
        /tmp/gcc-4.8-20121202/gcc/cselib.c:2375
0x59fcc7 cselib_record_sets
        /tmp/gcc-4.8-20121202/gcc/cselib.c:2592
0x5a005b cselib_process_insn(rtx_def*)
        /tmp/gcc-4.8-20121202/gcc/cselib.c:2667
0x7327f7 reload_cse_regs_1
        /tmp/gcc-4.8-20121202/gcc/postreload.c:224
0x734263 reload_cse_regs
        /tmp/gcc-4.8-20121202/gcc/postreload.c:70
0x734263 rest_of_handle_postreload
        /tmp/gcc-4.8-20121202/gcc/postreload.c:2289
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.

The older xslt test case ICEs all three of 4.8/4.7/4.6, all in cselib_record_set.

The reduced test case from PR43437 only ICEs 4.6, not 4.7 or 4.8.

I'll do some bisecting with the new kde4libs test case next.
Comment 9 Mikael Pettersson 2012-12-06 09:50:12 UTC
The ICE with gcc 4.8 on the second kde4libs test case started with Richard Biener's "Add gimple load/store predicates, use them from stmt estimates" patch in r192987:
http://gcc.gnu.org/ml/gcc-patches/2012-10/msg02728.html
http://gcc.gnu.org/ml/gcc-cvs/2012-10/msg01111.html

As far as I can tell from looking at the patch, it merely tweaks some inlining heuristics, so most likely it just exposed a latent issue.

The ICE occurs because

	  /* The register should have been invalidated.  */
	  gcc_assert (REG_VALUES (dreg)->elt == 0);

in cselib.c line 2375 fails.
Comment 10 Jakub Jelinek 2012-12-06 09:52:46 UTC
cselib.c ICEs if it sees an invalid insn, i.e. one that modifies the same register or memory more than once.  So it is a bug in whatever pass created that insn.
Comment 11 Thorsten Glaser 2012-12-24 00:53:01 UTC
Created attachment 29040 [details]
preprocessed source of festival occurrence

Here's another one, from festival (some TTS engine thingy I think). Maybe it's different enough from the others (or similar enough) for someone who knows about this stuff to figure out the root case…
Comment 12 Thorsten Glaser 2013-01-29 21:17:04 UTC
# cat /usr/bin/g++
#!/bin/mksh-static
/usr/bin/g++-4.6 "$@" && exit 0
/usr/bin/g++-4.6 "$@" -O1 && exit 0
exec /usr/bin/g++-4.6 "$@" -O0


Has anyone thought of making GCC do that automatically? It already retries on ICEs to weed out possibly bad memory… if we have an ICE like this specific case, where we *know* optimisation makes a difference and it’s a bug in the compiler, we could just retry lowering the optimisation level each time.

(My shell script is very suboptimal as it always retries twice. I’m using it only for the second time now, and both times only during the build, after all configury has been run. Some C++ code seems to exercise PR52306 more than, say, libvirt which has only one occurrence (times two)…)
Comment 13 Thorsten Glaser 2013-01-29 21:25:58 UTC
… oh, sorry, the automatic retry is apparently Debian/Feodora specific.

Still… in this very case… this ICE in CSE is hitting us often enough…
Comment 14 Thorsten Glaser 2013-01-29 23:29:54 UTC
just don’t hit me… I’m trying this now, until someone fixes this PR:

# DP: retry a known ICE with -O1 then -O0 in case it gets better

--- a/src/gcc/diagnostic.c
+++ b/src/gcc/diagnostic.c
@@ -242,6 +242,12 @@ diagnostic_action_after_output (diagnost
 	       "See %s for instructions.\n", bug_report_url);
       exit (ICE_EXIT_CODE);
 
+    case DK_TGV:
+      fnotice (stderr, "Retrying with lowered optimisation,\n"
+	       "this is a known bug, do not worry. If it'll\n"
+	       "still fail, just fail the package build.\n");
+      exit (TGV_EXIT_CODE);
+
     case DK_FATAL:
       if (context->abort_on_error)
 	real_abort ();
@@ -426,7 +432,7 @@ diagnostic_report_diagnostic (diagnostic
       /* If we're reporting an ICE in the middle of some other error,
 	 try to flush out the previous error, then let this one
 	 through.  Don't do this more than once.  */
-      if (diagnostic->kind == DK_ICE && context->lock == 1)
+      if ((diagnostic->kind == DK_ICE || diagnostic->kind == DK_TGV) && context->lock == 1)
 	pp_flush (context->printer);
       else
 	error_recursion (context);
@@ -494,7 +500,7 @@ diagnostic_report_diagnostic (diagnostic
 
   context->lock++;
 
-  if (diagnostic->kind == DK_ICE)
+  if (diagnostic->kind == DK_ICE || diagnostic->kind == DK_TGV)
     {
 #ifndef ENABLE_CHECKING
       /* When not checking, ICEs are converted to fatal errors when an
@@ -507,7 +513,7 @@ diagnostic_report_diagnostic (diagnostic
 	  expanded_location s = expand_location (diagnostic->location);
 	  fnotice (stderr, "%s:%d: confused by earlier errors, bailing out\n",
 		   s.file, s.line);
-	  exit (ICE_EXIT_CODE);
+	  exit (diagnostic->kind == DK_TGV ? TGV_EXIT_CODE : ICE_EXIT_CODE);
 	}
 #endif
       if (context->internal_error)
@@ -902,3 +908,17 @@ real_abort (void)
 {
   abort ();
 }
+
+void
+tgv_abort (const char *gmsgid, ...)
+{
+  diagnostic_info diagnostic;
+  va_list ap;
+
+  va_start (ap, gmsgid);
+  diagnostic_set_info (&diagnostic, gmsgid, &ap, input_location, DK_TGV);
+  report_diagnostic (&diagnostic);
+  va_end (ap);
+
+  gcc_unreachable ();
+}
--- a/src/gcc/diagnostic.def
+++ b/src/gcc/diagnostic.def
@@ -43,3 +43,4 @@ prefix does not matter.  */
 DEFINE_DIAGNOSTIC_KIND (DK_PEDWARN, "pedwarn: ")
 DEFINE_DIAGNOSTIC_KIND (DK_PERMERROR, "permerror: ")
 
+DEFINE_DIAGNOSTIC_KIND (DK_TGV, "internal compiler error: ")
--- a/src/gcc/system.h
+++ b/src/gcc/system.h
@@ -235,6 +235,7 @@ extern int errno;
 #endif
 
 #define ICE_EXIT_CODE 4
+#define TGV_EXIT_CODE 23
 
 #ifdef HAVE_UNISTD_H
 # include <unistd.h>
@@ -642,6 +643,15 @@ extern void fancy_abort (const char *, i
 #define gcc_assert(EXPR) ((void)(0 && (EXPR)))
 #endif
 
+extern void tgv_abort (const char *, ...) ATTRIBUTE_NORETURN;
+#if ENABLE_ASSERT_CHECKING
+#define gcc_assert_and_retry_with_lowered_optimisation_level(EXPR) 	\
+   ((void)(!(EXPR) ? tgv_abort ("in %s, at %s:%d", __FUNCTION__, trim_filename (__FILE__), __LINE__), 0 : 0))
+#else
+#define gcc_assert_and_retry_with_lowered_optimisation_level(EXPR)	\
+   gcc_assert (EXPR)
+#endif
+
 #ifdef ENABLE_CHECKING
 #define gcc_checking_assert(EXPR) gcc_assert (EXPR)
 #else
--- a/src/gcc/gcc.c
+++ b/src/gcc/gcc.c
@@ -253,6 +253,7 @@ static const char *convert_filename (con
 #if !(defined (__MSDOS__) || defined (OS2) || defined (VMS))
 static void retry_ice (const char *prog, const char **argv);
 #endif
+static const char **tgv_argv(const char **);
 
 static const char *getenv_spec_function (int, const char **);
 static const char *if_exists_spec_function (int, const char **);
@@ -2464,6 +2465,7 @@ execute (void)
 {
   int i;
   int n_commands;		/* # of command.  */
+  int retry_tgv = 0;
   char *string;
   struct pex_obj *pex;
   struct command
@@ -2529,7 +2531,8 @@ execute (void)
 
   /* If -v, print what we are about to do, and maybe query.  */
 
-  if (verbose_flag)
+  do {
+  if (verbose_flag || retry_tgv)
     {
       /* For help listings, put a blank line between sub-processes.  */
       if (print_help_list)
@@ -2659,13 +2662,12 @@ execute (void)
 	      pfatal_with_name (errmsg);
 	    }
 	}
-
-      if (i && string != commands[i].prog)
-	free (CONST_CAST (char *, string));
     }
 
   execution_count++;
 
+  retry_tgv &= ~0x80;
+
   /* Wait for all the subprocesses to finish.  */
 
   {
@@ -2717,6 +2719,26 @@ execute (void)
 	    /* For ICEs in cc1, cc1obj, cc1plus see if it is
 	       reproducible or not.  */
 	    const char *p;
+	    if (WEXITSTATUS (status) == TGV_EXIT_CODE
+		&& i == 0
+		&& (p = strrchr (commands[0].argv[0], DIR_SEPARATOR))
+		&& ! strncmp (p + 1, "cc1", 3)) {
+	      if (retry_tgv == 0) {
+		commands[0].argv = tgv_argv(commands[0].argv);
+		retry_tgv = 0x81;
+		goto foo_tgv;
+              }
+              if (retry_tgv == 1) {
+		size_t qqq = 0;
+
+		while (commands[0].argv[qqq])
+		  ++qqq;
+		commands[0].argv[--qqq] = "-O0";
+		retry_tgv = 0x82;
+		goto foo_tgv;
+              }
+	      retry_tgv = 3;
+	    }
 	    if (WEXITSTATUS (status) == ICE_EXIT_CODE
 		&& i == 0
 		&& (p = strrchr (commands[0].argv[0], DIR_SEPARATOR))
@@ -2725,6 +2747,7 @@ execute (void)
 #endif
 	    if (WEXITSTATUS (status) > greatest_status)
 	      greatest_status = WEXITSTATUS (status);
+ foo_tgv:
 	    ret_code = -1;
 	  }
 
@@ -2779,12 +2802,12 @@ execute (void)
 	      }
 	  }
       }
-
-    if (commands[0].argv[0] != commands[0].prog)
-      free (CONST_CAST (char *, commands[0].argv[0]));
-
+   if (!(retry_tgv & 0x80))
     return ret_code;
+   retry_tgv &= ~0x80;
   }
+  } while (retry_tgv < 3);
+  return TGV_EXIT_CODE;
 }
 
 /* Find all the switches given to us
@@ -8587,3 +8610,22 @@ pass_through_libs_spec_func (int argc, c
     }
   return prepended;
 }
+
+static const char **
+tgv_argv(const char **oargv)
+{
+	size_t qqq = 0;
+	const char **rv;
+
+	while (oargv[qqq])
+		++qqq;
+	rv = (const char **)xmalloc((qqq + 2) * sizeof(const char *));
+	qqq = 0;
+	while (oargv[qqq]) {
+		rv[qqq] = oargv[qqq];
+		++qqq;
+	}
+	rv[qqq++] = "-O1";
+	rv[qqq] = NULL;
+	return (rv);
+}
--- a/src/gcc/cselib.c
+++ b/src/gcc/cselib.c
@@ -2155,7 +2155,7 @@ cselib_record_set (rtx dest, cselib_val
       else
 	{
 	  /* The register should have been invalidated.  */
-	  gcc_assert (REG_VALUES (dreg)->elt == 0);
+	  gcc_assert_and_retry_with_lowered_optimisation_level (REG_VALUES (dreg)->elt == 0);
 	  REG_VALUES (dreg)->elt = src_elt;
 	}
Comment 15 Mikael Pettersson 2013-01-30 17:34:11 UTC
(In reply to comment #1)
> The bug, duplicated by compiling attachment 26150 [details], from bug 43437 comment 16,
> with a cross-compiler to m68k-elf with -O -c, exposes a reload problem.  Before
> reload, we have this insn:
> 
> (insn 288 287 290 40 (set (reg/f:SI 124 [ D.1788 ])
>         (mem/f:SI (post_inc:SI (reg:SI 145 [ ivtmp.76 ])) [5 MEM[base:
> D.1889_285, offset: 0B]+0 S4 A16])) pr52306.c:278 37 {*movsi_m68k2}
>      (expr_list:REG_INC (reg:SI 145 [ ivtmp.76 ])
>         (nil)))

But isn't this already invalid, as it has both a post_inc and a REG_INC of the same (reg:SI 145 [ ivtmp.76 ]) operand?

This is produced by the auto_inc_dec pass.  Before that we have (175r.fwprop2):

(insn 188 187 190 22 (set (reg/v:SI 76 [ i ])
        (plus:SI (reg/v:SI 76 [ i ])
            (const_int 1 [0x1]))) pr43437-2.c:222 132 {*addsi3_internal}
     (nil))

(insn 190 188 191 22 (set (cc0)
        (compare (reg/v:SI 76 [ i ])
            (reg/v:SI 68 [ n ]))) pr43437-2.c:222 14 {*m68k.md:486}
     (nil))

which auto_inc_dec turns into (176r.auto_inc_dec):

  289 r145:SI=r145:SI+0x4
  288 r124:SI=[r145:SI]
  288 r124:SI=[r145:SI]
found mem(288) *(r[145]+0)
  289 r145:SI=r145:SI+0x4
found post inc(289) r[145]+=4
trying SIMPLE_POST_INC
rescanning insn with uid = 288.
deleting insn with uid = 288.
deleting insn with uid = 289.
****success   288 r124:SI=[r145:SI++]
      REG_INC: r145:SI
...
  288 r124:SI=[r145:SI++]
      REG_INC: r145:SI
...
(insn 288 287 290 41 (set (reg/f:SI 124 [ D.1812 ])
        (mem/f:SI (post_inc:SI (reg:SI 145 [ ivtmp.80 ])) [5 MEM[base: D.1914_291, offset: 0B]+0 S4 A16])) pr43437-2.c:278 37 {*movsi_m68k2}
     (expr_list:REG_INC (reg:SI 145 [ ivtmp.80 ])
        (nil)))

Is this valid?
Comment 16 Mikael Pettersson 2013-01-30 17:50:07 UTC
Sorry I quoted the wrong fragment from 175r.fwprop, the correct fragment is:

(insn 288 287 289 41 (set (reg/f:SI 124 [ D.1812 ])
        (mem/f:SI (reg:SI 145 [ ivtmp.80 ]) [5 MEM[base: D.1914_291, offset: 0B]+0 S4 A16])) pr43437-2.c:278 37 {*movsi_m68k2}
     (nil))

(insn 289 288 290 41 (set (reg:SI 145 [ ivtmp.80 ])
        (plus:SI (reg:SI 145 [ ivtmp.80 ])
            (const_int 4 [0x4]))) pr43437-2.c:278 132 {*addsi3_internal}
     (nil))
Comment 17 Mikael Pettersson 2013-02-06 23:23:58 UTC
Created attachment 29376 [details]
reduced test case

Please disregard my last two comments, I misread the insn dump and mistook a note for the invalid concurrent modification of a register.

Thorsten's initial xslt test case, attachment 27325 [details], ICEs 4.6 and 4.7 -with -O2 -fPIC, and 4.8 with just -O2.  This new test case is substantially reduced from the initial one, but it only ICEs 4.8.  The ICE occurs when cselib processes insn 67.

IRA produced

(insn 67 66 69 10 (set (reg/f:SI 52 [ D.1521 ])
        (mem/f:SI (post_inc:SI (reg:SI 59 [ ivtmp.11 ])) [3 MEM[base: 0B, index: ivtmp.11_66, offset: 0B]+0 S4 A16])) pr52306-4.c:44 38 {*movsi_m68k2}
     (expr_list:REG_INC (reg:SI 59 [ ivtmp.11 ])
        (nil)))

with, if I read the dump correctly, reg 52 mapped to %a1 and reg 59 spilled.

Reload then sees fit to reload reg 59 into %a1, resulting in

(insn 67 253 69 10 (set (reg:SI 9 %a1)
        (mem/f:SI (post_inc:SI (reg:SI 9 %a1)) [3 MEM[base: 0B, index: ivtmp.11_66, offset: 0B]+0 S4 A16])) pr52306-4.c:44 38 {*movsi_m68k2}
     (expr_list:REG_INC (reg:SI 9 %a1)
        (nil)))

which uses %a1 both as an auto-inc source pointer and as the dest of the load, causing the assertion failure in cselib.

The post_inc of reg 59 (%a1) is actually dead, reload added insns to load %a1 from reg 59's spill slot and to increment that memory cell directly.  I'm guessing either reload should not have coalesced reg 59 with reg 52 (due to the post_inc on reg 59), or it should have cancelled the post_inc when it added the new insn to increment the spill slot directly.
Comment 18 Jakub Jelinek 2013-02-07 08:13:03 UTC
That sounds like reload bug then.  It should either not reload the destination to the same register used in POST_INC, or change the POST_INC into non-autoinc form.
Comment 19 Thorsten Glaser 2013-08-17 17:31:38 UTC
Created attachment 30668 [details]
Testcase from qtbase-opensource-src_5.1.0+dfsg-4 and g++ 4.8.1

This issue still appears with GCC 4.8

In GCC 4.6 in Debian/m68k I eventually applied a patch that simply retries the build with -O1 then -O0 to mask this issue, but the underlying issue is still unfixed. This occurs in 4.8 now too, so I guess (from a distro PoV) I need to port said patch, until someone finds out what precisely is going on here.

g++ -c x.cc -std=c++0x -fno-exceptions ⇒ compiles fine
g++ -c x.cc -std=c++0x -fno-exceptions -O2 ⇒ ICEs:
In file included from ../../../include/QtCore/qlist.h:1:0,
                 from ../../../include/QtCore/../../src/corelib/tools/qhash.h:47,
                 from ../../../include/QtCore/qhash.h:1,
                 from ../../../include/QtCore/../../src/corelib/io/qdebug.h:46,
                 from ../../../include/QtCore/qdebug.h:1,
                 from ditaxmlgenerator.cpp:46:
../../../include/QtCore/../../src/corelib/tools/qlist.h: In member function ‘void QList<T>::append(const T&) [with T = Section]’:
../../../include/QtCore/../../src/corelib/tools/qlist.h:521:1: internal compiler error: in cselib_record_set, at cselib.c:2373
Please submit a full bug report,
with preprocessed source if appropriate.
See <file:///usr/share/doc/gcc-4.8/README.Bugs> for instructions.
Preprocessed source stored into /tmp/ccUYfyzd.out file, please attach this to your bugreport.
Comment 20 Mikael Pettersson 2013-08-17 18:31:00 UTC
(In reply to Thorsten Glaser from comment #19)
> Created attachment 30668 [details]
> Testcase from qtbase-opensource-src_5.1.0+dfsg-4 and g++ 4.8.1
> 
> This issue still appears with GCC 4.8
> 
> In GCC 4.6 in Debian/m68k I eventually applied a patch that simply retries
> the build with -O1 then -O0 to mask this issue, but the underlying issue is
> still unfixed. This occurs in 4.8 now too, so I guess (from a distro PoV) I
> need to port said patch, until someone finds out what precisely is going on
> here.

Please try compiling with -O2 -fno-auto-inc-dec before dropping to -O1 or -O0.
Comment 21 Thorsten Glaser 2013-08-17 18:43:59 UTC
-fno-auto-inc-dec helps, thanks!
Comment 22 Thorsten Glaser 2013-08-22 16:28:31 UTC
The Debian GCC maintainer refuses to accept my workaround patch. I’m not sure whether he’d accept a patch to always disable -fauto-inc-dec on m68k. A “proper” fix would be welcome as lots of software, most prominently anything Qt (both 4 and 5), FTBFS without this.
Comment 23 Andreas Schwab 2013-12-19 09:02:19 UTC
*** Bug 59536 has been marked as a duplicate of this bug. ***
Comment 24 Mikael Pettersson 2013-12-19 10:40:04 UTC
So where does that leave us?  Disable -fauto-inc-dec by default, or try to make m68k work with LRA (which hopefully should avoid this reload bug)?
Comment 25 Eric Botcazou 2013-12-19 11:12:10 UTC
We should fix reload, it's too late for LRA.
Comment 26 Andreas Schwab 2013-12-19 11:22:13 UTC
What does that mean, it's too late?
Comment 27 bin.cheng 2013-12-19 11:28:26 UTC
(In reply to Andreas Schwab from comment #26)
> What does that mean, it's too late?

We are in stage 3 now, enabling LRA needs non-trivial work, so it's very likely we can't make it work in time.
Comment 28 Jeffrey A. Law 2014-02-10 05:37:21 UTC
The problem here is clearly reload.

reload has an optimization for reloading an input value of the current insn where it looks at the prior insn to see if it sets the same (unallocated) pseudo register.  If so and replacing the unallocated pseudo in the prior insn results in an insn that is recognized and satisfies its constraints, then the optimization is applied.

Perhaps RTL will make this clearly.  This is the key block after emitting reload insns for insn 71:

(code_label 87 225 69 10 13 "" [1 uses])
(note 69 87 245 10 [bb 10] NOTE_INSN_BASIC_BLOCK)
(insn 245 69 70 10 (set (reg:SI 8 %a0)
        (reg/v/f:SI 7 %d7 [orig:43 cur ] [43])) j.c:44 -1
     (nil))
(insn 70 245 246 10 (set (reg/f:SI 0 %d0 [orig:46 D.1493 ] [46])
        (mem/f:SI (reg:SI 8 %a0) [3 cur_22->prefix+0 S4 A16])) j.c:44 39 {*movsi_m68k2}
     (nil))
(insn 246 70 247 10 (set (reg:SI 8 %a0)
        (reg:SI 54 [ ivtmp.11 ])) j.c:44 -1
     (nil))
(insn 247 246 71 10 (set (reg:SI 54 [ ivtmp.11 ])
        (plus:SI (reg:SI 54 [ ivtmp.11 ])
            (const_int 4 [0x4]))) j.c:44 141 {*addsi3_internal}
     (nil))
(insn 71 247 240 10 (set (reg/f:SI 48 [ D.1497 ])
        (mem/f:SI (post_inc:SI (reg:SI 8 %a0)) [3 MEM[base: 0B, index: ivtmp.11_45, offset: 0B]+0 S4 A16])) j.c:44 39 {*movsi_m68k2}
     (expr_list:REG_INC (reg:SI 8 %a0)
        (expr_list:REG_INC (reg:SI 8 %a0)
            (nil))))
(note 240 71 73 NOTE_INSN_DELETED)
(insn 73 240 74 10 (set (cc0)
        (compare (reg/f:SI 0 %d0 [orig:46 D.1493 ] [46])
            (mem/f:SI (reg/f:SI 48 [ D.1497 ]) [3 _30->prefix+0 S4 A16]))) j.c:44 16 {*m68k.md:492}
     (expr_list:REG_DEAD (reg/f:SI 48 [ D.1497 ])
        (nil)))
(jump_insn 74 73 132 10 (set (pc)
        (if_then_else (eq (cc0)
                (const_int 0 [0]))
            (label_ref:SI 114)
            (pc))) j.c:44 405 {beq}
     (int_list:REG_BR_PROB 300 (nil))
 -> 114)



We are processing insn 73.  It has an input reload for (reg:SI 48).  The prior insn (71) has (reg:SI 48) as its output.  So ideally whatever reload register we use to satisfy (reg:SI 48) would be used for the output of 71 and the input of 73.   In this specific case, the reload register selected for the input reload of insn 73 is a0.  Substituting a0 for (reg:SI 48) results in insn 71 being a recognizable, successfully constrained insn, but it is ill-formed as it uses a0 within an auto-increment addressing mode and elsewhere in the same insn.

It's worth noting that the selection of a0 as the reload register for the input value of insn 73 is legitimate.  It's only the back-substitution of a0 into the output of insn 71 that is invalid.

A patch to address this properly in reload is in testing.
Comment 29 Jeffrey A. Law 2014-02-10 16:26:15 UTC
Author: law
Date: Mon Feb 10 16:25:44 2014
New Revision: 207662

URL: http://gcc.gnu.org/viewcvs?rev=207662&root=gcc&view=rev
Log:
	PR middle-end/52306
	* reload1.c (emit_input_reload_insns): Do not create invalid RTL
	when changing the SET_DEST of a prior insn to avoid an input
	reload.

	PR middle-end-52306
	* gcc.c-torture/compile/pr52306.c: New test.

Added:
    trunk/gcc/testsuite/gcc.c-torture/compile/pr52306.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/reload1.c
    trunk/gcc/testsuite/ChangeLog
Comment 30 Jeffrey A. Law 2014-02-10 16:26:37 UTC
Fixed on the trunk.
Comment 31 Andreas Schwab 2014-02-19 22:02:48 UTC
Created attachment 32175 [details]
Testcase from jumpnbump

After backporting the patch to 4.8 there is still a package that fails with the same error, though it doesn't fail with 4.9.

$ gcc/xgcc -B gcc/ -O -fomit-frame-pointer filter.i 
filter.i: In function ‘do_scale2x’:
filter.i:202:1: internal compiler error: in cselib_record_set, at cselib.c:2373
 }
 ^
0x5c092e cselib_record_set
        ../../gcc/gcc/cselib.c:2373
0x5c092e cselib_record_sets
        ../../gcc/gcc/cselib.c:2590
0x5c0c8f cselib_process_insn(rtx_def*)
        ../../gcc/gcc/cselib.c:2665
0x753c27 reload_cse_regs_1
        ../../gcc/gcc/postreload.c:222
0x75426b reload_cse_regs
        ../../gcc/gcc/postreload.c:68
0x75426b rest_of_handle_postreload
        ../../gcc/gcc/postreload.c:2287
Comment 32 Mikael Pettersson 2014-02-23 14:20:54 UTC
(In reply to Andreas Schwab from comment #31)
> After backporting the patch to 4.8 there is still a package that fails with
> the same error, though it doesn't fail with 4.9.

This ICE stopped on trunk with Bin Cheng's "Compute, cache and use cost of auto-increment rtx patterns in IVOPT" patch in r205015, see also

http://gcc.gnu.org/ml/gcc-patches/2013-11/msg00156.html

Backporting this too to 4.8 stops the ICE there for this test case.  However, this patch is just a missed-optimization tweak in ivopts, so I suspect it hides the underlying bug rather than fixing it.
Comment 33 Richard Biener 2014-05-22 09:05:48 UTC
GCC 4.8.3 is being released, adjusting target milestone.
Comment 34 Jakub Jelinek 2014-12-19 13:37:00 UTC
GCC 4.8.4 has been released.
Comment 35 Jeffrey A. Law 2015-01-19 22:33:50 UTC
Fixed in 4.9 and on trunk.  Not planning to backport to 4.8.