Bug 43742 - [4.6 Regression] web.c/union_match_dups segfaults for a null *ref on sh-elf
Summary: [4.6 Regression] web.c/union_match_dups segfaults for a null *ref on sh-elf
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 4.6.0
: P3 normal
Target Milestone: 4.6.0
Assignee: Not yet assigned to anyone
URL:
Keywords: ice-on-valid-code
Depends on:
Blocks:
 
Reported: 2010-04-13 01:38 UTC by Kazumoto Kojima
Modified: 2010-04-15 09:06 UTC (History)
3 users (show)

See Also:
Host:
Target: sh-elf
Build:
Known to work:
Known to fail:
Last reconfirmed: 2010-04-13 06:59:44


Attachments
A reduced test case (426 bytes, text/plain)
2010-04-13 01:40 UTC, Kazumoto Kojima
Details
A patch to fix the problem (521 bytes, patch)
2010-04-13 09:09 UTC, Bernd Schmidt
Details | Diff
Another attempt (433 bytes, patch)
2010-04-15 00:10 UTC, Bernd Schmidt
Details | Diff
Maybe this one. (510 bytes, patch)
2010-04-15 00:16 UTC, Bernd Schmidt
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kazumoto Kojima 2010-04-13 01:38:10 UTC
sh4-unknown-linux-gnu failed to build during compiling libgfortran
with

/exp/ldroot/dodes/xsh-gcc-orig/./gcc/xgcc -B/exp/ldroot/dodes/xsh-gcc-orig/./gcc/ -B/usr/local/sh4-unknown-linux-gnu/bin/ -B/usr/local/sh4-unknown-linux-gnu/lib/ -isystem /usr/local/sh4-unknown-linux-gnu/include -isystem /usr/local/sh4-unknown-linux-gnu/sys-include -DHAVE_CONFIG_H -I. -I../../../ORIG/trunk/libgfortran -iquote../../../ORIG/trunk/libgfortran/io -I../../../ORIG/trunk/libgfortran/../gcc -I../../../ORIG/trunk/libgfortran/../gcc/config -I../.././gcc -D_GNU_SOURCE -std=gnu99 -Wall -Wstrict-prototypes -Wmissing-prototypes -Wold-style-definition -Wextra -Wwrite-strings -fcx-fortran-rules -ffunction-sections -fdata-sections -mieee -ftree-vectorize -funroll-loops -g -O2 -MT matmul_i1.lo -MD -MP -MF .deps/matmul_i1.Tpo -c ../../../ORIG/trunk/libgfortran/generated/matmul_i1.c  -fPIC -DPIC -o .libs/matmul_i1.o
../../../ORIG/trunk/libgfortran/generated/matmul_i1.c: In function 'matmul_i1':
../../../ORIG/trunk/libgfortran/generated/matmul_i1.c:374:1: internal compiler error: Segmentation fault

It starts to fail after

r158187 | bernds | 2010-04-10 21:30:29 +0900 (Sat, 10 Apr 2010) | 7 lines

	* Makefile.in (web.o): Depend on insn-config.h and $(RECOG_H).
	* web.c: Include "insn-config.h" and "recog.h".
	(union_match_dups): New function.
	(web_main): Call it.
	(union_defs): Don't try to recognize match_dups.

and that segfault happens at the line

      (*fun) (use_entry + DF_REF_ID (*dupref), entry + DF_REF_ID (*ref));

of union_match_dups for a null *ref.
Comment 1 Kazumoto Kojima 2010-04-13 01:40:20 UTC
Created attachment 20375 [details]
A reduced test case

It fails also on sh-elf with -m4 -O -funroll-loops.

I've confirmed that the patch below can fix the failure, though
I'm not sure that this is the right thing to do.

--
	* web.c (union_match_dups): Call FUN only if *REF is non null.

diff -up ORIG/trunk/gcc/web.c trunk/gcc/web.c
--- ORIG/trunk/gcc/web.c	2010-04-12 09:52:37.000000000 +0900
+++ trunk/gcc/web.c	2010-04-12 11:14:13.000000000 +0900
@@ -123,7 +123,8 @@ union_match_dups (rtx insn, struct web_e
 	if (DF_REF_LOC (*ref) == recog_data.operand_loc[op])
 	  break;
 
-      (*fun) (use_entry + DF_REF_ID (*dupref), entry + DF_REF_ID (*ref));
+      if (*ref != NULL)
+	(*fun) (use_entry + DF_REF_ID (*dupref), entry + DF_REF_ID (*ref));
     }
 }
Comment 2 Steven Bosscher 2010-04-13 06:59:43 UTC
The patch of comment #1 is not the right thing to do. What it means, is that recog_data finds an operand for which the insn has no df_ref.

Caused by http://gcc.gnu.org/viewcvs?view=revision&revision=158187
Comment 3 Bernd Schmidt 2010-04-13 09:09:12 UTC
Created attachment 20377 [details]
A patch to fix the problem

This seems to be due to a pattern that uses a "+" constraint in an input-only operand.  The attached patch seems to fix it for me; please confirm.
Comment 4 Kazumoto Kojima 2010-04-14 00:55:03 UTC
(In reply to comment #3)
> This seems to be due to a pattern that uses a "+" constraint in an input-only
> operand.  The attached patch seems to fix it for me; please confirm.

[I'd like to add Christian to the cc list.]

Although it solves the build failure for sh4-unknown-linux-gnu
and a quick regtesting for c and c++ on that target shows no
new failures, I see some performance regressions for programs
using loops.  Looks that it makes PR target/29953 reopen.

Comment 5 Kazumoto Kojima 2010-04-14 04:52:37 UTC
It looks that simply removing the problematic constraint from
doloop_end_split restores build with no performance regressions.

	* config/sh/sh.md (doloop_end_split): Remove "+r" constraint
	in an input-only operand.	

--- ORIG/trunk/gcc/config/sh/sh.md	2010-04-12 09:52:36.000000000 +0900
+++ trunk/gcc/config/sh/sh.md	2010-04-14 10:18:47.000000000 +0900
@@ -7050,7 +7050,7 @@ label:
 
 (define_insn_and_split "doloop_end_split"
   [(set (pc)
-	(if_then_else (ne:SI (match_operand:SI 0 "arith_reg_dest" "+r")
+	(if_then_else (ne:SI (match_operand:SI 0 "arith_reg_dest" "")
 			  (const_int 1))
 		      (label_ref (match_operand 1 "" ""))
 		      (pc)))
Comment 6 Kazumoto Kojima 2010-04-14 22:04:26 UTC
I've tried to modify doloop_end* along in the line Bernd's patch
suggests, but can't find a good one yet.  Although doloop_optimize
can be modified to recognize those patterns, it isn't a very good
idea to add extra lines to the generic code for one special target.
I'll apply the patch in #5 as a work around.
Comment 7 Kazumoto Kojima 2010-04-14 23:58:25 UTC
Subject: Bug 43742

Author: kkojima
Date: Wed Apr 14 23:58:10 2010
New Revision: 158361

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=158361
Log:
	PR target/43742
	* config/sh/sh.md (doloop_end_split): Remove "+r" constraint
	in an input-only operand.	


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/sh/sh.md

Comment 8 Bernd Schmidt 2010-04-15 00:10:51 UTC
Created attachment 20382 [details]
Another attempt

The patch that was checked in looks wrong to me.  How about this one instead?
Comment 9 Bernd Schmidt 2010-04-15 00:16:47 UTC
Created attachment 20383 [details]
Maybe this one.

Actually, following the split leads to another pattern that's broken.
Comment 10 Kazumoto Kojima 2010-04-15 00:51:20 UTC
Ah, indeed.  It should be the right thing.  Thanks!
I've confirmed that it fixes the build failure with no doloop
optimization problem.  Could you please apply it to trunk?
Comment 11 Bernd Schmidt 2010-04-15 08:57:44 UTC
Subject: Bug 43742

Author: bernds
Date: Thu Apr 15 08:57:27 2010
New Revision: 158367

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=158367
Log:
	PR target/43742
	* config/sh/sh.md (doloop_end_split, dect): Undo previous patch.  Use
	matching constraints to ensure inputs match the output.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/sh/sh.md

Comment 12 Steven Bosscher 2010-04-15 09:06:09 UTC
.