Re: c/9564: compiling sdiff causes ICE on powerpc

Re: c/9564: compiling sdiff causes ICE on powerpc

[Segher Boessenkool]

Hi,

This patch fixes it; if okay, please install on 3.3 and
mainline (and 3.2, too, if that branch is open for this kind
of thing; I believe it's not, at the moment :) ).

Does something like this need a testcase?  If so, in what
directory?  gcc.dg?  A simplified testcase is attached, but I'll
write a real testsuite thingy if wanted.


Segher



/* compile with -maltivec -mabi=altivec -g */

extern int vfork(void);

void boom(void)
{
	char buf[65536];
	vfork();
}



2003-02-04  Segher Boessenkool  <segher@koffie.nl>

	* config/rs6000/rs6000.c (altivec_frame_fixup): Remove.
	(rs6000_emit_prologue): Use rs6000_frame_related instead.


*** config/rs6000/rs6000.c~	Tue Feb  4 09:23:02 2003
--- config/rs6000/rs6000.c	Tue Feb  4 10:51:42 2003
*************** static int first_altivec_reg_to_save PAR
*** 262,268 ****
    static unsigned int compute_vrsave_mask PARAMS ((void));
    static void is_altivec_return_reg PARAMS ((rtx, void *));
    static rtx generate_set_vrsave PARAMS ((rtx, rs6000_stack_t *, int));
- static void altivec_frame_fixup PARAMS ((rtx, rtx, HOST_WIDE_INT));
    static int easy_vector_constant PARAMS ((rtx));
    \f
    /* Default register names.  */
--- 262,267 ----
*************** rs6000_emit_allocate_stack (size, copy_r
*** 9976,10007 ****
    		       REG_NOTES (insn));
    }

- /* Add a RTX_FRAME_RELATED note so that dwarf2out_frame_debug_expr
-    knows that:
-
-      (mem (plus (blah) (regXX)))
-
-    is really:
-
-      (mem (plus (blah) (const VALUE_OF_REGXX))).  */
-
- static void
- altivec_frame_fixup (insn, reg, val)
-      rtx insn, reg;
-      HOST_WIDE_INT val;
- {
-   rtx real;
-
-   real = copy_rtx (PATTERN (insn));
-
-   real = replace_rtx (real, reg, GEN_INT (val));
-
-   RTX_FRAME_RELATED_P (insn) = 1;
-   REG_NOTES (insn) = gen_rtx_EXPR_LIST (REG_FRAME_RELATED_EXPR,
- 					real,
- 					REG_NOTES (insn));
- }
-
    /* Add to 'insn' a note which is PATTERN (INSN) but with REG replaced
       with (plus:P (reg 1) VAL), and with REG2 replaced with RREG if REG2
       is not NULL.  It would be nice if dwarf2out_frame_debug_expr could
--- 9975,9980 ----
*************** rs6000_emit_prologue ()
*** 10299,10305 ****

    	    insn = emit_move_insn (mem, savereg);

! 	    altivec_frame_fixup (insn, areg, offset);
    	  }
        }

--- 10272,10279 ----

    	    insn = emit_move_insn (mem, savereg);

! 	    rs6000_frame_related (insn, frame_ptr_rtx, info->total_size,
! 				  areg, GEN_INT (offset));
    	  }
        }

Re: c/9564: compiling sdiff causes ICE on powerpc

[David Edelsohn]

	Aldy should review this.

http://gcc.gnu.org/ml/gcc-patches/2003-02/msg00275.html

David

Re: c/9564: compiling sdiff causes ICE on powerpc

[Aldy Hernandez]

On Tuesday, February 4, 2003, at 09:15  PM, David Edelsohn wrote:

> 	Aldy should review this.
>
> http://gcc.gnu.org/ml/gcc-patches/2003-02/msg00275.html
>
> David
>

Hi Segher.

The frame stuff on altivec is all bogus, so removal of the 
altivec_frame_fixup() shouldn't make things worse ;-).  Sometime I'll 
have to write some C++ code with exceptions and AltiVec code and fix 
all the notes appropriately.

Could you add a testcase in the dg directory?  After which, the patch 
is fine.

Also, you've already had half a dozen patches in gcc, I think it's time 
you get a CVS write account unless anyone else objects.  Do you have 
the paperwork filled out?

Thanks.
Aldy

Re: c/9564: compiling sdiff causes ICE on powerpc

[Segher Boessenkool]

Aldy Hernandez wrote:

> The frame stuff on altivec is all bogus, so removal of the 
> altivec_frame_fixup() shouldn't make things worse ;-).  Sometime I'll 
> have to write some C++ code with exceptions and AltiVec code and fix all 
> the notes appropriately.
> 
> Could you add a testcase in the dg directory?  After which, the patch is 
> fine.

Will do.

> Also, you've already had half a dozen patches in gcc, I think it's time 
> you get a CVS write account unless anyone else objects.  Do you have the 
> paperwork filled out?

I sent the FSF the paperwork for GCC and binutils a few months ago,
but I believe it's lost in the mail; will try again.  A CVS account isn't
all that useful to me, because of my local non-connectivity issues.
Maybe later.

Thanks,


Segher

Re: c/9564: compiling sdiff causes ICE on powerpc

[Segher Boessenkool]

Segher Boessenkool wrote:

> Aldy Hernandez wrote:
> 
>> Could you add a testcase in the dg directory?  After which, the patch 
>> is fine.
> 
> Will do.

Sorry for this late follow-up.

Well, I'm not really sure what you want to test for: the ICE
or correct generated code.  Testcase for the ICE attached,
although it's pretty useless as it won't ICE on even minor
changes to the testcase (or changes to GCC, presumably).

I'm not going to write a test for the generated code, as I
don't know enough of the various ABI's to know what it is
supposed to do; the only thing my patch did was replace
an incomplete AltiVec-specific routine by a call to a
generic routine that already handled all possible cases.

Also, instead of just testing for this particular case,
it would be more useful to have a full suite of tests
for the AltiVec ABI's.


Cheers,

Segher


[gcc.dg/altivec-9.c]

/* { dg-do compile { target powerpc-*-* } } */
/* { dg-options "-maltivec -mabi=altivec -g" } */

/* PR9564 */

extern int vfork(void);

void boom(void)
{
	char buf[65536];
	vfork();
}

Re: c/9564: compiling sdiff causes ICE on powerpc

[Aldy Hernandez]

Segher.  Thanks so much for your patch, and my apologies for the delay.

I'm committing the patch below to mainline.  No changes from your version,
except a few whitespace mods.

2003-03-10  Segher Boessenkool  <segher@koffie.nl>

	* testsuite/gcc.dg/altivec-9.c: New file.

	* config/rs6000/rs6000.c (altivec_frame_fixup): Remove.
	(rs6000_emit_prologue): Use rs6000_frame_related instead.
	
Index: testsuite/gcc.dg/altivec-9.c
===================================================================
RCS file: testsuite/gcc.dg/altivec-9.c
diff -N testsuite/gcc.dg/altivec-9.c
*** /dev/null	1 Jan 1970 00:00:00 -0000
--- testsuite/gcc.dg/altivec-9.c	10 Mar 2003 19:32:52 -0000
***************
*** 0 ****
--- 1,13 ----
+ /* { dg-do compile { target powerpc-*-* } } */
+ /* { dg-options "-maltivec -mabi=altivec -g" } */
+ 
+ /* PR9564 */
+ 
+ extern int vfork(void);
+ 
+ void
+ boom (void)
+ {
+   char buf[65536];
+   vfork();
+ }
Index: config/rs6000/rs6000.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/rs6000/rs6000.c,v
retrieving revision 1.433
diff -c -p -r1.433 rs6000.c
*** config/rs6000/rs6000.c	10 Mar 2003 18:03:17 -0000	1.433
--- config/rs6000/rs6000.c	10 Mar 2003 19:32:53 -0000
*************** static int first_altivec_reg_to_save PAR
*** 267,273 ****
  static unsigned int compute_vrsave_mask PARAMS ((void));
  static void is_altivec_return_reg PARAMS ((rtx, void *));
  static rtx generate_set_vrsave PARAMS ((rtx, rs6000_stack_t *, int));
- static void altivec_frame_fixup PARAMS ((rtx, rtx, HOST_WIDE_INT));
  static int easy_vector_constant PARAMS ((rtx));
  static bool is_ev64_opaque_type PARAMS ((tree));
  static rtx rs6000_dwarf_register_span PARAMS ((rtx));
--- 267,272 ----
*************** rs6000_emit_allocate_stack (size, copy_r
*** 10102,10133 ****
  		       REG_NOTES (insn));
  }
  
- /* Add a RTX_FRAME_RELATED note so that dwarf2out_frame_debug_expr
-    knows that:
- 
-      (mem (plus (blah) (regXX)))
- 
-    is really:
- 
-      (mem (plus (blah) (const VALUE_OF_REGXX))).  */
- 
- static void
- altivec_frame_fixup (insn, reg, val)
-      rtx insn, reg;
-      HOST_WIDE_INT val;
- {
-   rtx real;
- 
-   real = copy_rtx (PATTERN (insn));
- 
-   real = replace_rtx (real, reg, GEN_INT (val));
- 
-   RTX_FRAME_RELATED_P (insn) = 1;
-   REG_NOTES (insn) = gen_rtx_EXPR_LIST (REG_FRAME_RELATED_EXPR,
- 					real,
- 					REG_NOTES (insn));
- }
- 
  /* Add to 'insn' a note which is PATTERN (INSN) but with REG replaced
     with (plus:P (reg 1) VAL), and with REG2 replaced with RREG if REG2
     is not NULL.  It would be nice if dwarf2out_frame_debug_expr could
--- 10101,10106 ----
*************** rs6000_emit_prologue ()
*** 10494,10500 ****
  
  	    insn = emit_move_insn (mem, savereg);
  
! 	    altivec_frame_fixup (insn, areg, offset);
  	  }
      }
  
--- 10467,10474 ----
  
  	    insn = emit_move_insn (mem, savereg);
  
! 	    rs6000_frame_related (insn, frame_ptr_rtx, info->total_size,
! 				  areg, GEN_INT (offset));
  	  }
      }
  

Re: c/9564: compiling sdiff causes ICE on powerpc

[Segher Boessenkool]

Aldy Hernandez wrote:
> Segher.  Thanks so much for your patch, and my apologies for the delay.

No problem, thanks!

> I'm committing the patch below to mainline.  No changes from your version,
> except a few whitespace mods.

Heh, I thought I could use sane formatting in testcases.
Oh well :)


Segher