Bug 7114 - ICE building strcoll.op from glibc-2.2.5
ICE building strcoll.op from glibc-2.2.5
Status: RESOLVED FIXED
Product: gcc
Classification: Unclassified
Component: other
3.1
: P3 normal
: ---
Assigned To: Alan Modra
: ice-on-valid-code
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2002-06-25 01:26 UTC by d.mueller
Modified: 2003-07-25 17:33 UTC (History)
5 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments
strcoll.i (90.02 KB, text/x-c)
2003-05-21 15:17 UTC, d.mueller
Details

Note You need to log in before you can comment on or make changes to this bug.
Description d.mueller 2002-06-25 01:26:00 UTC
If the "-mmultiple" options is specified, GCC throws an ICE while compiling the profiled version of strcoll.c from the "string" subdirectory of glibc-2.2.5. If the "-mmultiple" option is removed, everything is fine.

Release:
gcc-3.1

Environment:
PowerPC PPC405GP based hardware, native gcc-3.1, native binutils-2.12.1, glibc-2.2.5

How-To-Repeat:
> gcc strcoll.i -c -O2 -Wall -Wbad-function-cast -Wcast-qual -Wcomment -Wcomments -Wfloat-equal -Winline -Wmissing-declarations -Wmissing-noreturn -Wmissing-prototypes -Wmultichar -Wsign-compare -Wstrict-prototypes -Wtrigraphs -Wwrite-strings -mmultiple -msoft-float -mnew-mnemonics -Wa,-mppc -mpowerpc -pg -nostdinc -o ~/strcoll.op
strcoll.c: In function `findidx':
strcoll.c:549: Attempt to delete prologue/epilogue insn:
(insn 693 692 695 (parallel[ 
            (set (reg:SI 30 r30)
                (mem:SI (plus:SI (reg/f:SI 1 r1)
                        (const_int 24 [0x18])) [1 S4 A8]))
            (set (reg/f:SI 31 r31)
                (mem:SI (plus:SI (reg/f:SI 1 r1)
                        (const_int 28 [0x1c])) [1 S4 A8]))
        ] ) -1 (nil)
    (nil))
strcoll.c:549: Internal compiler error in propagate_one_insn, at flow.c:1615
Please submit a full bug report,
with preprocessed source if appropriate.
See <URL:http://www.gnu.org/software/gcc/bugs.html> for instructions.

> gcc strcoll.i -c -O2 -Wall -Wbad-function-cast -Wcast-qual -Wcomment -Wcomments -Wfloat-equal -Winline -Wmissing-declarations -Wmissing-noreturn -Wmissing-prototypes -Wmultichar -Wsign-compare -Wstrict-prototypes -Wtrigraphs -Wwrite-strings -msoft-float -mnew-mnemonics -Wa,-mppc -mpowerpc -pg -nostdinc -o ~/strcoll.op
Comment 1 Geoff Keating 2002-07-15 12:43:02 UTC
From: Geoff Keating <geoffk@geoffk.org>
To: amodra@bigpond.net.au
Cc: d.mueller@elsoft.ch, gcc-gnats@gcc.gnu.org, gcc-patches@gcc.gnu.org,
   dje@watson.ibm.com
Subject: Re: other/7114: ICE building strcoll.op from glibc-2.2.5
Date: Mon, 15 Jul 2002 12:43:02 -0700

 > Date: Mon, 15 Jul 2002 18:56:03 +0930
 > From: Alan Modra <amodra@bigpond.net.au>
 
 > This patch cures the testcase.  The !using_store_multiple code tests
 > whether regs are live before saving.  We need to do something similar
 > for using_store_multiple, in case all regs need not be saved.
 
 Those registers are actually saved, whether they need to be or not,
 correct?
 
 So the RTL generated is an accurate representation of the instruction,
 and the bug must be elsewhere.
 
 > 	* config/rs6000/rs6000.c (rs6000_emit_prologue): Trim saved regs
 > 	for -mmultiple case like -mno-multiple case.
 > 	(rs6000_emit_epilogue): Likewise.
 > 
 > Index: gcc/config/rs6000/rs6000.c
 > ===================================================================
 > RCS file: /cvs/gcc/gcc/gcc/config/rs6000/rs6000.c,v
 > retrieving revision 1.291.2.13
 > diff -u -p -r1.291.2.13 rs6000.c
 > --- gcc/config/rs6000/rs6000.c	23 May 2002 23:22:44 -0000	1.291.2.13
 > +++ gcc/config/rs6000/rs6000.c	15 Jul 2002 09:06:31 -0000
 > @@ -8836,30 +8836,45 @@ rs6000_emit_prologue ()
 >       the store-multiple instructions.  */
 >    if (using_store_multiple)
 >      {
 > -      rtvec p, dwarfp;
 > -      int i;
 > -      p = rtvec_alloc (32 - info->first_gp_reg_save);
 > -      dwarfp = rtvec_alloc (32 - info->first_gp_reg_save);
 > -      for (i = 0; i < 32 - info->first_gp_reg_save; i++)
 > +      int n = info->first_gp_reg_save;
 > +
 > +      while (n < 32
 > +	     && !((regs_ever_live[n]
 > +		   && ! call_used_regs[n])
 > +		  || (n == RS6000_PIC_OFFSET_TABLE_REGNUM
 > +		      && ((DEFAULT_ABI == ABI_V4 && flag_pic == 1)
 > +			  || (DEFAULT_ABI == ABI_DARWIN && flag_pic)))))
 > +	n++;
 > +
 > +      if (n < 32)
 >  	{
 > -	  rtx addr, reg, mem;
 > -	  reg = gen_rtx_REG (reg_mode, info->first_gp_reg_save + i);
 > -	  addr = gen_rtx_PLUS (Pmode, frame_reg_rtx, 
 > -			       GEN_INT (info->gp_save_offset 
 > -					+ sp_offset 
 > -					+ reg_size * i));
 > -	  mem = gen_rtx_MEM (reg_mode, addr);
 > -	  set_mem_alias_set (mem, rs6000_sr_alias_set);
 > +	  rtvec p, dwarfp;
 > +	  int i;
 > +
 > +	  p = rtvec_alloc (32 - n);
 > +	  dwarfp = rtvec_alloc (32 - n);
 > +	  for (i = 0; i < 32 - n; i++)
 > +	    {
 > +	      rtx addr, reg, mem;
 > +	      reg = gen_rtx_REG (reg_mode, n + i);
 > +	      addr = gen_rtx_PLUS (Pmode, frame_reg_rtx, 
 > +				   GEN_INT (info->gp_save_offset 
 > +					    + sp_offset 
 > +					    + reg_size * i));
 > +	      mem = gen_rtx_MEM (reg_mode, addr);
 > +	      set_mem_alias_set (mem, rs6000_sr_alias_set);
 >  
 > -	  RTVEC_ELT (p, i) = gen_rtx_SET (VOIDmode, mem, reg);
 > +	      RTVEC_ELT (p, i) = gen_rtx_SET (VOIDmode, mem, reg);
 > +	    }
 > +	  insn = emit_insn (gen_rtx_PARALLEL (VOIDmode, p));
 > +	  rs6000_frame_related (insn, frame_ptr_rtx, info->total_size, 
 > +				NULL_RTX, NULL_RTX);
 >  	}
 > -      insn = emit_insn (gen_rtx_PARALLEL (VOIDmode, p));
 > -      rs6000_frame_related (insn, frame_ptr_rtx, info->total_size, 
 > -			    NULL_RTX, NULL_RTX);
 >      }
 >    else
 >      {
 >        int i;
 > +
 >        for (i = 0; i < 32 - info->first_gp_reg_save; i++)
 >  	if ((regs_ever_live[info->first_gp_reg_save+i] 
 >  	     && ! call_used_regs[info->first_gp_reg_save+i])
 > @@ -9226,24 +9241,38 @@ rs6000_emit_epilogue (sibcall)
 >       the load-multiple instructions.  */
 >    if (using_load_multiple)
 >      {
 > -      rtvec p;
 > -      p = rtvec_alloc (32 - info->first_gp_reg_save);
 > -      for (i = 0; i < 32 - info->first_gp_reg_save; i++)
 > -	{
 > -	  rtx addr = gen_rtx_PLUS (Pmode, frame_reg_rtx, 
 > -				   GEN_INT (info->gp_save_offset 
 > -					    + sp_offset 
 > -					    + reg_size * i));
 > -	  rtx mem = gen_rtx_MEM (reg_mode, addr);
 > +      int n = info->first_gp_reg_save;
 >  
 > -	  set_mem_alias_set (mem, rs6000_sr_alias_set);
 > +      while (n < 32
 > +	     && !((regs_ever_live[n]
 > +		   && ! call_used_regs[n])
 > +		  || (n == RS6000_PIC_OFFSET_TABLE_REGNUM
 > +		      && ((DEFAULT_ABI == ABI_V4 && flag_pic == 1)
 > +			  || (DEFAULT_ABI == ABI_DARWIN && flag_pic)))))
 > +	n++;
 > +
 > +      if (n < 32)
 > +	{
 > +	  rtvec p;
 >  
 > -	  RTVEC_ELT (p, i) = 
 > -	    gen_rtx_SET (VOIDmode,
 > -			 gen_rtx_REG (reg_mode, info->first_gp_reg_save + i),
 > -			 mem);
 > +	  p = rtvec_alloc (32 - n);
 > +	  for (i = 0; i < 32 - n; i++)
 > +	    {
 > +	      rtx addr = gen_rtx_PLUS (Pmode, frame_reg_rtx, 
 > +				       GEN_INT (info->gp_save_offset 
 > +						+ sp_offset 
 > +						+ reg_size * i));
 > +	      rtx mem = gen_rtx_MEM (reg_mode, addr);
 > +
 > +	      set_mem_alias_set (mem, rs6000_sr_alias_set);
 > +
 > +	      RTVEC_ELT (p, i) = 
 > +		gen_rtx_SET (VOIDmode,
 > +			     gen_rtx_REG (reg_mode, n + i),
 > +			     mem);
 > +	    }
 > +	  emit_insn (gen_rtx_PARALLEL (VOIDmode, p));
 >  	}
 > -      emit_insn (gen_rtx_PARALLEL (VOIDmode, p));
 >      }
 >    else
 >      for (i = 0; i < 32 - info->first_gp_reg_save; i++)
 > 
 > -- 
 > Alan Modra
 > IBM OzLabs - Linux Technology Centre
 > 
 
 
 -- 
 - Geoffrey Keating <geoffk@geoffk.org> <geoffk@redhat.com>

Comment 2 Alan Modra 2002-07-15 18:56:03 UTC
From: Alan Modra <amodra@bigpond.net.au>
To: d.mueller@elsoft.ch
Cc: gcc-gnats@gcc.gnu.org, gcc-patches@gcc.gnu.org,
  David Edelsohn <dje@watson.ibm.com>, geoffk@redhat.com
Subject: Re: other/7114: ICE building strcoll.op from glibc-2.2.5
Date: Mon, 15 Jul 2002 18:56:03 +0930

 This patch cures the testcase.  The !using_store_multiple code tests
 whether regs are live before saving.  We need to do something similar
 for using_store_multiple, in case all regs need not be saved.
 
 	* config/rs6000/rs6000.c (rs6000_emit_prologue): Trim saved regs
 	for -mmultiple case like -mno-multiple case.
 	(rs6000_emit_epilogue): Likewise.
 
 Index: gcc/config/rs6000/rs6000.c
 ===================================================================
 RCS file: /cvs/gcc/gcc/gcc/config/rs6000/rs6000.c,v
 retrieving revision 1.291.2.13
 diff -u -p -r1.291.2.13 rs6000.c
 --- gcc/config/rs6000/rs6000.c	23 May 2002 23:22:44 -0000	1.291.2.13
 +++ gcc/config/rs6000/rs6000.c	15 Jul 2002 09:06:31 -0000
 @@ -8836,30 +8836,45 @@ rs6000_emit_prologue ()
       the store-multiple instructions.  */
    if (using_store_multiple)
      {
 -      rtvec p, dwarfp;
 -      int i;
 -      p = rtvec_alloc (32 - info->first_gp_reg_save);
 -      dwarfp = rtvec_alloc (32 - info->first_gp_reg_save);
 -      for (i = 0; i < 32 - info->first_gp_reg_save; i++)
 +      int n = info->first_gp_reg_save;
 +
 +      while (n < 32
 +	     && !((regs_ever_live[n]
 +		   && ! call_used_regs[n])
 +		  || (n == RS6000_PIC_OFFSET_TABLE_REGNUM
 +		      && ((DEFAULT_ABI == ABI_V4 && flag_pic == 1)
 +			  || (DEFAULT_ABI == ABI_DARWIN && flag_pic)))))
 +	n++;
 +
 +      if (n < 32)
  	{
 -	  rtx addr, reg, mem;
 -	  reg = gen_rtx_REG (reg_mode, info->first_gp_reg_save + i);
 -	  addr = gen_rtx_PLUS (Pmode, frame_reg_rtx, 
 -			       GEN_INT (info->gp_save_offset 
 -					+ sp_offset 
 -					+ reg_size * i));
 -	  mem = gen_rtx_MEM (reg_mode, addr);
 -	  set_mem_alias_set (mem, rs6000_sr_alias_set);
 +	  rtvec p, dwarfp;
 +	  int i;
 +
 +	  p = rtvec_alloc (32 - n);
 +	  dwarfp = rtvec_alloc (32 - n);
 +	  for (i = 0; i < 32 - n; i++)
 +	    {
 +	      rtx addr, reg, mem;
 +	      reg = gen_rtx_REG (reg_mode, n + i);
 +	      addr = gen_rtx_PLUS (Pmode, frame_reg_rtx, 
 +				   GEN_INT (info->gp_save_offset 
 +					    + sp_offset 
 +					    + reg_size * i));
 +	      mem = gen_rtx_MEM (reg_mode, addr);
 +	      set_mem_alias_set (mem, rs6000_sr_alias_set);
  
 -	  RTVEC_ELT (p, i) = gen_rtx_SET (VOIDmode, mem, reg);
 +	      RTVEC_ELT (p, i) = gen_rtx_SET (VOIDmode, mem, reg);
 +	    }
 +	  insn = emit_insn (gen_rtx_PARALLEL (VOIDmode, p));
 +	  rs6000_frame_related (insn, frame_ptr_rtx, info->total_size, 
 +				NULL_RTX, NULL_RTX);
  	}
 -      insn = emit_insn (gen_rtx_PARALLEL (VOIDmode, p));
 -      rs6000_frame_related (insn, frame_ptr_rtx, info->total_size, 
 -			    NULL_RTX, NULL_RTX);
      }
    else
      {
        int i;
 +
        for (i = 0; i < 32 - info->first_gp_reg_save; i++)
  	if ((regs_ever_live[info->first_gp_reg_save+i] 
  	     && ! call_used_regs[info->first_gp_reg_save+i])
 @@ -9226,24 +9241,38 @@ rs6000_emit_epilogue (sibcall)
       the load-multiple instructions.  */
    if (using_load_multiple)
      {
 -      rtvec p;
 -      p = rtvec_alloc (32 - info->first_gp_reg_save);
 -      for (i = 0; i < 32 - info->first_gp_reg_save; i++)
 -	{
 -	  rtx addr = gen_rtx_PLUS (Pmode, frame_reg_rtx, 
 -				   GEN_INT (info->gp_save_offset 
 -					    + sp_offset 
 -					    + reg_size * i));
 -	  rtx mem = gen_rtx_MEM (reg_mode, addr);
 +      int n = info->first_gp_reg_save;
  
 -	  set_mem_alias_set (mem, rs6000_sr_alias_set);
 +      while (n < 32
 +	     && !((regs_ever_live[n]
 +		   && ! call_used_regs[n])
 +		  || (n == RS6000_PIC_OFFSET_TABLE_REGNUM
 +		      && ((DEFAULT_ABI == ABI_V4 && flag_pic == 1)
 +			  || (DEFAULT_ABI == ABI_DARWIN && flag_pic)))))
 +	n++;
 +
 +      if (n < 32)
 +	{
 +	  rtvec p;
  
 -	  RTVEC_ELT (p, i) = 
 -	    gen_rtx_SET (VOIDmode,
 -			 gen_rtx_REG (reg_mode, info->first_gp_reg_save + i),
 -			 mem);
 +	  p = rtvec_alloc (32 - n);
 +	  for (i = 0; i < 32 - n; i++)
 +	    {
 +	      rtx addr = gen_rtx_PLUS (Pmode, frame_reg_rtx, 
 +				       GEN_INT (info->gp_save_offset 
 +						+ sp_offset 
 +						+ reg_size * i));
 +	      rtx mem = gen_rtx_MEM (reg_mode, addr);
 +
 +	      set_mem_alias_set (mem, rs6000_sr_alias_set);
 +
 +	      RTVEC_ELT (p, i) = 
 +		gen_rtx_SET (VOIDmode,
 +			     gen_rtx_REG (reg_mode, n + i),
 +			     mem);
 +	    }
 +	  emit_insn (gen_rtx_PARALLEL (VOIDmode, p));
  	}
 -      emit_insn (gen_rtx_PARALLEL (VOIDmode, p));
      }
    else
      for (i = 0; i < 32 - info->first_gp_reg_save; i++)
 
 -- 
 Alan Modra
 IBM OzLabs - Linux Technology Centre

Comment 3 Alan Modra 2002-07-15 19:08:59 UTC
Responsible-Changed-From-To: unassigned->amodra
Responsible-Changed-Why: Patch submitted.
Comment 4 Alan Modra 2002-07-15 19:08:59 UTC
State-Changed-From-To: open->analyzed
State-Changed-Why: Patch submitted.
Comment 5 Richard Henderson 2002-07-15 21:31:24 UTC
From: Richard Henderson <rth@redhat.com>
To: Geoff Keating <geoffk@redhat.com>, d.mueller@elsoft.ch,
   gcc-gnats@gcc.gnu.org, gcc-patches@gcc.gnu.org, dje@watson.ibm.com
Cc:  
Subject: Re: other/7114: ICE building strcoll.op from glibc-2.2.5
Date: Mon, 15 Jul 2002 21:31:24 -0700

 On Tue, Jul 16, 2002 at 11:08:16AM +0930, Alan Modra wrote:
 > It gets worse.  rs6000/sysv4.h sets PROFILE_BEFORE_PROLOGUE.
 
 Make rs6000/sysv4.h can use PROFILE_HOOK instead.
 
 I don't know what the ppc svr4 _mcount abi looks like, but I know that
 you can emit any sort of rtl you want, which pretty much means that you
 can avoid all of these register allocation problems by making an actual
 register allocator take care of them.
 
 
 r~

Comment 6 Alan Modra 2002-07-16 09:20:27 UTC
From: Alan Modra <amodra@bigpond.net.au>
To: Geoff Keating <geoffk@redhat.com>
Cc: d.mueller@elsoft.ch, gcc-gnats@gcc.gnu.org, gcc-patches@gcc.gnu.org,
  dje@watson.ibm.com
Subject: Re: other/7114: ICE building strcoll.op from glibc-2.2.5
Date: Tue, 16 Jul 2002 09:20:27 +0930

 On Mon, Jul 15, 2002 at 12:43:02PM -0700, Geoff Keating wrote:
 > > Date: Mon, 15 Jul 2002 18:56:03 +0930
 > > From: Alan Modra <amodra@bigpond.net.au>
 > 
 > > This patch cures the testcase.  The !using_store_multiple code tests
 > > whether regs are live before saving.  We need to do something similar
 > > for using_store_multiple, in case all regs need not be saved.
 > 
 > Those registers are actually saved, whether they need to be or not,
 > correct?
 > 
 > So the RTL generated is an accurate representation of the instruction,
 > and the bug must be elsewhere.
 
 The testcase saves r30 and r31, but both are marked unused (don't
 appear in regs_ever_live).  Later rtl analysis decides that the
 save instruction can be eliminated, thus the ICE.  The real bug is
 that r30 is not marked used when current_function_needs_context.
 This is also the reason for PR5967.
 
 The code that I copied from the !using_store_multiple case just
 papers over this bug.  So the above patch merely makes -mmultiple
 and -mno-multiple consistently wrong.
 
 -- 
 Alan Modra
 IBM OzLabs - Linux Technology Centre

Comment 7 Geoff Keating 2002-07-16 10:45:33 UTC
From: Geoff Keating <geoffk@geoffk.org>
To: amodra@bigpond.net.au
Cc: d.mueller@elsoft.ch, gcc-gnats@gcc.gnu.org, gcc-patches@gcc.gnu.org,
   dje@watson.ibm.com
Subject: Re: other/7114: ICE building strcoll.op from glibc-2.2.5
Date: Tue, 16 Jul 2002 10:45:33 -0700

 > Date: Tue, 16 Jul 2002 09:20:27 +0930
 > From: Alan Modra <amodra@bigpond.net.au>
 
 > On Mon, Jul 15, 2002 at 12:43:02PM -0700, Geoff Keating wrote:
 > > > Date: Mon, 15 Jul 2002 18:56:03 +0930
 > > > From: Alan Modra <amodra@bigpond.net.au>
 > > 
 > > > This patch cures the testcase.  The !using_store_multiple code tests
 > > > whether regs are live before saving.  We need to do something similar
 > > > for using_store_multiple, in case all regs need not be saved.
 > > 
 > > Those registers are actually saved, whether they need to be or not,
 > > correct?
 > > 
 > > So the RTL generated is an accurate representation of the instruction,
 > > and the bug must be elsewhere.
 > 
 > The testcase saves r30 and r31, but both are marked unused (don't
 > appear in regs_ever_live).  Later rtl analysis decides that the
 > save instruction can be eliminated, thus the ICE.  The real bug is
 > that r30 is not marked used when current_function_needs_context.
 > This is also the reason for PR5967.
 > 
 > The code that I copied from the !using_store_multiple case just
 > papers over this bug.  So the above patch merely makes -mmultiple
 > and -mno-multiple consistently wrong.
 
 No, the code below causes unused registers to actually not be saved,
 which is correct (it does sometimes happen that all uses of a register
 are eliminated after reload).  This can be done when individual loads
 and stores are being used, you just don't emit that store.  It can't
 be done when store-multiple is being used.
 
 -- 
 - Geoffrey Keating <geoffk@geoffk.org> <geoffk@redhat.com>

Comment 8 Geoff Keating 2002-07-16 10:48:14 UTC
From: Geoff Keating <geoffk@geoffk.org>
To: amodra@bigpond.net.au
Cc: d.mueller@elsoft.ch, gcc-gnats@gcc.gnu.org, gcc-patches@gcc.gnu.org,
   dje@watson.ibm.com
Subject: Re: other/7114: ICE building strcoll.op from glibc-2.2.5
Date: Tue, 16 Jul 2002 10:48:14 -0700

 > Date: Tue, 16 Jul 2002 11:08:16 +0930
 > From: Alan Modra <amodra@bigpond.net.au>
 > Mail-Followup-To: Geoff Keating <geoffk@redhat.com>, d.mueller@elsoft.ch,
 > 	gcc-gnats@gcc.gnu.org, gcc-patches@gcc.gnu.org, dje@watson.ibm.com
 > Content-Disposition: inline
 > User-Agent: Mutt/1.3.25i
 > 
 > On Tue, Jul 16, 2002 at 09:20:27AM +0930, Alan Modra wrote:
 > > The testcase saves r30 and r31, but both are marked unused (don't
 > > appear in regs_ever_live).  Later rtl analysis decides that the
 > > save instruction can be eliminated, thus the ICE.  The real bug is
 > > that r30 is not marked used when current_function_needs_context.
 > > This is also the reason for PR5967.
 > > 
 > > The code that I copied from the !using_store_multiple case just
 > > papers over this bug.  So the above patch merely makes -mmultiple
 > > and -mno-multiple consistently wrong.
 > 
 > It gets worse.  rs6000/sysv4.h sets PROFILE_BEFORE_PROLOGUE.  That
 > means it is completely wrong for first_reg_to_save to decide r30
 > needs saving when profiling and current_function_needs_context.
 > It's too late to think about saving r30.  What really needs to
 > happen is one of
 > 
 > a) Don't use PROFILE_BEFORE_PROLOGUE.
 > or
 > b) Save the static chain reg on the stack somewhere before the mcount
 >    call.
 > or
 > c) Clobber r30 in a call to a nested function when profiling is
 >    enabled.
 > 
 > People would probably scream if we went with (a) as special purpose
 > implementations of mcount might make assumptions about the stack.
 > So, anyone want to speak up on the best way to solve this?
 
 The profiling function isn't allowed to clobber r30.  It never has
 been, so this should be no surprise.
 
 -- 
 - Geoffrey Keating <geoffk@geoffk.org> <geoffk@redhat.com>

Comment 9 Alan Modra 2002-07-16 11:08:16 UTC
From: Alan Modra <amodra@bigpond.net.au>
To: Geoff Keating <geoffk@redhat.com>, d.mueller@elsoft.ch,
  gcc-gnats@gcc.gnu.org, gcc-patches@gcc.gnu.org, dje@watson.ibm.com
Cc:  
Subject: Re: other/7114: ICE building strcoll.op from glibc-2.2.5
Date: Tue, 16 Jul 2002 11:08:16 +0930

 On Tue, Jul 16, 2002 at 09:20:27AM +0930, Alan Modra wrote:
 > The testcase saves r30 and r31, but both are marked unused (don't
 > appear in regs_ever_live).  Later rtl analysis decides that the
 > save instruction can be eliminated, thus the ICE.  The real bug is
 > that r30 is not marked used when current_function_needs_context.
 > This is also the reason for PR5967.
 > 
 > The code that I copied from the !using_store_multiple case just
 > papers over this bug.  So the above patch merely makes -mmultiple
 > and -mno-multiple consistently wrong.
 
 It gets worse.  rs6000/sysv4.h sets PROFILE_BEFORE_PROLOGUE.  That
 means it is completely wrong for first_reg_to_save to decide r30
 needs saving when profiling and current_function_needs_context.
 It's too late to think about saving r30.  What really needs to
 happen is one of
 
 a) Don't use PROFILE_BEFORE_PROLOGUE.
 or
 b) Save the static chain reg on the stack somewhere before the mcount
    call.
 or
 c) Clobber r30 in a call to a nested function when profiling is
    enabled.
 
 People would probably scream if we went with (a) as special purpose
 implementations of mcount might make assumptions about the stack.
 So, anyone want to speak up on the best way to solve this?
 
 -- 
 Alan Modra
 IBM OzLabs - Linux Technology Centre

Comment 10 Alan Modra 2002-07-16 14:38:08 UTC
From: Alan Modra <amodra@bigpond.net.au>
To: Richard Henderson <rth@redhat.com>, Geoff Keating <geoffk@redhat.com>,
  d.mueller@elsoft.ch, gcc-gnats@gcc.gnu.org, gcc-patches@gcc.gnu.org,
  dje@watson.ibm.com
Cc:  
Subject: Re: other/7114: ICE building strcoll.op from glibc-2.2.5
Date: Tue, 16 Jul 2002 14:38:08 +0930

 On Mon, Jul 15, 2002 at 09:31:24PM -0700, Richard Henderson wrote:
 > On Tue, Jul 16, 2002 at 11:08:16AM +0930, Alan Modra wrote:
 > > It gets worse.  rs6000/sysv4.h sets PROFILE_BEFORE_PROLOGUE.
 > 
 > Make rs6000/sysv4.h can use PROFILE_HOOK instead.
 
 Aye, that's the nice way to do it.  However, on powerpc64-linux,
 I've had kernel people complaining that the profiling code isn't
 what they want:  All those register saves from the prologue
 preceding the mcount call apparently are messing up accurate
 count values, and it's hard for an mcount implementation to
 adjust times, or so I'm told.  I implemented a simple hack to
 do PROFILE_BEFORE_PROLOGUE on powerpc64-linux for people who
 want it.
 
 I suspect we'll get the same sort of complaint if we change
 powerpc mcount.  There's also the issue that some special-purpose
 mcount functions may expect to be called before the stack has
 been adjusted.
 
 I'm rapidly approaching the point where I either give up on this
 problem, or simply remove support for profiling on nested
 functions.  The current code just doesn't work.

Comment 11 Geoff Keating 2002-07-17 00:07:11 UTC
From: Geoff Keating <geoffk@geoffk.org>
To: amodra@bigpond.net.au
Cc: d.mueller@elsoft.ch, gcc-gnats@gcc.gnu.org, gcc-patches@gcc.gnu.org,
   dje@watson.ibm.com
Subject: Re: other/7114: ICE building strcoll.op from glibc-2.2.5
Date: Wed, 17 Jul 2002 00:07:11 -0700

 > Date: Wed, 17 Jul 2002 11:17:39 +0930
 > From: Alan Modra <amodra@bigpond.net.au>
 
 > On Tue, Jul 16, 2002 at 10:48:14AM -0700, Geoff Keating wrote:
 > > The profiling function isn't allowed to clobber r30.  It never has
 > > been, so this should be no surprise.
 > 
 > I'm not quite sure what to make of this response.  We're talking about
 > this code from rs6000.c:10479
 > 
 >       if (current_function_needs_context)
 > 	asm_fprintf (file, "\tmr %s,%s\n",
 > 		     reg_names[30], reg_names[STATIC_CHAIN_REGNUM]);
 >       fprintf (file, "\tbl %s\n", RS6000_MCOUNT);
 >       if (current_function_needs_context)
 > 	asm_fprintf (file, "\tmr %s,%s\n",
 > 		     reg_names[STATIC_CHAIN_REGNUM], reg_names[30]);
 
 I see, I was confused.  I thought we were already using r30 for
 STATIC_CHAIN_REGNUM.  The ABI specifies r31, but I see we can't use
 that if we want trampolines to be efficient.
 
 > This is currently emitted _before_ the prologue in the nested function,
 > thus trashes r30.  I was considering the idea of adding a clobber of
 > r30 to CALL_INSN_FUNCTION_USAGE when calling a nested function.  That's
 > a workable solution, but means you need to zap r30 on all calls via
 > function pointers too.
 
 From David's mail message when he put the code in:
 
      I have ripped out all of the stack PUSH/POP stuff that was
      causing ABI problems and replaced it with explicit moves to a
      temporary register.  This includes having the SVR4 ABI act more
      like AIX using a register instead of the dangerous stack
      save/restore game.  I could not test the SVR4 changes, so I would
      appreciate if the LinuxPPC testers would make sure that I have
      not broken anything when profiling is enabled.
 
 So, thanks for testing it!  We now know this doesn't work. :-)
 
 So, why don't we go back to the push/pop implementation, but this time
 do it properly?  We'd only need to push/pop in the (rare)
 nested-function case.
 
 -- 
 - Geoffrey Keating <geoffk@geoffk.org> <geoffk@redhat.com>

Comment 12 Alan Modra 2002-07-17 11:17:39 UTC
From: Alan Modra <amodra@bigpond.net.au>
To: Geoff Keating <geoffk@redhat.com>
Cc: d.mueller@elsoft.ch, gcc-gnats@gcc.gnu.org, gcc-patches@gcc.gnu.org,
  dje@watson.ibm.com
Subject: Re: other/7114: ICE building strcoll.op from glibc-2.2.5
Date: Wed, 17 Jul 2002 11:17:39 +0930

 On Tue, Jul 16, 2002 at 10:48:14AM -0700, Geoff Keating wrote:
 > The profiling function isn't allowed to clobber r30.  It never has
 > been, so this should be no surprise.
 
 I'm not quite sure what to make of this response.  We're talking about
 this code from rs6000.c:10479
 
       if (current_function_needs_context)
 	asm_fprintf (file, "\tmr %s,%s\n",
 		     reg_names[30], reg_names[STATIC_CHAIN_REGNUM]);
       fprintf (file, "\tbl %s\n", RS6000_MCOUNT);
       if (current_function_needs_context)
 	asm_fprintf (file, "\tmr %s,%s\n",
 		     reg_names[STATIC_CHAIN_REGNUM], reg_names[30]);
 
 This is currently emitted _before_ the prologue in the nested function,
 thus trashes r30.  I was considering the idea of adding a clobber of
 r30 to CALL_INSN_FUNCTION_USAGE when calling a nested function.  That's
 a workable solution, but means you need to zap r30 on all calls via
 function pointers too.
 
 -- 
 Alan Modra
 IBM OzLabs - Linux Technology Centre

Comment 13 David Edelsohn 2002-07-17 11:42:02 UTC
From: David Edelsohn <dje@watson.ibm.com>
To: Geoff Keating <geoffk@redhat.com>
Cc: amodra@bigpond.net.au, d.mueller@elsoft.ch, gcc-gnats@gcc.gnu.org,
   gcc-patches@gcc.gnu.org
Subject: Re: other/7114: ICE building strcoll.op from glibc-2.2.5 
Date: Wed, 17 Jul 2002 11:42:02 -0400

 >>>>> Geoff Keating writes:
 
 Geoff> So, thanks for testing it!  We now know this doesn't work. :-)
 
 	The patch was tested on SVR4 by Franz and showed no new failures:
 
 http://gcc.gnu.org/ml/gcc-patches/1999-03n/msg00584.html
 
 PUSH/POP cannot work on PowerPC.  On AIX PUSH/POP were corrupting the
 stack.
 
 David

Comment 14 Geoff Keating 2002-07-17 12:03:14 UTC
From: Geoff Keating <geoffk@geoffk.org>
To: amodra@bigpond.net.au
Cc: d.mueller@elsoft.ch, gcc-gnats@gcc.gnu.org, gcc-patches@gcc.gnu.org,
   dje@watson.ibm.com
Subject: Re: other/7114: ICE building strcoll.op from glibc-2.2.5
Date: Wed, 17 Jul 2002 12:03:14 -0700

 > Date: Wed, 17 Jul 2002 18:30:49 +0930
 > From: Alan Modra <amodra@bigpond.net.au>
 > Cc: d.mueller@elsoft.ch, gcc-gnats@gcc.gnu.org, gcc-patches@gcc.gnu.org,
 >    dje@watson.ibm.com
 
 > On Wed, Jul 17, 2002 at 12:07:11AM -0700, Geoff Keating wrote:
 > > So, why don't we go back to the push/pop implementation, but this time
 > > do it properly?  We'd only need to push/pop in the (rare)
 > > nested-function case.
 > 
 > I wasn't aware that powerpc used that scheme previously, and therefore
 > was worried that some mcount implementation might peek at the stack.
 > 
 > Here we go.
 > 
 > 	* config/rs6000/r6000.c (first_reg_to_save): Remove bogus
 > 	adjustments to first_reg for profiling case.
 > 	(output_function_profiler): Correct lr save slot for ABI_AIX_NODESC.
 > 	Disable profiling for 64 bit code on both ABI_V4 and ABI_AIX_NODESC.
 > 	Save static chain reg to sp + 12 on ABI_AIX_NODESC.
 > 	* config/rs6000/sysv4.h (ASM_OUTPUT_REG_PUSH): Define.
 > 	(ASM_OUTPUT_REG_POP): Define.
 > 	* config/rs6000/linux64.h (ASM_OUTPUT_REG_PUSH): Undef.
 > 	(ASM_OUTPUT_REG_POP): Undef.
 
 This is OK.  I'm pretty sure it's right, and completely sure it's no
 worse than before. :-)
 
 
 -- 
 - Geoffrey Keating <geoffk@geoffk.org> <geoffk@redhat.com>

Comment 15 Geoff Keating 2002-07-17 12:10:12 UTC
From: Geoff Keating <geoffk@geoffk.org>
To: dje@watson.ibm.com
Cc: amodra@bigpond.net.au, d.mueller@elsoft.ch, gcc-gnats@gcc.gnu.org,
   gcc-patches@gcc.gnu.org
Subject: Re: other/7114: ICE building strcoll.op from glibc-2.2.5
Date: Wed, 17 Jul 2002 12:10:12 -0700

 > cc: amodra@bigpond.net.au, d.mueller@elsoft.ch, gcc-gnats@gcc.gnu.org,
 >    gcc-patches@gcc.gnu.org
 > Date: Wed, 17 Jul 2002 11:42:02 -0400
 > From: David Edelsohn <dje@watson.ibm.com>
 > 
 > >>>>> Geoff Keating writes:
 > 
 > Geoff> So, thanks for testing it!  We now know this doesn't work. :-)
 > 
 > 	The patch was tested on SVR4 by Franz and showed no new failures:
 > 
 > http://gcc.gnu.org/ml/gcc-patches/1999-03n/msg00584.html
 
 The testsuite probably doesn't check that r30 is not clobbered in a
 nested function when profiling is switched on.  Actually, Alan, could
 you write a test for that?  (That's something I missed while reviewing
 your patch.)
 
 > PUSH/POP cannot work on PowerPC.  On AIX PUSH/POP were corrupting the
 > stack.
 
 Because they were implemented wrongly?
 
 Clearly, push/pop can work, because procedures push and pop call
 frames all the time.  It's just necessary to do it the right way. 
 
 For AIX, of course, push/pop is unnecessary, and Alan's patch didn't
 add it.
 
 -- 
 - Geoffrey Keating <geoffk@geoffk.org> <geoffk@redhat.com>

Comment 16 David Edelsohn 2002-07-17 13:38:23 UTC
From: David Edelsohn <dje@watson.ibm.com>
To: Alan Modra <amodra@bigpond.net.au>
Cc: Geoff Keating <geoffk@redhat.com>, d.mueller@elsoft.ch,
   gcc-gnats@gcc.gnu.org, gcc-patches@gcc.gnu.org
Subject: Re: other/7114: ICE building strcoll.op from glibc-2.2.5 
Date: Wed, 17 Jul 2002 13:38:23 -0400

 	If you want to define PUSH/POP for sysv4.h, that's fine.  I would
 recommend removing the !TARGET_32BIT case for those macros and only using
 the PowerPC mnemonics, to simplify things.
 
 >        * config/rs6000/r6000.c (first_reg_to_save): Remove bogus
 >        adjustments to first_reg for profiling case.
 > first_reg_to_save doesn't need to do anything special for any of these
 > registers as profiling is done via PROFILE_HOOK when ABI_AIX or
 > ABI_DARWIN.  The normal register allocation code will set up
 > regs_ever_live for us.  We're also not trying to use a reg when ABI_V4.
 
 	I guess this works now because of the scheduled prologue.
 
 	I think this is okay, once the trunk can bootstrap again and the
 patch can be tested there.  Joern has a patch which fixes the regrename.c
 bug he introduced
 
 http://gcc.gnu.org/ml/gcc-patches/2002-07/msg00840.html
 
 but no one with global write privileges has approved it.
 
 David

Comment 17 Alan Modra 2002-07-17 13:42:00 UTC
From: Alan Modra <amodra@bigpond.net.au>
To: Geoff Keating <geoffk@redhat.com>, d.mueller@elsoft.ch,
  gcc-gnats@gcc.gnu.org, gcc-patches@gcc.gnu.org, dje@watson.ibm.com
Cc:  
Subject: Re: other/7114: ICE building strcoll.op from glibc-2.2.5
Date: Wed, 17 Jul 2002 13:42:00 +0930

 	PR target/5967, PR other/7114
 	* config/rs6000/r6000.c (first_reg_to_save): Remove bogus
 	adjustments to first_reg for profiling case.
 	(output_function_profiler): Correct lr save slot for ABI_AIX_NODESC.
 	Disable profiling for nested functions on ABI_V4, and for 64 bit
 	code on both ABI_V4 and ABI_AIX_NODESC.  Save static chain reg
 	to sp + 12 on ABI_AIX_NODESC.
 
 Rationale:
 
 	* config/rs6000/r6000.c (first_reg_to_save): Remove bogus
 	adjustments to first_reg for profiling case.
 first_reg_to_save doesn't need to do anything special for any of these
 registers as profiling is done via PROFILE_HOOK when ABI_AIX or
 ABI_DARWIN.  The normal register allocation code will set up
 regs_ever_live for us.  We're killing profiling on nested functions
 when ABI_V4.
 
 	(output_function_profiler): Correct lr save slot for ABI_AIX_NODESC.
 ABI_AIX_NODESC saves lr to sp + 8.  This change is perhaps a little
 contentious as existing mcount implementations may take into account
 the current ABI breakage.
 
 	Disable profiling for nested functions on ABI_V4
 See the comment below.
 
 	and for 64 bit code on both ABI_V4 and ABI_AIX_NODESC.
 The instructions emitted here are 32 bit ones.
 
 	Save static chain reg to sp + 12 on ABI_AIX_NODESC.
 We need to save it somewhere, or disable profiling of nested functions
 for ABI_AIX_NODESC too.
 
 bootstrapped (a slightly different patch) and regression tested
 powerpc-linux.  I'm re-running the bootstrap now.  Built powerpc-linux
 and powerpc64-linux glibc --enable-profile to test ABI_V4 and ABI_AIX
 changes.
 
 -- 
 Alan Modra
 IBM OzLabs - Linux Technology Centre
 
 Index: rs6000.c
 ===================================================================
 RCS file: /cvs/gcc/gcc/gcc/config/rs6000/rs6000.c,v
 retrieving revision 1.343
 diff -u -p -r1.343 rs6000.c
 --- rs6000.c	16 Jul 2002 02:16:41 -0000	1.343
 +++ rs6000.c	17 Jul 2002 03:43:03 -0000
 @@ -7357,53 +7357,6 @@ first_reg_to_save ()
  		    || (DEFAULT_ABI == ABI_DARWIN && flag_pic)))))
        break;
  
 -  if (current_function_profile)
 -    {
 -      /* AIX must save/restore every register that contains a parameter
 -	 before/after the .__mcount call plus an additional register
 -	 for the static chain, if needed; use registers from 30 down to 22
 -	 to do this.  */
 -      if (DEFAULT_ABI == ABI_AIX || DEFAULT_ABI == ABI_DARWIN)
 -	{
 -	  int last_parm_reg, profile_first_reg;
 -
 -	  /* Figure out last used parameter register.  The proper thing
 -	     to do is to walk incoming args of the function.  A function
 -	     might have live parameter registers even if it has no
 -	     incoming args.  */
 -	  for (last_parm_reg = 10;
 -	       last_parm_reg > 2 && ! regs_ever_live [last_parm_reg];
 -	       last_parm_reg--)
 -	    ;
 -
 -	  /* Calculate first reg for saving parameter registers
 -	     and static chain.
 -	     Skip reg 31 which may contain the frame pointer.  */
 -	  profile_first_reg = (33 - last_parm_reg
 -			       - (current_function_needs_context ? 1 : 0));
 -#if TARGET_MACHO
 -          /* Need to skip another reg to account for R31 being PICBASE
 -             (when flag_pic is set) or R30 being used as the frame
 -             pointer (when flag_pic is not set).  */
 -          --profile_first_reg;
 -#endif
 -	  /* Do not save frame pointer if no parameters needs to be saved.  */
 -	  if (profile_first_reg == 31)
 -	    profile_first_reg = 32;
 -
 -	  if (first_reg > profile_first_reg)
 -	    first_reg = profile_first_reg;
 -	}
 -
 -      /* SVR4 may need one register to preserve the static chain.  */
 -      else if (current_function_needs_context)
 -	{
 -	  /* Skip reg 31 which may contain the frame pointer.  */
 -	  if (first_reg > 30)
 -	    first_reg = 30;
 -	}
 -    }
 -
  #if TARGET_MACHO
    if (flag_pic && current_function_uses_pic_offset_table &&
        (first_reg > RS6000_PIC_OFFSET_TABLE_REGNUM))
 @@ -10430,6 +10383,8 @@ output_function_profiler (file, labelno)
    int labelno;
  {
    char buf[100];
 +  int save_lr = 8;
 +  int save_chain = 12;
  
    ASM_GENERATE_INTERNAL_LABEL (buf, "LP", labelno);
    switch (DEFAULT_ABI)
 @@ -10438,13 +10393,35 @@ output_function_profiler (file, labelno)
        abort ();
  
      case ABI_V4:
 +      if (current_function_needs_context)
 +	{
 +	  /* There's no safe way to save STATIC_CHAIN_REGNUM around
 +	     the mcount call.  The ABI_V4 stack has no slot available,
 +	     and since we are PROFILE_BEFORE_PROLOGUE, we can't use a
 +	     call-saved register.  Adjusting the stack to give us some
 +	     space might confuse special purpose mcount functions.
 +	     And finally, clobbering a callee saved register for the
 +	     purpose of saving the static chain when calling a nested
 +	     function is difficult to get right;  You might be calling
 +	     a nested function via a function pointer.  */
 +	  warning ("no profiling on nested functions for this ABI");
 +	  return;
 +	}
 +      save_lr = 4;
 +      /* Fall through.  */
 +
      case ABI_AIX_NODESC:
 +      if (!TARGET_32BIT)
 +	{
 +	  warning ("no profiling of 64-bit code for this ABI");
 +	  return;
 +	}
        fprintf (file, "\tmflr %s\n", reg_names[0]);
        if (flag_pic == 1)
  	{
  	  fputs ("\tbl _GLOBAL_OFFSET_TABLE_@local-4\n", file);
 -	  asm_fprintf (file, "\t{st|stw} %s,4(%s)\n",
 -		       reg_names[0], reg_names[1]);
 +	  asm_fprintf (file, "\t{st|stw} %s,%d(%s)\n",
 +		       reg_names[0], save_lr, reg_names[1]);
  	  asm_fprintf (file, "\tmflr %s\n", reg_names[12]);
  	  asm_fprintf (file, "\t{l|lwz} %s,", reg_names[0]);
  	  assemble_name (file, buf);
 @@ -10452,8 +10429,8 @@ output_function_profiler (file, labelno)
  	}
        else if (flag_pic > 1)
  	{
 -	  asm_fprintf (file, "\t{st|stw} %s,4(%s)\n",
 -		       reg_names[0], reg_names[1]);
 +	  asm_fprintf (file, "\t{st|stw} %s,%d(%s)\n",
 +		       reg_names[0], save_lr, reg_names[1]);
  	  /* Now, we need to get the address of the label.  */
  	  fputs ("\tbl 1f\n\t.long ", file);
  	  assemble_name (file, buf);
 @@ -10469,20 +10446,25 @@ output_function_profiler (file, labelno)
  	  asm_fprintf (file, "\t{liu|lis} %s,", reg_names[12]);
  	  assemble_name (file, buf);
  	  fputs ("@ha\n", file);
 -	  asm_fprintf (file, "\t{st|stw} %s,4(%s)\n",
 -		       reg_names[0], reg_names[1]);
 +	  asm_fprintf (file, "\t{st|stw} %s,%d(%s)\n",
 +		       reg_names[0], save_lr, reg_names[1]);
  	  asm_fprintf (file, "\t{cal|la} %s,", reg_names[0]);
  	  assemble_name (file, buf);
  	  asm_fprintf (file, "@l(%s)\n", reg_names[12]);
  	}
  
        if (current_function_needs_context)
 -	asm_fprintf (file, "\tmr %s,%s\n",
 -		     reg_names[30], reg_names[STATIC_CHAIN_REGNUM]);
 -      fprintf (file, "\tbl %s\n", RS6000_MCOUNT);
 -      if (current_function_needs_context)
 -	asm_fprintf (file, "\tmr %s,%s\n",
 -		     reg_names[STATIC_CHAIN_REGNUM], reg_names[30]);
 +	{
 +	  asm_fprintf (file, "\t{st|stw} %s,%d(%s)\n",
 +		       reg_names[STATIC_CHAIN_REGNUM],
 +		       save_chain, reg_names[1]);
 +	  fprintf (file, "\tbl %s\n", RS6000_MCOUNT);
 +	  asm_fprintf (file, "\t{l|lwz} %s,%d(%s)\n",
 +		       reg_names[STATIC_CHAIN_REGNUM],
 +		       save_chain, reg_names[1]);
 +	}
 +      else
 +	fprintf (file, "\tbl %s\n", RS6000_MCOUNT);
        break;
  
      case ABI_AIX:

Comment 18 David Edelsohn 2002-07-17 16:28:02 UTC
From: David Edelsohn <dje@watson.ibm.com>
To: Geoff Keating <geoffk@redhat.com>
Cc: amodra@bigpond.net.au, d.mueller@elsoft.ch, gcc-gnats@gcc.gnu.org,
   gcc-patches@gcc.gnu.org
Subject: Re: other/7114: ICE building strcoll.op from glibc-2.2.5 
Date: Wed, 17 Jul 2002 16:28:02 -0400

 >>>>> Geoff Keating writes:
 
 >> PUSH/POP cannot work on PowerPC.  On AIX PUSH/POP were corrupting the
 >> stack.
 
 Geoff> Because they were implemented wrongly?
 
 Geoff> Clearly, push/pop can work, because procedures push and pop call
 Geoff> frames all the time.  It's just necessary to do it the right way. 
 
 	We are discussing PUSH/POP in the context of the macros to save
 and restore individual registers used when generating a profiler call in
 final.c, so GCC's ability to correctly allocate call frames is irrelevant.
 
 	The discussion about the PUSH/POP macros in March 1999 provides
 more background about the problem: the macros were not respecting the
 ABI-defined location of dynamic allocation on the stack.  PUSH/POP ideally
 should work like alloca and open a hole in the stack frame at the alloca
 location.  The PUSH/POP macros modify the stack pointer without updating
 the compiler's internal knowledge about the stack layout and assume that
 the alloca area is adjacent to the top of the stack, so adjusting the
 stack pointer will not have any bad consequences, e.g., if a function call
 is invoked.
 
 	On AIX, things went haywire because the PUSH/POP macros only
 copied the backchain pointer, not all of the ABI-specified area between
 the backchain pointer and the alloca area.  When GCC called the profiler
 function, the call frame had random garbage in ABI-specified locations,
 causing the application to run off the rails when an ABI stack location
 was accessed in the called function.  Copying the backchain *and* CR *and*
 LR may be sufficient for the AIX case for this particular use.
 
 	Because SVR4 PowerPC invokes the profiler before the prologue and
 has a slightly different stack layout, the simplistic definition of
 PUSH/POP may work correctly.
 
 David
 

Comment 19 Alan Modra 2002-07-17 18:30:49 UTC
From: Alan Modra <amodra@bigpond.net.au>
To: Geoff Keating <geoffk@redhat.com>
Cc: d.mueller@elsoft.ch, gcc-gnats@gcc.gnu.org, gcc-patches@gcc.gnu.org,
  dje@watson.ibm.com
Subject: Re: other/7114: ICE building strcoll.op from glibc-2.2.5
Date: Wed, 17 Jul 2002 18:30:49 +0930

 On Wed, Jul 17, 2002 at 12:07:11AM -0700, Geoff Keating wrote:
 > So, why don't we go back to the push/pop implementation, but this time
 > do it properly?  We'd only need to push/pop in the (rare)
 > nested-function case.
 
 I wasn't aware that powerpc used that scheme previously, and therefore
 was worried that some mcount implementation might peek at the stack.
 
 Here we go.
 
 	* config/rs6000/r6000.c (first_reg_to_save): Remove bogus
 	adjustments to first_reg for profiling case.
 	(output_function_profiler): Correct lr save slot for ABI_AIX_NODESC.
 	Disable profiling for 64 bit code on both ABI_V4 and ABI_AIX_NODESC.
 	Save static chain reg to sp + 12 on ABI_AIX_NODESC.
 	* config/rs6000/sysv4.h (ASM_OUTPUT_REG_PUSH): Define.
 	(ASM_OUTPUT_REG_POP): Define.
 	* config/rs6000/linux64.h (ASM_OUTPUT_REG_PUSH): Undef.
 	(ASM_OUTPUT_REG_POP): Undef.
 
 Rationale:
 
 	* config/rs6000/r6000.c (first_reg_to_save): Remove bogus
 	adjustments to first_reg for profiling case.
 first_reg_to_save doesn't need to do anything special for any of these
 registers as profiling is done via PROFILE_HOOK when ABI_AIX or
 ABI_DARWIN.  The normal register allocation code will set up
 regs_ever_live for us.  We're also not trying to use a reg when ABI_V4.
 
 	(output_function_profiler): Correct lr save slot for ABI_AIX_NODESC.
 ABI_AIX_NODESC saves lr to sp + 8.  This change is perhaps a little
 contentious as existing mcount implementations may take into account
 the current ABI breakage.
 
 	Disable profiling for 64 bit code on both ABI_V4 and ABI_AIX_NODESC.
 The instructions emitted here are 32 bit ones.  Fix this with a later
 patch.
 
 	Save static chain reg to sp + 12 on ABI_AIX_NODESC.
 We need to save it somewhere.  This seems a likely spot.
 
 	* config/rs6000/sysv4.h (ASM_OUTPUT_REG_PUSH): Define.
 	(ASM_OUTPUT_REG_POP): Define.
 Code resurrected from prior to Mon Mar 15 22:45:25 1999 delta, but
 with DEFAULT_ABI == ABI_V4 test added.
 
 
 powerpc-linux bootstrap on mainline seems to be broken at the moment.
 
 internal compiler error: Internal compiler error
  in tree_low_cst, at tree.c:3312
 
 so I'm in the process of bootstrapping this one on the 3.1 branch.
 
 -- 
 Alan Modra
 IBM OzLabs - Linux Technology Centre
 
 Index: gcc/config/rs6000/rs6000.c
 ===================================================================
 RCS file: /cvs/gcc/gcc/gcc/config/rs6000/rs6000.c,v
 retrieving revision 1.344
 diff -u -p -r1.344 rs6000.c
 --- gcc/config/rs6000/rs6000.c	16 Jul 2002 20:59:03 -0000	1.344
 +++ gcc/config/rs6000/rs6000.c	17 Jul 2002 08:19:35 -0000
 @@ -7356,53 +7356,6 @@ first_reg_to_save ()
  		    || (DEFAULT_ABI == ABI_DARWIN && flag_pic)))))
        break;
  
 -  if (current_function_profile)
 -    {
 -      /* AIX must save/restore every register that contains a parameter
 -	 before/after the .__mcount call plus an additional register
 -	 for the static chain, if needed; use registers from 30 down to 22
 -	 to do this.  */
 -      if (DEFAULT_ABI == ABI_AIX || DEFAULT_ABI == ABI_DARWIN)
 -	{
 -	  int last_parm_reg, profile_first_reg;
 -
 -	  /* Figure out last used parameter register.  The proper thing
 -	     to do is to walk incoming args of the function.  A function
 -	     might have live parameter registers even if it has no
 -	     incoming args.  */
 -	  for (last_parm_reg = 10;
 -	       last_parm_reg > 2 && ! regs_ever_live [last_parm_reg];
 -	       last_parm_reg--)
 -	    ;
 -
 -	  /* Calculate first reg for saving parameter registers
 -	     and static chain.
 -	     Skip reg 31 which may contain the frame pointer.  */
 -	  profile_first_reg = (33 - last_parm_reg
 -			       - (current_function_needs_context ? 1 : 0));
 -#if TARGET_MACHO
 -          /* Need to skip another reg to account for R31 being PICBASE
 -             (when flag_pic is set) or R30 being used as the frame
 -             pointer (when flag_pic is not set).  */
 -          --profile_first_reg;
 -#endif
 -	  /* Do not save frame pointer if no parameters needs to be saved.  */
 -	  if (profile_first_reg == 31)
 -	    profile_first_reg = 32;
 -
 -	  if (first_reg > profile_first_reg)
 -	    first_reg = profile_first_reg;
 -	}
 -
 -      /* SVR4 may need one register to preserve the static chain.  */
 -      else if (current_function_needs_context)
 -	{
 -	  /* Skip reg 31 which may contain the frame pointer.  */
 -	  if (first_reg > 30)
 -	    first_reg = 30;
 -	}
 -    }
 -
  #if TARGET_MACHO
    if (flag_pic && current_function_uses_pic_offset_table &&
        (first_reg > RS6000_PIC_OFFSET_TABLE_REGNUM))
 @@ -10429,6 +10382,7 @@ output_function_profiler (file, labelno)
    int labelno;
  {
    char buf[100];
 +  int save_lr = 8;
  
    ASM_GENERATE_INTERNAL_LABEL (buf, "LP", labelno);
    switch (DEFAULT_ABI)
 @@ -10437,13 +10391,21 @@ output_function_profiler (file, labelno)
        abort ();
  
      case ABI_V4:
 +      save_lr = 4;
 +      /* Fall through.  */
 +
      case ABI_AIX_NODESC:
 +      if (!TARGET_32BIT)
 +	{
 +	  warning ("no profiling of 64-bit code for this ABI");
 +	  return;
 +	}
        fprintf (file, "\tmflr %s\n", reg_names[0]);
        if (flag_pic == 1)
  	{
  	  fputs ("\tbl _GLOBAL_OFFSET_TABLE_@local-4\n", file);
 -	  asm_fprintf (file, "\t{st|stw} %s,4(%s)\n",
 -		       reg_names[0], reg_names[1]);
 +	  asm_fprintf (file, "\t{st|stw} %s,%d(%s)\n",
 +		       reg_names[0], save_lr, reg_names[1]);
  	  asm_fprintf (file, "\tmflr %s\n", reg_names[12]);
  	  asm_fprintf (file, "\t{l|lwz} %s,", reg_names[0]);
  	  assemble_name (file, buf);
 @@ -10451,8 +10413,8 @@ output_function_profiler (file, labelno)
  	}
        else if (flag_pic > 1)
  	{
 -	  asm_fprintf (file, "\t{st|stw} %s,4(%s)\n",
 -		       reg_names[0], reg_names[1]);
 +	  asm_fprintf (file, "\t{st|stw} %s,%d(%s)\n",
 +		       reg_names[0], save_lr, reg_names[1]);
  	  /* Now, we need to get the address of the label.  */
  	  fputs ("\tbl 1f\n\t.long ", file);
  	  assemble_name (file, buf);
 @@ -10468,27 +10430,32 @@ output_function_profiler (file, labelno)
  	  asm_fprintf (file, "\t{liu|lis} %s,", reg_names[12]);
  	  assemble_name (file, buf);
  	  fputs ("@ha\n", file);
 -	  asm_fprintf (file, "\t{st|stw} %s,4(%s)\n",
 -		       reg_names[0], reg_names[1]);
 +	  asm_fprintf (file, "\t{st|stw} %s,%d(%s)\n",
 +		       reg_names[0], save_lr, reg_names[1]);
  	  asm_fprintf (file, "\t{cal|la} %s,", reg_names[0]);
  	  assemble_name (file, buf);
  	  asm_fprintf (file, "@l(%s)\n", reg_names[12]);
  	}
  
 -      if (current_function_needs_context)
 -	asm_fprintf (file, "\tmr %s,%s\n",
 -		     reg_names[30], reg_names[STATIC_CHAIN_REGNUM]);
 -      fprintf (file, "\tbl %s\n", RS6000_MCOUNT);
 -      if (current_function_needs_context)
 -	asm_fprintf (file, "\tmr %s,%s\n",
 -		     reg_names[STATIC_CHAIN_REGNUM], reg_names[30]);
 +      if (current_function_needs_context && DEFAULT_ABI == ABI_AIX_NODESC)
 +	{
 +	  asm_fprintf (file, "\t{st|stw} %s,%d(%s)\n",
 +		       reg_names[STATIC_CHAIN_REGNUM],
 +		       12, reg_names[1]);
 +	  fprintf (file, "\tbl %s\n", RS6000_MCOUNT);
 +	  asm_fprintf (file, "\t{l|lwz} %s,%d(%s)\n",
 +		       reg_names[STATIC_CHAIN_REGNUM],
 +		       12, reg_names[1]);
 +	}
 +      else
 +	/* ABI_V4 saves the static chain reg with ASM_OUTPUT_REG_PUSH.  */
 +	fprintf (file, "\tbl %s\n", RS6000_MCOUNT);
        break;
  
      case ABI_AIX:
      case ABI_DARWIN:
        /* Don't do anything, done in output_profile_hook ().  */
        break;
 -
      }
  }
  
 Index: gcc/config/rs6000/sysv4.h
 ===================================================================
 RCS file: /cvs/gcc/gcc/gcc/config/rs6000/sysv4.h,v
 retrieving revision 1.98
 diff -u -p -r1.98 sysv4.h
 --- gcc/config/rs6000/sysv4.h	10 Jul 2002 00:33:51 -0000	1.98
 +++ gcc/config/rs6000/sysv4.h	17 Jul 2002 08:19:36 -0000
 @@ -736,6 +736,38 @@ do {									\
    ASM_OUTPUT_ALIGNED_LOCAL (FILE, NAME, SIZE, ALIGN);			\
  } while (0)
  
 +/* This is how to output code to push a register on the stack.
 +   It need not be very fast code.
 +
 +   On the rs6000, we must keep the backchain up to date.  In order
 +   to simplify things, always allocate 16 bytes for a push (System V
 +   wants to keep stack aligned to a 16 byte boundary).  */
 +
 +#define	ASM_OUTPUT_REG_PUSH(FILE, REGNO)				\
 +do {									\
 +  if (DEFAULT_ABI == ABI_V4)						\
 +    asm_fprintf (FILE,							\
 +		 (TARGET_32BIT						\
 +		  ? "\t{stu|stwu} %s,-16(%s)\n\t{st|stw} %s,12(%s)\n"	\
 +		  : "\tstdu %s,-32(%s)\n\tstd %s,24(%s)\n"),		\
 +		 reg_names[1], reg_names[1], reg_names[REGNO],		\
 +		 reg_names[1]);						\
 +} while (0)
 +
 +/* This is how to output an insn to pop a register from the stack.
 +   It need not be very fast code.  */
 +
 +#define	ASM_OUTPUT_REG_POP(FILE, REGNO)					\
 +do {									\
 +  if (DEFAULT_ABI == ABI_V4)						\
 +    asm_fprintf (FILE,							\
 +		 (TARGET_32BIT						\
 +		  ? "\t{l|lwz} %s,12(%s)\n\t{ai|addic} %s,%s,16\n"	\
 +		  : "\tld %s,24(%s)\n\t{ai|addic} %s,%s,32\n"),		\
 +		 reg_names[REGNO], reg_names[1], reg_names[1],		\
 +		 reg_names[1]);						\
 +} while (0)
 +
  /* Switch  Recognition by gcc.c.  Add -G xx support.  */
  
  /* Override svr4.h definition.  */
 Index: gcc/config/rs6000/linux64.h
 ===================================================================
 RCS file: /cvs/gcc/gcc/gcc/config/rs6000/linux64.h,v
 retrieving revision 1.21
 diff -u -p -r1.21 linux64.h
 --- gcc/config/rs6000/linux64.h	11 Jul 2002 00:23:16 -0000	1.21
 +++ gcc/config/rs6000/linux64.h	17 Jul 2002 08:19:25 -0000
 @@ -329,3 +329,7 @@ do									\
      sym_lineno += 1;							\
    }									\
  while (0)
 +
 +/* Override sysv4.h as these are ABI_V4 only.  */
 +#undef	ASM_OUTPUT_REG_PUSH
 +#undef	ASM_OUTPUT_REG_POP
 

Comment 20 Alan Modra 2002-09-13 06:53:06 UTC
From: amodra@gcc.gnu.org
To: gcc-gnats@gcc.gnu.org
Cc:  
Subject: other/7114
Date: 13 Sep 2002 06:53:06 -0000

 CVSROOT:	/cvs/gcc
 Module name:	gcc
 Branch: 	gcc-3_2-branch
 Changes by:	amodra@gcc.gnu.org	2002-09-12 23:53:06
 
 Modified files:
 	gcc            : ChangeLog 
 	gcc/config/rs6000: rs6000.c sysv4.h linux64.h 
 
 Log message:
 	2002-07-18  Alan Modra  <amodra@bigpond.net.au>
 	PR other/7114, target/5967
 	* config/rs6000/rs6000.c (first_reg_to_save): Remove bogus
 	adjustments to first_reg for profiling case.
 	(output_function_profiler): Correct lr save slot for ABI_AIX_NODESC.
 	Disable profiling for 64 bit code on both ABI_V4 and ABI_AIX_NODESC.
 	Save static chain reg to sp + 12 on ABI_AIX_NODESC.
 	* config/rs6000/sysv4.h (ASM_OUTPUT_REG_PUSH): Define.
 	(ASM_OUTPUT_REG_POP): Define.
 	* config/rs6000/linux64.h (ASM_OUTPUT_REG_PUSH): Undef.
 	(ASM_OUTPUT_REG_POP): Undef.
 
 Patches:
 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_2-branch&r1=1.13152.2.657.2.42&r2=1.13152.2.657.2.43
 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/rs6000/rs6000.c.diff?cvsroot=gcc&only_with_tag=gcc-3_2-branch&r1=1.291.2.13.2.7&r2=1.291.2.13.2.8
 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/rs6000/sysv4.h.diff?cvsroot=gcc&only_with_tag=gcc-3_2-branch&r1=1.84.2.3.4.2&r2=1.84.2.3.4.3
 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/rs6000/linux64.h.diff?cvsroot=gcc&only_with_tag=gcc-3_2-branch&r1=1.10.2.2.2.3&r2=1.10.2.2.2.4
 
Comment 21 Alan Modra 2002-09-13 16:37:22 UTC
State-Changed-From-To: analyzed->closed
State-Changed-Why: Fixed both mainline and 3.2 branch.