public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Reload related segfaults
@ 2011-10-27  6:20 Alan Modra
  2011-10-27  6:28 ` David Miller
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Alan Modra @ 2011-10-27  6:20 UTC (permalink / raw)
  To: gcc-patches

Some recent patch has exposed a reload bug.  I'm seeing

libtool: compile:  /home/amodra/build/gcc-curr/./gcc/xgcc -B/home/amodra/build/gcc-curr/./gcc/ -B/home/amodra/gnu/powerpc-linux/bin/ -B/home/amodra/gnu/powerpc-linux/lib/ -isystem /home/amodra/gnu/powerpc-linux/include -isystem /home/amodra/gnu/powerpc-linux/sys-include -m64 -fPIC -mstrict-align -DHAVE_CONFIG_H -I. -I/home/amodra/src/gcc-current/libjava/classpath/native/fdlibm -I../../include -fexceptions -fasynchronous-unwind-tables -g -O2 -m64 -fPIC -mstrict-align -MT sf_fabs.lo -MD -MP -MF .deps/sf_fabs.Tpo -c /home/amodra/src/gcc-current/libjava/classpath/native/fdlibm/sf_fabs.c  -fPIC -DPIC -o .libs/sf_fabs.o
/home/amodra/src/gcc-current/libjava/classpath/native/fdlibm/sf_fabs.c: In function 'fabsf':
/home/amodra/src/gcc-current/libjava/classpath/native/fdlibm/sf_fabs.c:33:1: internal compiler error: Segmentation fault

(insn 11 9 15 2 (parallel [
            (set (subreg:SI (reg:SF 123 [ <retval> ]) 0)
                (and:SI (subreg:SI (reg:SF 33 1 [ x ]) 0)
                    (const_int 2147483647 [0x7fffffff])))
            (clobber (scratch:CC))
        ]) /home/amodra/src/gcc-current/libjava/classpath/native/fdlibm/sf_fabs.c:32 146 {andsi3_mc}
     (expr_list:REG_DEAD (reg:SF 33 1 [ x ])
        (nil)))

Reloads for insn # 11
Reload 0: GENERAL_REGS, RELOAD_FOR_INPUT_ADDRESS (opnum = 0), can't combine, secondary_reload_p
	reload_reg_rtx: (reg:SF 8 8)
Reload 1: GENERAL_REGS, RELOAD_FOR_OUTPUT_ADDRESS (opnum = 0), can't combine, secondary_reload_p
	reload_reg_rtx: (reg:SF 8 8)
Reload 2: reload_in (SF) = (reg:SF 33 1 [orig:123 <retval> ] [123])
	reload_out (SF) = (reg:SF 33 1 [orig:123 <retval> ] [123])
	NON_FLOAT_REGS, RELOAD_OTHER (opnum = 0)
	reload_in_reg: (reg:SF 33 1 [orig:123 <retval> ] [123])
	reload_out_reg: (reg:SF 33 1 [orig:123 <retval> ] [123])
	reload_reg_rtx: (reg:SF 10 10)
	secondary_in_reload = 0, secondary_out_reload = 1

Reload 3: reload_out (SI) = (subreg:SI (reg:SF 33 1 [orig:123 <retval> ] [123]) 0)
	GENERAL_REGS, RELOAD_FOR_OUTPUT (opnum = 0)
	reload_out_reg: (subreg:SI (reg:SF 33 1 [orig:123 <retval> ] [123]) 0)
	reload_reg_rtx: (reg:SI 9 9)
Reload 4: reload_in (SF) = (reg:SF 33 1 [ x ])
	GENERAL_REGS, RELOAD_FOR_INPUT (opnum = 1)
	reload_in_reg: (reg:SF 33 1 [ x ])
	reload_reg_rtx: (reg:SF 8 8)

Notice reload 3 has a subreg for reload_out.  We wind up in the
gen_reload code shown below, and try to use REGNO on a subreg, leading
to

(gdb) p debug_rtx(out)
(reg:DI -166922608)

Or at least, that's what you get after fixing print_rtx to not
segfault..  Bootstrap and regression test powerpc-linux in progress.
OK to apply, assuming no regressions?

	* reload1.c (gen_reload): Don't use REGNO on SUBREGs.
	* print-rtl.c (print_rtx): Don't segfault on negative regno.

Note that s/REGNO (in_rtx)/value/ in print_rtx is reasonable given
that REG in rtl.def has a format of "i00", so XINT (in_rtx, i) is
always XINT (in_rtx, 0) for a reg, which is equivalent to REGNO apart
from signedness.

Index: gcc/reload1.c
===================================================================
--- gcc/reload1.c	(revision 180542)
+++ gcc/reload1.c	(working copy)
@@ -8588,28 +8588,28 @@ gen_reload (rtx out, rtx in, int opnum, 
 	   && reg_or_subregno (in) < FIRST_PSEUDO_REGISTER
 	   && (REG_P (out)
 	       || (GET_CODE (out) == SUBREG && REG_P (SUBREG_REG (out))))
 	   && reg_or_subregno (out) < FIRST_PSEUDO_REGISTER
 	   && SECONDARY_MEMORY_NEEDED (REGNO_REG_CLASS (reg_or_subregno (in)),
 				       REGNO_REG_CLASS (reg_or_subregno (out)),
 				       GET_MODE (out)))
     {
       /* Get the memory to use and rewrite both registers to its mode.  */
       rtx loc = get_secondary_mem (in, GET_MODE (out), opnum, type);
 
       if (GET_MODE (loc) != GET_MODE (out))
-	out = gen_rtx_REG (GET_MODE (loc), REGNO (out));
+	out = gen_rtx_REG (GET_MODE (loc), reg_or_subregno (out));
 
       if (GET_MODE (loc) != GET_MODE (in))
-	in = gen_rtx_REG (GET_MODE (loc), REGNO (in));
+	in = gen_rtx_REG (GET_MODE (loc), reg_or_subregno (in));
 
       gen_reload (loc, in, opnum, type);
       gen_reload (out, loc, opnum, type);
     }
 #endif
   else if (REG_P (out) && UNARY_P (in))
     {
       rtx insn;
       rtx op1;
       rtx out_moded;
       rtx set;
 
Index: gcc/print-rtl.c
===================================================================
--- gcc/print-rtl.c	(revision 180542)
+++ gcc/print-rtl.c	(working copy)
@@ -465,13 +465,12 @@ print_rtx (const_rtx in_rtx)
 	    int value = XINT (in_rtx, i);
 	    const char *name;
 
 #ifndef GENERATOR_FILE
-	    if (REG_P (in_rtx) && value < FIRST_PSEUDO_REGISTER)
-	      fprintf (outfile, " %d %s", REGNO (in_rtx),
-		       reg_names[REGNO (in_rtx)]);
+	    if (REG_P (in_rtx) && (unsigned) value < FIRST_PSEUDO_REGISTER)
+	      fprintf (outfile, " %d %s", value, reg_names[value]);
 	    else if (REG_P (in_rtx)
-		     && value <= LAST_VIRTUAL_REGISTER)
+		     && (unsigned) value <= LAST_VIRTUAL_REGISTER)
 	      {
 		if (value == VIRTUAL_INCOMING_ARGS_REGNUM)
 		  fprintf (outfile, " %d virtual-incoming-args", value);
 		else if (value == VIRTUAL_STACK_VARS_REGNUM)

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Reload related segfaults
  2011-10-27  6:20 Reload related segfaults Alan Modra
@ 2011-10-27  6:28 ` David Miller
  2011-10-27  8:46   ` David Miller
  2011-11-04 13:44 ` Alan Modra
  2011-11-04 17:02 ` Eric Botcazou
  2 siblings, 1 reply; 8+ messages in thread
From: David Miller @ 2011-10-27  6:28 UTC (permalink / raw)
  To: amodra; +Cc: gcc-patches, ebotcazou

From: Alan Modra <amodra@gmail.com>
Date: Thu, 27 Oct 2011 13:29:56 +1030

> Some recent patch has exposed a reload bug.  I'm seeing

I think this might be a side effect or Eric's recent changes,
CC:'d.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Reload related segfaults
  2011-10-27  6:28 ` David Miller
@ 2011-10-27  8:46   ` David Miller
  2011-10-27 12:01     ` Eric Botcazou
  0 siblings, 1 reply; 8+ messages in thread
From: David Miller @ 2011-10-27  8:46 UTC (permalink / raw)
  To: amodra; +Cc: gcc-patches, ebotcazou

From: David Miller <davem@davemloft.net>
Date: Wed, 26 Oct 2011 23:07:11 -0400 (EDT)

> From: Alan Modra <amodra@gmail.com>
> Date: Thu, 27 Oct 2011 13:29:56 +1030
> 
>> Some recent patch has exposed a reload bug.  I'm seeing
> 
> I think this might be a side effect or Eric's recent changes,
> CC:'d.

Eric, I'm seeing a similar segmentation fault in reload on sparc64.
But it's in a slightly different place than Alan's crash.

Simply compile gcc.target/ultrasp12.c with "-m64 -O2 -mcpu=ultrasparc -mvis"
to see this.

The crash is in find_valid_class() called from push_reload(), via this
code block around line 1184 of reload.c:

      enum reg_class in_out_class
	= find_valid_class (outmode, GET_MODE (SUBREG_REG (out)),
			    subreg_regno_offset (REGNO (SUBREG_REG (out)),
						 GET_MODE (SUBREG_REG (out)),
						 SUBREG_BYTE (out),
						 GET_MODE (out)),
			    REGNO (SUBREG_REG (out)));

'out' is:

(subreg:DI (reg/v:V4QI 50 %f18 [orig:314 s2hi4_ ] [314]) 0)

so subreg_regno_offset() returns -1, and find_valid_class() isn't too happy
about getting "-1" for it's 'n' argument.

I suspect the test that you removed in order to fix rtl-optimization/46603
would guard against this happening.

And indeed, I confirmed that backing out the rtl-optimization/46603
fix makes the segmentation fault go away.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Reload related segfaults
  2011-10-27  8:46   ` David Miller
@ 2011-10-27 12:01     ` Eric Botcazou
  2011-10-27 13:21       ` Iain Sandoe
  2011-10-27 19:18       ` Eric Botcazou
  0 siblings, 2 replies; 8+ messages in thread
From: Eric Botcazou @ 2011-10-27 12:01 UTC (permalink / raw)
  To: David Miller; +Cc: amodra, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1045 bytes --]

> The crash is in find_valid_class() called from push_reload(), via this
> code block around line 1184 of reload.c:
>
>       enum reg_class in_out_class
> 	= find_valid_class (outmode, GET_MODE (SUBREG_REG (out)),
> 			    subreg_regno_offset (REGNO (SUBREG_REG (out)),
> 						 GET_MODE (SUBREG_REG (out)),
> 						 SUBREG_BYTE (out),
> 						 GET_MODE (out)),
> 			    REGNO (SUBREG_REG (out)));
>
> 'out' is:
>
> (subreg:DI (reg/v:V4QI 50 %f18 [orig:314 s2hi4_ ] [314]) 0)
>
> so subreg_regno_offset() returns -1, and find_valid_class() isn't too happy
> about getting "-1" for it's 'n' argument.

OK, Alan's case is the same-sized and yours is the paradoxical one.  Clearly we 
shouldn't have tried to change anything for them.

Tentative fix attached, it should restore the old behavior for the cases where 
everything was working fine before.  I'm going to give it some testing.


	PR rtl-optimization/46603
	* reload.c (push_reload): In the out case, restore previous behavior
	for subregs that don't have word mode.


-- 
Eric Botcazou

[-- Attachment #2: pr46603-2.diff --]
[-- Type: text/x-diff, Size: 818 bytes --]

Index: reload.c
===================================================================
--- reload.c	(revision 180526)
+++ reload.c	(working copy)
@@ -1140,6 +1140,14 @@ push_reload (rtx in, rtx out, rtx *inloc
 			   / UNITS_PER_WORD)))
 #endif
 		  ))
+	  || (REG_P (SUBREG_REG (out))
+	      && REGNO (SUBREG_REG (out)) < FIRST_PSEUDO_REGISTER
+	      /* The case of a word mode subreg
+		 is handled differently in the following statement.  */
+	      && ! (GET_MODE_SIZE (outmode) <= UNITS_PER_WORD
+		    && (GET_MODE_SIZE (GET_MODE (SUBREG_REG (out)))
+		        > UNITS_PER_WORD))
+	      && ! HARD_REGNO_MODE_OK (subreg_regno (out), outmode))
 	  || (secondary_reload_class (0, rclass, outmode, out) != NO_REGS
 	      && (secondary_reload_class (0, rclass, GET_MODE (SUBREG_REG (out)),
 					  SUBREG_REG (out))

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Reload related segfaults
  2011-10-27 12:01     ` Eric Botcazou
@ 2011-10-27 13:21       ` Iain Sandoe
  2011-10-27 19:18       ` Eric Botcazou
  1 sibling, 0 replies; 8+ messages in thread
From: Iain Sandoe @ 2011-10-27 13:21 UTC (permalink / raw)
  To: Eric Botcazou; +Cc: David Miller, amodra, gcc-patches


On 27 Oct 2011, at 11:33, Eric Botcazou wrote:

>> The crash is in find_valid_class() called from push_reload(), via  
>> this
>> code block around line 1184 of reload.c:
>>
>>      enum reg_class in_out_class
>> 	= find_valid_class (outmode, GET_MODE (SUBREG_REG (out)),
>> 			    subreg_regno_offset (REGNO (SUBREG_REG (out)),
>> 						 GET_MODE (SUBREG_REG (out)),
>> 						 SUBREG_BYTE (out),
>> 						 GET_MODE (out)),
>> 			    REGNO (SUBREG_REG (out)));
>>
>> 'out' is:
>>
>> (subreg:DI (reg/v:V4QI 50 %f18 [orig:314 s2hi4_ ] [314]) 0)
>>
>> so subreg_regno_offset() returns -1, and find_valid_class() isn't  
>> too happy
>> about getting "-1" for it's 'n' argument.
>
> OK, Alan's case is the same-sized and yours is the paradoxical one.   
> Clearly we
> shouldn't have tried to change anything for them.
>
> Tentative fix attached, it should restore the old behavior for the  
> cases where
> everything was working fine before.  I'm going to give it some  
> testing.
>
>
> 	PR rtl-optimization/46603
> 	* reload.c (push_reload): In the out case, restore previous behavior
> 	for subregs that don't have word mode.

powerpc darwin9 is also affected - I stage3-bubbled the patch above  
and libjava built OK (full reg-strap is a long job).
Iain

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Reload related segfaults
  2011-10-27 12:01     ` Eric Botcazou
  2011-10-27 13:21       ` Iain Sandoe
@ 2011-10-27 19:18       ` Eric Botcazou
  1 sibling, 0 replies; 8+ messages in thread
From: Eric Botcazou @ 2011-10-27 19:18 UTC (permalink / raw)
  To: David Miller; +Cc: gcc-patches, amodra

> Tentative fix attached, it should restore the old behavior for the cases
> where everything was working fine before.  I'm going to give it some
> testing.
>
>
> 	PR rtl-optimization/46603
> 	* reload.c (push_reload): In the out case, restore previous behavior
> 	for subregs that don't have word mode.

Tested on x86, x86-64 and SPARC.  Applied on the mainline.

-- 
Eric Botcazou

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Reload related segfaults
  2011-10-27  6:20 Reload related segfaults Alan Modra
  2011-10-27  6:28 ` David Miller
@ 2011-11-04 13:44 ` Alan Modra
  2011-11-04 17:02 ` Eric Botcazou
  2 siblings, 0 replies; 8+ messages in thread
From: Alan Modra @ 2011-11-04 13:44 UTC (permalink / raw)
  To: gcc-patches

Ping http://gcc.gnu.org/ml/gcc-patches/2011-10/msg02429.html

Eric fixed the bootstrap breakage by another patch, but there is a
fairly obvious bug in gen_reload fixed by my patch.  I gave enough
contect in the diff that you don't even need to look at the file. :)

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Reload related segfaults
  2011-10-27  6:20 Reload related segfaults Alan Modra
  2011-10-27  6:28 ` David Miller
  2011-11-04 13:44 ` Alan Modra
@ 2011-11-04 17:02 ` Eric Botcazou
  2 siblings, 0 replies; 8+ messages in thread
From: Eric Botcazou @ 2011-11-04 17:02 UTC (permalink / raw)
  To: Alan Modra; +Cc: gcc-patches

> OK to apply, assuming no regressions?
>
> 	* reload1.c (gen_reload): Don't use REGNO on SUBREGs.
> 	* print-rtl.c (print_rtx): Don't segfault on negative regno.

OK, thanks.

-- 
Eric Botcazou

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2011-11-04 16:53 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-10-27  6:20 Reload related segfaults Alan Modra
2011-10-27  6:28 ` David Miller
2011-10-27  8:46   ` David Miller
2011-10-27 12:01     ` Eric Botcazou
2011-10-27 13:21       ` Iain Sandoe
2011-10-27 19:18       ` Eric Botcazou
2011-11-04 13:44 ` Alan Modra
2011-11-04 17:02 ` Eric Botcazou

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).