[parisc-linux] pa reload problem

[parisc-linux] pa reload problem

[Alan Modra]

Hi Dave, Jeff,
  This little testcase triggers a bug on hppa64.  It's been around for
quite a while, so it's not something due to Dave's recent changes.

*******
extern void foo (int);
extern unsigned int cpu_hz;

int hppa_dp_bug (void)
{
  foo (42);
  return (5 * ((unsigned long)cpu_hz / 1000000UL));
}
*******

-O2 generates code like

        b,l foo,%r2
        copy %r27,%r4
        addil LT'cpu_hz,%r4	! know r27 trashed in call, so use r4
        ldd RT'cpu_hz(%r1),%r1
        ldd -144(%r30),%r2
        ldw 0(%r1),%r19
        addil LT'L$C0000,%r27	! oops, should be using r4
        ldd RT'L$C0000(%r1),%r1

The bad code codes from config/pa/pa.c:legitimize_pic_address

  if (GET_CODE (orig) == SYMBOL_REF)
    {
      if (reg == 0)
	abort ();

      if (flag_pic == 2)
	{
	  emit_move_insn (reg,
			  gen_rtx_PLUS (word_mode, pic_offset_table_rtx,
					gen_rtx_HIGH (word_mode, orig)));

To fix the problem, it seems to me that we should be testing whether
pic_offset_table_rtx is valid at this point, and if not, use the saved reg
instead.  I'm not exactly sure how to go about testing whether r27 is
valid here, and I'm sure one of you can point me in the right direction.

Alan Modra
-- 
Linuxcare.  Support for the Revolution.

Re: pa reload problem

[John David Anglin]

>         b,l foo,%r2
>         copy %r27,%r4
>         addil LT'cpu_hz,%r4	! know r27 trashed in call, so use r4
>         ldd RT'cpu_hz(%r1),%r1
>         ldd -144(%r30),%r2
>         ldw 0(%r1),%r19
>         addil LT'L$C0000,%r27	! oops, should be using r4
>         ldd RT'L$C0000(%r1),%r1

It looks to me like this happens because the register life analysis in
pass 13 decides that r27 (dp) is dead.  This causes combine to delete the
insn that restores r27 after the call to foo and use the value saved
in r4 as a proxy for dp.

(insn 16 14 21 (set (reg:DI 27 %r27)
        (reg:DI 4 %r4)) 123 {movstrsi_internal+11} (nil)
    (expr_list:REG_DEAD (reg:DI 4 %r4)
	(nil)))

(insn 21 16 23 (set (reg:DI 68)
	(plus:DI (reg:DI 27 %r27)
	    (high:DI (symbol_ref:DI ("cpu_hz"))))) 81 {post_std+3} (insn_list 16 (nil))
     (expr_list:REG_DEAD (reg:DI 27 %r27)
	(nil)))

The const_int's which follow haven't been expanded at this point.  Thus,
the life analysis can't tell that r27 is still needed.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)

Re: pa reload problem

[Alan Modra]

On Fri, 1 Dec 2000, John David Anglin wrote:

> >         b,l foo,%r2
> >         copy %r27,%r4
> >         addil LT'cpu_hz,%r4	! know r27 trashed in call, so use r4
> >         ldd RT'cpu_hz(%r1),%r1
> >         ldd -144(%r30),%r2
> >         ldw 0(%r1),%r19
> >         addil LT'L$C0000,%r27	! oops, should be using r4
> >         ldd RT'L$C0000(%r1),%r1
> 
> It looks to me like this happens because the register life analysis in
> pass 13 decides that r27 (dp) is dead.  This causes combine to delete the
> insn that restores r27 after the call to foo and use the value saved
> in r4 as a proxy for dp.

Yes.

> (insn 16 14 21 (set (reg:DI 27 %r27)
>         (reg:DI 4 %r4)) 123 {movstrsi_internal+11} (nil)
>     (expr_list:REG_DEAD (reg:DI 4 %r4)
> 	(nil)))
> 
> (insn 21 16 23 (set (reg:DI 68)
> 	(plus:DI (reg:DI 27 %r27)
> 	    (high:DI (symbol_ref:DI ("cpu_hz"))))) 81 {post_std+3} (insn_list 16 (nil))
>      (expr_list:REG_DEAD (reg:DI 27 %r27)
> 	(nil)))
> 
> The const_int's which follow haven't been expanded at this point.  Thus,
> the life analysis can't tell that r27 is still needed.

Exactly.  So how is the code in pa.c to know that PIC_OFFSET_TABLE_REGNUM 
has been clobbered - and should use PIC_OFFSET_TABLE_REGNUM_SAVED instead?
Using the saved version should be safe as it is a fixed reg.

Or is a better solution to simply ensure that r27 never dies?  I'm
thinking of ABI and (non-gnu) linker considerations.  eg. DLTIND21L relocs
not involving r27 may cause something to barf.

Alan
-- 
Linuxcare.  Support for the Revolution.

Re: pa reload problem

[Alan Modra]

On Fri, 1 Dec 2000, Richard Hirst wrote:

> Hi Alan,
>   I think I've found a case where it calls an external function with
> dp corrupt, sym53c8xx.c again, in ncr_setsync().  Don't know if this
> is relevant given what you now know about the bug.
> 
> .L2977:
>         ldh 66(%r3),%r28
>         extrw,u %r28,31,16,%r19
>         extrw,u %r19,30,31,%r26
>         copy %r19,%r25
>         add,l %r21,%r26,%r26
>         .IMPORT $$divU,MILLICODE
>         b,l $$divU,%r2			<< stub will corrupt dp
>         ldi 2000,%r20
>         cmpb,<< %r20,%r19,.L2978
>         extrd,u %r29,63,32,%r5
>         ldh 1688(%r6),%r20
>         ldi 32,%r19
>         cmpb,= %r19,%r20,.L2978
>         ldi 33,%r19
>         cmpb,=,n %r19,%r20,.L2978
>         ldd 1632(%r6),%r26
>         ldo -16(%r30),%r29
>         b,l _gsc_readb,%r2		<< enters stub with wrong dp
>         ldo 78(%r26),%r26
>         copy %r4,%r27
>         ldd 1632(%r6),%r25
> 
> I hope I'm not supposed to do anything special with that $$divU call,

Unfortunately, I think you are supposed to.  All the $$ routines are from
the millicode library, and are treated specially by the linker.  When
doing a final link, these functions are never supplied by a shared
library, but must come from a static lib like libgcc.a, or
libmilli.a.  This has the effect of never calling these routines via the
.plt, and so dp is preserved.  gcc isn't doing anything wrong here.

If you use a long branch stub that only trashes r1, you should be OK.
_Don't load dp for these routines!  (I hope all libmilli routines are
re-entrant - if not, we'd probably have weird kernel crashes...)

Note that all of the above applies to 32-bit modutils too.

cc:d to the mailing list in case any of the above needs correcting.

Alan
-- 
Linuxcare.  Support for the Revolution.

Re: pa reload problem

[John David Anglin]

> Or is a better solution to simply ensure that r27 never dies?  I'm
> thinking of ABI and (non-gnu) linker considerations.  eg. DLTIND21L relocs
> not involving r27 may cause something to barf.

Here is a patch that I think resolves the problem.  The principal fix is
not to define PIC_OFFSET_TABLE_REG_CALL_CLOBBERED.  The reason for not
defining this is the call expander explicitly restores the pic offset register
after each call.  Thus, I think we can lie about the register being
clobbered.

When PIC_OFFSET_TABLE_REG_CALL_CLOBBERED is not defined and 
PIC_OFFSET_TABLE_REGNUM is a fixed register, flow.c implicitly puts a
use for the register at the end of each function.  This prevents combine
from "eliminating" r27.

In testing this, I tried to build glibc.  However, there seems to be
a problem with `L' character constants.  I keep getting the error
"character constant too long".  As another test, I am running a PIC
bootstrap and check in 32 bit mode under hpux 10.20.

Give it a whirl and see what you think.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)

2000-12-02  John David Anglin  <dave@hiauly1.hia.nrc.ca>

	* pa.h (PIC_OFFSET_TABLE_REG_CALL_CLOBBERED): Don't define.
	* pa32-regs.h (CONDITIONAL_REGISTER_USAGE): When generating pic code,
	PIC_OFFSET_TABLE_REGNUM_SAVED is a call_used register.
	* pa64-regs.h (CONDITIONAL_REGISTER_USAGE): Likewise.

--- pa.h.orig	Wed Sep 27 14:28:34 2000
+++ pa.h	Fri Dec  1 14:37:02 2000
@@ -492,10 +492,11 @@
 #define STATIC_CHAIN_REGNUM 29
 
 /* Register which holds offset table for position-independent
-   data references.  */
+   data references.  We save and restore PIC_OFFSET_TABLE_REGNUM
+   across calls so PIC_OFFSET_TABLE_REG_CALL_CLOBBERED shouldn't
+   be defined.  */
 
 #define PIC_OFFSET_TABLE_REGNUM (TARGET_64BIT ? 27 : 19)
-#define PIC_OFFSET_TABLE_REG_CALL_CLOBBERED 1
 
 /* Register into which we save the PIC_OFFEST_TABLE_REGNUM so that it
    can be restore across function calls.  */
--- pa32-regs.h.orig	Fri Jul  7 19:59:16 2000
+++ pa32-regs.h	Fri Dec  1 16:10:07 2000
@@ -113,6 +113,7 @@
     {						\
       fixed_regs[PIC_OFFSET_TABLE_REGNUM] = 1;	\
       fixed_regs[PIC_OFFSET_TABLE_REGNUM_SAVED] = 1;\
+      call_used_regs[PIC_OFFSET_TABLE_REGNUM_SAVED] = 1;\
     }						\
 }
 
--- pa64-regs.h.orig	Fri Jul  7 19:59:16 2000
+++ pa64-regs.h	Fri Dec  1 16:11:09 2000
@@ -112,6 +112,7 @@
     {						\
       fixed_regs[PIC_OFFSET_TABLE_REGNUM] = 1;	\
       fixed_regs[PIC_OFFSET_TABLE_REGNUM_SAVED] = 1;\
+      call_used_regs[PIC_OFFSET_TABLE_REGNUM_SAVED] = 1;\
     }						\
 }
 

Re: pa reload problem

[John David Anglin]

> In testing this, I tried to build glibc.  However, there seems to be
> a problem with `L' character constants.  I keep getting the error
> "character constant too long".  As another test, I am running a PIC
> bootstrap and check in 32 bit mode under hpux 10.20.

The build failed but its too late to figure out what's wrong.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)

Re: pa reload problem

[Alan Modra]

For those reading this in gcc-patches, this thread starts with a testcase
distilled from the linux kernel at
http://puffin.external.hp.com/mailing-lists/parisc-linux/2000/11-Nov/0388.html
with Dave's analysis at
http://puffin.external.hp.com/mailing-lists/parisc-linux/2000/11-Nov/0391.html

On Sat, 2 Dec 2000, John David Anglin wrote:

> > Or is a better solution to simply ensure that r27 never dies?  I'm
> > thinking of ABI and (non-gnu) linker considerations.  eg. DLTIND21L relocs
> > not involving r27 may cause something to barf.
> 
> Here is a patch that I think resolves the problem.  The principal fix is
> not to define PIC_OFFSET_TABLE_REG_CALL_CLOBBERED.  The reason for not
> defining this is the call expander explicitly restores the pic offset register
> after each call.  Thus, I think we can lie about the register being
> clobbered.

Telling lies generally leads to being caught out. ;-)

I prefer this patch, which strikes at the heart of the problem, and
doesn't seem to cause any new problems.  (apart from an unnecessary
register transfer at the end of some functions)

	* flow.c (mark_regs_live_at_end): Set PIC_OFFSET_TABLE_REGNUM even
	when PIC_OFFSET_TABLE_REG_CALL_CLOBBERED.

Alan
-- 
Linuxcare.  Support for the Revolution.

--- gcc/flow.c~	Fri Dec  1 15:56:03 2000
+++ gcc/flow.c	Sat Dec  2 20:50:51 2000
@@ -3249,13 +3249,17 @@ mark_regs_live_at_end (set)
     }
 
 #ifdef PIC_OFFSET_TABLE_REGNUM
-#ifndef PIC_OFFSET_TABLE_REG_CALL_CLOBBERED
   /* Many architectures have a GP register even without flag_pic.
      Assume the pic register is not in use, or will be handled by
-     other means, if it is not fixed.  */
+     other means, if it is not fixed.
+     We need to mark PIC_OFFSET_TABLE_REGNUM live at the end of a
+     function even when PIC_OFFSET_TABLE_REG_CALL_CLOBBERED because
+     reload may require it be valid.
+     ie. PIC_OFFSET_TABLE_REG_CALL_CLOBBERED should be viewed as
+     indicating what may happen during a call, not viewed as a
+     license allowing a register to die.  */
   if (fixed_regs[PIC_OFFSET_TABLE_REGNUM])
     SET_REGNO_REG_SET (set, PIC_OFFSET_TABLE_REGNUM);
-#endif
 #endif
 
   /* Mark all global registers, and all registers used by the epilogue

Re: pa reload problem

[Richard Henderson]

On Sat, Dec 02, 2000 at 11:35:19PM +1100, Alan Modra wrote:
> 	* flow.c (mark_regs_live_at_end): Set PIC_OFFSET_TABLE_REGNUM even
> 	when PIC_OFFSET_TABLE_REG_CALL_CLOBBERED.

I can't believe this makes a difference.  We already have

#ifdef PIC_OFFSET_TABLE_REGNUM
              /* Before reload, do not allow sets of the pic register
                 to be deleted.  Reload can insert references to
                 constant pool memory anywhere in the function, making
                 the PIC register live where it wasn't before.  */
              if (regno == PIC_OFFSET_TABLE_REGNUM && fixed_regs[regno]
                  && ! reload_completed)
                return 0;
#endif

in insn_dead_p.  Although one could argue keeping the register
live rather than avoiding deaths is a cleaner solution.


r~

Re: pa reload problem

[John David Anglin]

> On Sat, Dec 02, 2000 at 11:35:19PM +1100, Alan Modra wrote:
> > 	* flow.c (mark_regs_live_at_end): Set PIC_OFFSET_TABLE_REGNUM even
> > 	when PIC_OFFSET_TABLE_REG_CALL_CLOBBERED.
> 
> I can't believe this makes a difference.  We already have
> 
> #ifdef PIC_OFFSET_TABLE_REGNUM
>               /* Before reload, do not allow sets of the pic register
>                  to be deleted.  Reload can insert references to
>                  constant pool memory anywhere in the function, making
>                  the PIC register live where it wasn't before.  */
>               if (regno == PIC_OFFSET_TABLE_REGNUM && fixed_regs[regno]
>                   && ! reload_completed)
>                 return 0;
> #endif

I saw this but for some reason it didn't stop the set after the call from being
deleted in the combine pass.  Maybe this needs to be looked at in more detail
to determine why it doesn't work in this circumstance.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)

Re: pa reload problem

[John David Anglin]

> I prefer this patch, which strikes at the heart of the problem, and
> doesn't seem to cause any new problems.  (apart from an unnecessary
> register transfer at the end of some functions)
> 
> 	* flow.c (mark_regs_live_at_end): Set PIC_OFFSET_TABLE_REGNUM even
> 	when PIC_OFFSET_TABLE_REG_CALL_CLOBBERED.

The comment above is incorrect.  It should read "Set SET_REGNO_REG_SET for
PIC_OFFSET_TABLE_REGNUM if it is a fixed register".

Dave
-- 
J. David Anglin                                  dave.anglin@nrc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)

Re: pa reload problem

[John David Anglin]

> > In testing this, I tried to build glibc.  However, there seems to be
> > a problem with `L' character constants.  I keep getting the error
> > "character constant too long".  As another test, I am running a PIC
> > bootstrap and check in 32 bit mode under hpux 10.20.
> 
> The build failed but its too late to figure out what's wrong.

I reverted the pic patch that I was testing and still get the same build
failure when building with "-g -O3 -fPIC".

stage1/xgcc -Bstage1/ -B/usr/local/hppa1.1-hp-hpux10.20/bin/ -c  -DIN_GCC    -g -O3 -fPIC -W -Wall -Wwrite-strings -Wstrict-prototypes -Wmissing-prototypes -Wtraditional -pedantic -Wno-long-long  -DHAVE_CONFIG_H    -I. -I. -I../../gcc -I../../gcc/. -I../../gcc/config -I../../gcc/../include ../../gcc/c-decl.c -o c-decl.o
../../gcc/c-decl.c: In function `groktypename':
../../gcc/c-decl.c:7212: warning: ICE: would have deleted prologue/epilogue insn
(insn 162 160 163 (set (reg:SI 4 %r4)
        (reg:SI 19 %r19)) -1 (nil)
    (nil))
../../gcc/c-decl.c: In function `groktypename_in_parm_context':
../../gcc/c-decl.c:7212: warning: ICE: would have deleted prologue/epilogue insn

(insn 162 160 163 (set (reg:SI 4 %r4)
        (reg:SI 19 %r19)) -1 (nil)
    (nil))
stage1/xgcc -Bstage1/ -B/usr/local/hppa1.1-hp-hpux10.20/bin/ -c  -DIN_GCC    -g -O3 -fPIC -W -Wall -Wwrite-strings -Wstrict-prototypes -Wmissing-prototypes -Wtraditional -pedantic -Wno-long-long  -DHAVE_CONFIG_H    -I. -I. -I../../gcc -I../../gcc/. -I../../gcc/config -I../../gcc/../include ../../gcc/c-typeck.c -o c-typeck.o
../../gcc/c-typeck.c: In function `really_start_incremental_init':
../../gcc/c-typeck.c:5180: output_operand: invalid expression as operand
../../gcc/c-typeck.c:5180: Internal compiler error in output_operand_lossage, at final.c:3397
confused by earlier errors, bailing out
make[2]: *** [c-typeck.o] Error 1
make[2]: Leaving directory `/xxx/gnu/gcc-2.97/objdir/gcc'
make[1]: *** [stage_c] Error 2
make[1]: Leaving directory `/xxx/gnu/gcc-2.97/objdir/gcc'
make: *** [bootstrap-lean] Error 2

Looks like there is still more work to be done regarding pic code generation
for the hppa.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)

Re: pa reload problem

[Richard Henderson]

On Sat, Dec 02, 2000 at 06:01:27PM -0500, John David Anglin wrote:
> I saw this but for some reason it didn't stop the set after the call
> from being deleted in the combine pass.

Oh, right.

In any case, the patch is applied in the wrong place.  It
needs to happen in calculate_global_regs_live, near

      /* Force the stack pointer to be live -- which might not already be
         the case for blocks within infinite loops.  */
      SET_REGNO_REG_SET (new_live_at_end, STACK_POINTER_REGNUM);



r~

Re: pa reload problem

[Alan Modra]

On Sat, 2 Dec 2000, Richard Henderson wrote:

> On Sat, Dec 02, 2000 at 11:35:19PM +1100, Alan Modra wrote:
> > 	* flow.c (mark_regs_live_at_end): Set PIC_OFFSET_TABLE_REGNUM even
> > 	when PIC_OFFSET_TABLE_REG_CALL_CLOBBERED.
> 
> I can't believe this makes a difference.  We already have

It did, I can assure you.  Thanks for the feedback though.  I'll freely 
admit to not having much of an understanding of gcc internals.  What I've 
learned has mostly been from poking around with gdb when faced with a gcc 
bug, and I rely on you and others pointing out the proper fix from a
higher level perspective.

Alan Modra
-- 
Linuxcare.  Support for the Revolution.

Re: pa reload problem

[Alan Modra]

I've applied this patch to pehc CVS to fix an hppa64 gcc problem that
Richard found when compiling modules for the 64-bit kernel.  It's not
specific to modules, but rather can affect anything requiring stubs to
call functions, eg. 64-bit userland linked to shared libs.

-- 
Linuxcare.  Support for the Revolution.

--- gcc/flow.c~	Thu Sep 28 10:37:45 2000
+++ gcc/flow.c	Mon Dec  4 09:53:44 2000
@@ -3298,7 +3298,17 @@
 	 to any pseudo before reload is a potential reference of the
 	 frame pointer.  */
       if (! reload_completed)
-	SET_REGNO_REG_SET (new_live_at_end, FRAME_POINTER_REGNUM);
+	{
+	  SET_REGNO_REG_SET (new_live_at_end, FRAME_POINTER_REGNUM);
+#ifdef PIC_OFFSET_TABLE_REGNUM
+	  /* And also for the pic register.  References to constants
+	     may require reloading via the pic register.  Assume the
+	     pic register is not in use, or will be handled by other
+	     means, if it is not fixed.  */
+	  if (fixed_regs[PIC_OFFSET_TABLE_REGNUM])
+	    SET_REGNO_REG_SET (new_live_at_end, PIC_OFFSET_TABLE_REGNUM);
+#endif
+	}
 
       /* Regs used in phi nodes are not included in
 	 global_live_at_start, since they are live only along a

Re: pa reload problem

[Richard Hirst]

On Sat, Dec 02, 2000 at 06:15:14PM +1100, Alan Modra wrote:
> > I hope I'm not supposed to do anything special with that $$divU call,
> 
> Unfortunately, I think you are supposed to.  All the $$ routines are from
> the millicode library, and are treated specially by the linker.  When
> doing a final link, these functions are never supplied by a shared
> library, but must come from a static lib like libgcc.a, or
> libmilli.a.  This has the effect of never calling these routines via the
> .plt, and so dp is preserved.  gcc isn't doing anything wrong here.
> 
> If you use a long branch stub that only trashes r1, you should be OK.
> _Don't load dp for these routines!  (I hope all libmilli routines are
> re-entrant - if not, we'd probably have weird kernel crashes...)

OK, done that, thanks.

> Note that all of the above applies to 32-bit modutils too.

Shouldn't be an issue with 32 bit, as kernel and modules all have
the same dp there.

Richard

Re: pa reload problem

[Richard Hirst]

On Mon, Dec 04, 2000 at 12:41:29PM +1100, Alan Modra wrote:
> I've applied this patch to pehc CVS to fix an hppa64 gcc problem that
> Richard found when compiling modules for the 64-bit kernel.  It's not
> specific to modules, but rather can affect anything requiring stubs to
> call functions, eg. 64-bit userland linked to shared libs.

And it works :-)  I can now insmod sym53c8xx.o on the A500 successfully.

Many thanks,
  Richard

Re: pa reload problem

[Alan Modra]

On Mon, 4 Dec 2000, Richard Hirst wrote:

> > Note that all of the above applies to 32-bit modutils too.
> 
> Shouldn't be an issue with 32 bit, as kernel and modules all have
> the same dp there.

Yes, silly me.  32 bit is non-pic, so you don't need to worry about
it.  (%r19 in 32 bit case)

-- 
Linuxcare.  Support for the Revolution.

Re: pa reload problem

[John David Anglin]

> On Mon, 4 Dec 2000, Richard Hirst wrote:
> 
> > > Note that all of the above applies to 32-bit modutils too.
> > 
> > Shouldn't be an issue with 32 bit, as kernel and modules all have
> > the same dp there.
> 
> Yes, silly me.  32 bit is non-pic, so you don't need to worry about
> it.  (%r19 in 32 bit case)

However, it may be an issue for 32 bit pic code under hpux.  It is not
clear to me why this didn't show up before with 32 bit pic code.  For
some reason, the circumstances which caused combine to delete the set
restoring dp after function calls didn't seem to occur.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)

Re: pa reload problem

[Alan Modra]

On Mon, 4 Dec 2000, John David Anglin wrote:

> > On Mon, 4 Dec 2000, Richard Hirst wrote:
> > 
> > > > Note that all of the above applies to 32-bit modutils too.
> > > 
> > > Shouldn't be an issue with 32 bit, as kernel and modules all have
> > > the same dp there.
> > 
> > Yes, silly me.  32 bit is non-pic, so you don't need to worry about
> > it.  (%r19 in 32 bit case)
> 
> However, it may be an issue for 32 bit pic code under hpux.  It is not
> clear to me why this didn't show up before with 32 bit pic code.  For
> some reason, the circumstances which caused combine to delete the set
> restoring dp after function calls didn't seem to occur.

Hi Dave,
  Richard and I had wandered off-topic in the above exchange, and were
talking about modutils and stubs, in particular stub calls to millicode.

Getting back on topic, the reload problem _does_ affect 32 bit hppa code.
Compiling my testcase with -O2 -fPIC -S, gives

        copy %r19,%r4
        bl foo,%r2
        stw %r19,-32(%r30)
        addil LT'cpu_hz,%r4
        copy %r1,%r21
        addil LT'.LC0,%r19		! oops
        ldw RT'cpu_hz(%r21),%r21
        ldw RT'.LC0(%r1),%r1

Alan
-- 
Linuxcare.  Support for the Revolution.

Re: pa reload problem

[John David Anglin]

> Hi Dave,
>   Richard and I had wandered off-topic in the above exchange, and were
> talking about modutils and stubs, in particular stub calls to millicode.
> 
> Getting back on topic, the reload problem _does_ affect 32 bit hppa code.
> Compiling my testcase with -O2 -fPIC -S, gives
> 
>         copy %r19,%r4
>         bl foo,%r2
>         stw %r19,-32(%r30)
>         addil LT'cpu_hz,%r4
>         copy %r1,%r21
>         addil LT'.LC0,%r19		! oops

For some reason, I only saw this with the 64 bit version.

There is one other bit of code in rtlanal.c related to the patch which
bothers me.  I am wondering if it is ok or needs changing:

#ifndef PIC_OFFSET_TABLE_REG_CALL_CLOBBERED
      /* ??? When call-clobbered, the value is stable modulo the restore
         that must happen after a call.  This currently screws up local-alloc
         into believing that the restore is not needed.  */
      if (x == pic_offset_table_rtx)
        return 0;
#endif

This is in rtx_unstable_p and rtx_varies_p.  Possibly, now that there is
a use until reload is complete, this problem is fixed and the
pic_offset_table_rtx should be stable even when
PIC_OFFSET_TABLE_REG_CALL_CLOBBERED?  See comment at beginning of
rtx_varies_p.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)

Re: pa reload problem

[Alan Modra]

On Mon, 4 Dec 2000, John David Anglin wrote:

> > Getting back on topic, the reload problem _does_ affect 32 bit hppa code.
> > Compiling my testcase with -O2 -fPIC -S, gives
> > 
> >         copy %r19,%r4
> >         bl foo,%r2
> >         stw %r19,-32(%r30)
> >         addil LT'cpu_hz,%r4
> >         copy %r1,%r21
> >         addil LT'.LC0,%r19		! oops
> 
> For some reason, I only saw this with the 64 bit version.

Probably because it only appears with -fpic (or -fPIC), which you get for
free on pa64

> There is one other bit of code in rtlanal.c related to the patch which
> bothers me.  I am wondering if it is ok or needs changing:
> 
> #ifndef PIC_OFFSET_TABLE_REG_CALL_CLOBBERED
>       /* ??? When call-clobbered, the value is stable modulo the restore
>          that must happen after a call.  This currently screws up local-alloc
>          into believing that the restore is not needed.  */
>       if (x == pic_offset_table_rtx)
>         return 0;
> #endif
> 
> This is in rtx_unstable_p and rtx_varies_p.  Possibly, now that there is
> a use until reload is complete, this problem is fixed and the
> pic_offset_table_rtx should be stable even when
> PIC_OFFSET_TABLE_REG_CALL_CLOBBERED?  See comment at beginning of
> rtx_varies_p.

Likely so.  With rth's latest patch to calculate_global_regs_live, it's
possibile that I can remove some of the ARG_POINTER_INVARIANT patch too.

-- 
Linuxcare.  Support for the Revolution.

Re: pa reload problem

[John David Anglin]

> I reverted the pic patch that I was testing and still get the same build
> failure when building with "-g -O3 -fPIC".
...
> stage1/xgcc -Bstage1/ -B/usr/local/hppa1.1-hp-hpux10.20/bin/ -c  -DIN_GCC    -g -O3 -fPIC -W -Wall -Wwrite-strings -Wstrict-prototypes -Wmissing-prototypes -Wtraditional -pedantic -Wno-long-long  -DHAVE_CONFIG_H    -I. -I. -I../../gcc -I../../gcc/. -I../../gcc/config -I../../gcc/../include ../../gcc/c-typeck.c -o c-typeck.o
> ../../gcc/c-typeck.c: In function `really_start_incremental_init':
> ../../gcc/c-typeck.c:5180: output_operand: invalid expression as operand
> ../../gcc/c-typeck.c:5180: Internal compiler error in output_operand_lossage, at final.c:3397
> confused by earlier errors, bailing out

This error results from the following substitution which occurs in the
"lreg" pass.  Here is the "lreg" rtl:

(insn 67 926 927 (set (reg/f:SI 107)
        (mem/u:SI (lo_sum:SI (reg/f:SI 309)
                (unspec:SI[
                        (symbol_ref:SI ("constructor_max_index"))
		    ]  0)) 0)) 82 {*pa.md:2352} (insn_list 65 (nil))
    (expr_list:REG_EQUIV (mem/u:SI (lo_sum:SI (reg/f:SI 309)
		(unspec:SI[
			(symbol_ref:SI ("constructor_max_index"))
		    ]  0)) 0)
        (expr_list:REG_DEAD (reg/f:SI 309)
            (nil))))

...

(insn 569 619 627 (set (reg/f:SI 232)
        (reg/f:SI 107)) 69 {*pa.md:2099} (nil)
    (expr_list:REG_DEAD (reg/f:SI 107)
        (nil)))

Then, after the "greg" pass we get for insn 569:

(insn 569 619 627 (set (reg/f:SI 21 %r21)
        (mem/u:SI (lo_sum:SI (reg/f:SI 8 %r8)
		(unspec:SI[
			(symbol_ref:SI ("constructor_max_index"))
		    ]  0)) 0)) 69 {*pa.md:2099} (nil)
    (nil))

The "mem" apparently satisfies the RQ constraint for the "ldw%M1 %1,%0"
code of the insn {*pa.md:2099} but this operand can't be printed printed.
Thus, either we need to improve the printing of pic symbol references or
disallow MEMs of this type in the Q constraint.  Any thoughts on this?

Dave
-- 
J. David Anglin                                  dave.anglin@nrc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)

Re: pa reload problem

[John David Anglin]

> > stage1/xgcc -Bstage1/ -B/usr/local/hppa1.1-hp-hpux10.20/bin/ -c  -DIN_GCC    -g -O3 -fPIC -W -Wall -Wwrite-strings -Wstrict-prototypes -Wmissing-prototypes -Wtraditional -pedantic -Wno-long-long  -DHAVE_CONFIG_H    -I. -I. -I../../gcc -I../../gcc/. -I../../gcc/config -I../../gcc/../include ../../gcc/c-typeck.c -o c-typeck.o
> > ../../gcc/c-typeck.c: In function `really_start_incremental_init':
> > ../../gcc/c-typeck.c:5180: output_operand: invalid expression as operand
> > ../../gcc/c-typeck.c:5180: Internal compiler error in output_operand_lossage, at final.c:3397
> > confused by earlier errors, bailing out
> 
> This error results from the following substitution which occurs in the
> "lreg" pass.  Here is the "lreg" rtl:
> 
> (insn 67 926 927 (set (reg/f:SI 107)
>         (mem/u:SI (lo_sum:SI (reg/f:SI 309)
>                 (unspec:SI[
>                         (symbol_ref:SI ("constructor_max_index"))
> 		    ]  0)) 0)) 82 {*pa.md:2352} (insn_list 65 (nil))
>     (expr_list:REG_EQUIV (mem/u:SI (lo_sum:SI (reg/f:SI 309)
> 		(unspec:SI[
> 			(symbol_ref:SI ("constructor_max_index"))
> 		    ]  0)) 0)
>         (expr_list:REG_DEAD (reg/f:SI 309)
>             (nil))))
> 
> ...
> 
> (insn 569 619 627 (set (reg/f:SI 232)
>         (reg/f:SI 107)) 69 {*pa.md:2099} (nil)
>     (expr_list:REG_DEAD (reg/f:SI 107)
>         (nil)))
> 
> Then, after the "greg" pass we get for insn 569:
> 
> (insn 569 619 627 (set (reg/f:SI 21 %r21)
>         (mem/u:SI (lo_sum:SI (reg/f:SI 8 %r8)
> 		(unspec:SI[
> 			(symbol_ref:SI ("constructor_max_index"))
> 		    ]  0)) 0)) 69 {*pa.md:2099} (nil)
>     (nil))

After further investigation, this appears to be a reload problem.  Reload
substitutes the mem for psuedo 107 because reg_equiv_mem[107] is set
to the above mem rtx and reg_renumber[107] = -1 (ie, there is no hard register
for the psuedo).  The function strict_memory_address_p is used to check
whether the the memory address is valid for the mode.  This uses
GO_IF_LEGITIMATE_ADDRESS.  The address is legit so reg_equiv_mem[107] gets
set.  The problem is reload doesn't appear to check if the insns resulting
from the substitution still match the conditions for the insn.  If it did,
it would find that the substituted mem is not a valid move_operand.

Any thoughts on what is the best approach to fix this?

Dave
-- 
J. David Anglin                                  dave.anglin@nrc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)

Re: pa reload problem

[Richard Henderson]

On Wed, Dec 06, 2000 at 03:41:06PM -0500, John David Anglin wrote:
> >     (expr_list:REG_EQUIV (mem/u:SI (lo_sum:SI (reg/f:SI 309)
> > 		(unspec:SI[
> > 			(symbol_ref:SI ("constructor_max_index"))
> > 		    ]  0)) 0)

This appears to be the same sort of bug as in compile/20001205-1.c
except that an unspec is involved instead of an asm_operands.

I wonder if the best solution is to avoid local-alloc creating
REG_EQUIV notes for every little thing under the sun.  Allow
only general_operand or something.


r~

Re: pa reload problem

[John David Anglin]

> On Wed, Dec 06, 2000 at 03:41:06PM -0500, John David Anglin wrote:
> > >     (expr_list:REG_EQUIV (mem/u:SI (lo_sum:SI (reg/f:SI 309)
> > > 		(unspec:SI[
> > > 			(symbol_ref:SI ("constructor_max_index"))
> > > 		    ]  0)) 0)
> 
> This appears to be the same sort of bug as in compile/20001205-1.c
> except that an unspec is involved instead of an asm_operands.
> 
> I wonder if the best solution is to avoid local-alloc creating
> REG_EQUIV notes for every little thing under the sun.  Allow
> only general_operand or something.

I am testing the following.  The bootstrap has gone well past the above
point.  Maybe some generalization of this would work in general or
some similar control could be added to local-alloc regarding the creation
of REG_EQUIV notes.  This method works well here because there are no
valid substitutions of PIC mem's on the PA as far as I am aware.  Another
method might be to scan the insn list and check that the substitution is
valid for all the insns that use the register before deciding that a
REG_EQUIV note should be added.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)

2000-12-06  John David Anglin  <dave@hiauly1.hia.nrc.ca>

	* pa.h (PIC_ADDRESS_P): Define macro to recognize PIC address.
	* reload1.c (reload): Don't set reg_equiv_mem if PIC_ADDRESS_P
	is defined and true.

--- config/pa/pa.h.orig	Sat Dec  2 02:43:02 2000
+++ config/pa/pa.h	Wed Dec  6 17:27:21 2000
@@ -1091,6 +1091,14 @@
    || GET_CODE (X) == HIGH) 						\
    && (reload_in_progress || reload_completed || ! symbolic_expression_p (X)))
 
+/* Recognize a PIC address.  */
+
+#define PIC_ADDRESS_P(X)			\
+   (GET_CODE (X) == LO_SUM			\
+    && GET_CODE (XEXP (X, 0)) == REG            \
+    && REG_OK_FOR_BASE_P (XEXP (X, 0))		\
+    && GET_CODE (XEXP (X, 1)) == UNSPEC)	\
+
 /* Include all constant integers and constant doubles, but not
    floating-point, except for floating-point zero.
 
--- reload1.c.orig	Thu Nov 30 17:39:21 2000
+++ reload1.c	Wed Dec  6 17:18:27 2000
@@ -881,8 +881,14 @@
 	  {
 	    rtx x = eliminate_regs (reg_equiv_memory_loc[i], 0, NULL_RTX);
 
+#ifndef PIC_ADDRESS_P
 	    if (strict_memory_address_p (GET_MODE (regno_reg_rtx[i]),
 					 XEXP (x, 0)))
+#else
+	    if (strict_memory_address_p (GET_MODE (regno_reg_rtx[i]),
+					 XEXP (x, 0))
+		&& ! PIC_ADDRESS_P (XEXP (x, 0)))
+#endif
 	      reg_equiv_mem[i] = x, reg_equiv_address[i] = 0;
 	    else if (CONSTANT_P (XEXP (x, 0))
 		     || (GET_CODE (XEXP (x, 0)) == REG

Re: pa reload problem

[John David Anglin]

> I am testing the following.  The bootstrap has gone well past the above
> point.  Maybe some generalization of this would work in general or

Failure:

stage1/xgcc -Bstage1/ -B/usr/local/hppa1.1-hp-hpux10.20/bin/ -c  -DIN_GCC    -g -O3 -fPIC -W -Wall -Wwrite-strings -Wstrict-prototypes -Wmissing-prototypes   -DHAVE_CONFIG_H    -I. -Icp -I../../gcc -I../../gcc/cp -I../../gcc/config -I../../gcc/../include ../../gcc/cp/decl.c -o cp/decl.o
../../gcc/cp/decl.c: In function `duplicate_decls':
../../gcc/cp/decl.c:3760: Internal error: Segmentation fault.
Please submit a full bug report.
See <URL:http://www.gnu.org/software/gcc/bugs.html> for instructions.
make[2]: *** [cp/decl.o] Error 1
make[2]: Leaving directory `/xxx/gnu/gcc-2.97/objdir/gcc'
make[1]: *** [stage_c] Error 2
make[1]: Leaving directory `/xxx/gnu/gcc-2.97/objdir/gcc'
make: *** [bootstrap-lean] Error 2

Hopefully, this is a different problem.

[snip]
decls_match warn_extern_redeclared_static duplicate_decls {GC 6819k -> 2774k} {GC 5707k -> 2827k}
Program received signal SIGSEGV, Segmentation fault0x35c264 in make_reorder_chain_1 (bb=0x401a3408, prev=0x401a3408)
    at ../../gcc/bb-reorder.c:409
409           next = (taken e_taken : e_fall)->dest;
(gdb) p taken
$1 = 1
(gdb) p e_taken->dest
Cannot access memory at address 0xc
(gdb) p e_fall->dest
$2 = (struct basic_block_def *) 0x401a3440

Dave
-- 
J. David Anglin                                  dave.anglin@nrc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)

Re: pa reload problem

[John David Anglin]

> On Wed, Dec 06, 2000 at 03:41:06PM -0500, John David Anglin wrote:
> > >     (expr_list:REG_EQUIV (mem/u:SI (lo_sum:SI (reg/f:SI 309)
> > > 		(unspec:SI[
> > > 			(symbol_ref:SI ("constructor_max_index"))
> > > 		    ]  0)) 0)
> 
> This appears to be the same sort of bug as in compile/20001205-1.c
> except that an unspec is involved instead of an asm_operands.
> 
> I wonder if the best solution is to avoid local-alloc creating
> REG_EQUIV notes for every little thing under the sun.  Allow
> only general_operand or something.

I am wondering if side_effects_p should return 1 if it encounters
an UNSPEC.  Currently, it just ignores it.  This would prevent the
generation of the REG_EQUIV notes and I think resolve the problem.
However, it might have other side effects.  Any thoughts on this?

Dave
-- 
J. David Anglin                                  dave.anglin@nrc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)

Re: pa reload problem

[Richard Henderson]

On Fri, Dec 08, 2000 at 05:05:21PM -0500, John David Anglin wrote:
> I am wondering if side_effects_p should return 1 if it encounters
> an UNSPEC.

No, because an UNSPEC has no side effects.

As I said before, the simplest thing is to check for general_operand
before creating the REG_EQUAL note.  Any other solution requires reload
to Do Things to the INSN_CODE during rematerialization.


r~

Re: pa reload problem

[John David Anglin]

> On Fri, Dec 08, 2000 at 05:05:21PM -0500, John David Anglin wrote:
> > I am wondering if side_effects_p should return 1 if it encounters
> > an UNSPEC.
> 
> No, because an UNSPEC has no side effects.
> 
> As I said before, the simplest thing is to check for general_operand
> before creating the REG_EQUAL note.  Any other solution requires reload
> to Do Things to the INSN_CODE during rematerialization.

It is my impression that the MEM would pass as a general_operand unless
the volatile flag is set.  It will pass GO_IF_LEGITIMATE_ADDRESS.  Thus,
the general_operand test doesn't look like it will work.

Currently, the unchanging bit is set for the MEM.  The comments in
rtl.h say that the bit in a SYMBOL_REF is used for machine specific
purposes.  Could it be used to add machine dependent side effects to
a SYMBOL_REF (or possibly an UNSPEC)?

I think Doing Things to the INSN_CODE during rematerialization is too
late unless there is an initial check to determine whether this is possible.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)

Re: pa reload problem

[Richard Henderson]

On Fri, Dec 08, 2000 at 08:07:23PM -0500, John David Anglin wrote:
> It is my impression that the MEM would pass as a general_operand unless
> the volatile flag is set.  It will pass GO_IF_LEGITIMATE_ADDRESS.  Thus,
> the general_operand test doesn't look like it will work.

Your GO_IF_LEGITIMATE_ADDRESS will accept the UNSPEC?
The mind boggles.  Why, then, is this strange beast its own insn?

Perhaps that is part of the bug...


r~

Re: pa reload problem

[John David Anglin]

> 
> On Fri, Dec 08, 2000 at 08:07:23PM -0500, John David Anglin wrote:
> > It is my impression that the MEM would pass as a general_operand unless
> > the volatile flag is set.  It will pass GO_IF_LEGITIMATE_ADDRESS.  Thus,
> > the general_operand test doesn't look like it will work.
> 
> Your GO_IF_LEGITIMATE_ADDRESS will accept the UNSPEC?
> The mind boggles.  Why, then, is this strange beast its own insn?

Sure looks like it:

#define GO_IF_LEGITIMATE_ADDRESS(MODE, X, ADDR)  \ 
[snip]
  /* Needed for -fPIC */                                \
  else if (GET_CODE (X) == LO_SUM                       \
           && GET_CODE (XEXP (X, 0)) == REG             \
           && REG_OK_FOR_BASE_P (XEXP (X, 0))           \
           && GET_CODE (XEXP (X, 1)) == UNSPEC)         \
    goto ADDR;                                          \
}

The comment for the insn used to load a PIC MEM says:

; We need this to make sure CSE doesn't simplify a memory load with a
; symbolic address, whose content it think it knows.  For PIC, what CSE
; think is the real value will be the address of that value.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)

Re: pa reload problem

[Jeffrey A Law]

  In message <20001208173948.B4198@redhat.com>you write:
  > On Fri, Dec 08, 2000 at 08:07:23PM -0500, John David Anglin wrote:
  > > It is my impression that the MEM would pass as a general_operand unless
  > > the volatile flag is set.  It will pass GO_IF_LEGITIMATE_ADDRESS.  Thus,
  > > the general_operand test doesn't look like it will work.
  > 
  > Your GO_IF_LEGITIMATE_ADDRESS will accept the UNSPEC?
  > The mind boggles.  Why, then, is this strange beast its own insn?
  > 
  > Perhaps that is part of the bug...
Ironic that this is the hack that I ripped out (for basically the same reasons)
when I started working on V3.

Unfortunately, I can't actually test it yet to see what problems removing
that old hack will expose (FWIW, it's not my hack, and it pre-dates movement
of everything to public lists, so there's no discussion of why the change
was made).


jeff

Re: pa reload problem

[Richard Henderson]

On Fri, Dec 08, 2000 at 09:05:27PM -0700, Jeffrey A Law wrote:
> Unfortunately, I can't actually test it yet to see what problems removing
> that old hack will expose...

Heh.  Perhaps you could forward it to the hppa-linux folk and
see what happens there.  Mayhap their linker is differently
crash-prone.


r~

Re: pa reload problem

[Alan Modra]

On Fri, 8 Dec 2000, Jeffrey A Law wrote:

> Ironic that this is the hack that I ripped out (for basically the same reasons)
> when I started working on V3.

Talking of hacks, and changing the subject, I've been hacking the pa
prologue and epilogue code for dwarf2 unwind support.  Still at the
head-scratching stage (danger of splinters), with aborts in
dwarf2out_frame_debug_expr, but here's a fix for a couple of problems in
except.c:eh_regs.

	* except.c (eh_regs): Save results of build_pointer_type to a temp
	as FUNCTION_VALUE macro may evaluate its args multiple times.
	When allocating registers, check for GENERAL_REGS class.

On hppa, we tried to allocate r0 :-(

Alan Modra
-- 
Linuxcare.  Support for the Revolution.

--- gcc/except.c~	Mon Dec  4 14:55:35 2000
+++ gcc/except.c	Sat Dec  9 17:48:21 2000
@@ -3000,15 +3000,15 @@ eh_regs (pcontext, psp, pra, outgoing)
 {
   rtx rcontext, rsp, rra;
   unsigned int i;
+  tree t;
 
+  t = build_pointer_type (void_type_node);
 #ifdef FUNCTION_OUTGOING_VALUE
   if (outgoing)
-    rcontext = FUNCTION_OUTGOING_VALUE (build_pointer_type (void_type_node),
-				        current_function_decl);
+    rcontext = FUNCTION_OUTGOING_VALUE (t, current_function_decl);
   else
 #endif
-    rcontext = FUNCTION_VALUE (build_pointer_type (void_type_node),
-			       current_function_decl);
+    rcontext = FUNCTION_VALUE (t, current_function_decl);
 
 #ifdef STATIC_CHAIN_REGNUM
   if (outgoing)
@@ -3023,7 +3023,9 @@ eh_regs (pcontext, psp, pra, outgoing)
   if (rsp == NULL_RTX)
     {
       for (i = 0; i < FIRST_PSEUDO_REGISTER; ++i)
-	if (call_used_regs[i] && ! fixed_regs[i] && i != REGNO (rcontext))
+	if (call_used_regs[i] && ! fixed_regs[i]
+	    && REGNO_REG_CLASS (i) == GENERAL_REGS
+	    && i != REGNO (rcontext))
 	  break;
       if (i == FIRST_PSEUDO_REGISTER)
 	abort();
@@ -3033,6 +3035,7 @@ eh_regs (pcontext, psp, pra, outgoing)
 
   for (i = 0; i < FIRST_PSEUDO_REGISTER; ++i)
     if (call_used_regs[i] && ! fixed_regs[i]
+	&& REGNO_REG_CLASS (i) == GENERAL_REGS
 	&& i != REGNO (rcontext) && i != REGNO (rsp))
       break;
   if (i == FIRST_PSEUDO_REGISTER)

Re: pa reload problem

[Richard Henderson]

On Sat, Dec 09, 2000 at 06:52:13PM +1100, Alan Modra wrote:
>    for (i = 0; i < FIRST_PSEUDO_REGISTER; ++i)
>      if (call_used_regs[i] && ! fixed_regs[i]
> +	&& REGNO_REG_CLASS (i) == GENERAL_REGS

This isn't what you meant.  You meant "is this register a 
member of general regs".  What you got was "is general regs
the most specific class for this register".

Which means that ports like x86 where practically every
register has its own class will break.


r~

Re: pa reload problem

[Alan Modra]

On Fri, 8 Dec 2000, Richard Henderson wrote:

> On Sat, Dec 09, 2000 at 06:52:13PM +1100, Alan Modra wrote:
> >    for (i = 0; i < FIRST_PSEUDO_REGISTER; ++i)
> >      if (call_used_regs[i] && ! fixed_regs[i]
> > +	&& REGNO_REG_CLASS (i) == GENERAL_REGS
> 
> This isn't what you meant.  You meant "is this register a 
> member of general regs".  What you got was "is general regs
> the most specific class for this register".

Ah.  What I had to start with was REGNO_REG_CLASS (i) != NO_REGS.
Should have stuck with that.  Hmm, I see local_alloc uses
HARD_REGNO_MODE_OK, so is this better?

        * except.c (eh_regs): Save results of build_pointer_type to a temp
        as FUNCTION_VALUE macro may evaluate its args multiple times.
        When allocating registers, check that the mode is OK.

-- 
Linuxcare.  Support for the Revolution.

--- gcc/except.c~	Mon Dec  4 14:55:35 2000
+++ gcc/except.c	Sat Dec  9 18:56:48 2000
@@ -3000,15 +3000,15 @@ eh_regs (pcontext, psp, pra, outgoing)
 {
   rtx rcontext, rsp, rra;
   unsigned int i;
+  tree t;
 
+  t = build_pointer_type (void_type_node);
 #ifdef FUNCTION_OUTGOING_VALUE
   if (outgoing)
-    rcontext = FUNCTION_OUTGOING_VALUE (build_pointer_type (void_type_node),
-				        current_function_decl);
+    rcontext = FUNCTION_OUTGOING_VALUE (t, current_function_decl);
   else
 #endif
-    rcontext = FUNCTION_VALUE (build_pointer_type (void_type_node),
-			       current_function_decl);
+    rcontext = FUNCTION_VALUE (t, current_function_decl);
 
 #ifdef STATIC_CHAIN_REGNUM
   if (outgoing)
@@ -3023,7 +3023,9 @@ eh_regs (pcontext, psp, pra, outgoing)
   if (rsp == NULL_RTX)
     {
       for (i = 0; i < FIRST_PSEUDO_REGISTER; ++i)
-	if (call_used_regs[i] && ! fixed_regs[i] && i != REGNO (rcontext))
+	if (call_used_regs[i] && ! fixed_regs[i]
+	    && HARD_REGNO_MODE_OK (i, Pmode)
+	    && i != REGNO (rcontext))
 	  break;
       if (i == FIRST_PSEUDO_REGISTER)
 	abort();
@@ -3033,6 +3035,7 @@ eh_regs (pcontext, psp, pra, outgoing)
 
   for (i = 0; i < FIRST_PSEUDO_REGISTER; ++i)
     if (call_used_regs[i] && ! fixed_regs[i]
+	&& HARD_REGNO_MODE_OK (i, Pmode)
 	&& i != REGNO (rcontext) && i != REGNO (rsp))
       break;
   if (i == FIRST_PSEUDO_REGISTER)

Re: pa reload problem

[Alan Modra]

On Sat, 9 Dec 2000, Alan Modra wrote:

> Ah.  What I had to start with was REGNO_REG_CLASS (i) != NO_REGS.
> Should have stuck with that.  Hmm, I see local_alloc uses
> HARD_REGNO_MODE_OK, so is this better?

Grrr, that allocates r1 first, which loses on 32-bit hppa.  Here's yet
another try, a little more sophisticated this time.

        * except.c (eh_regs): Save results of build_pointer_type to a temp
        as FUNCTION_VALUE macro may evaluate its args multiple times.
        When allocating registers, do so according to REG_ALLOC_ORDER and
	choose GENERAL_REGS.

I was tempted to remove the call_used_regs and fixed_regs tests when
REG_ALLOC_ORDER is defined, but on checking some targets found i860 has a
fixed reg as it's no. 1 priority reg.  Strange.

Alan Modra
-- 
Linuxcare.  Support for the Revolution.

--- gcc/except.c~	Mon Dec  4 14:55:35 2000
+++ gcc/except.c	Sun Dec 10 00:11:29 2000
@@ -2999,16 +2999,16 @@ eh_regs (pcontext, psp, pra, outgoing)
      int outgoing ATTRIBUTE_UNUSED;
 {
   rtx rcontext, rsp, rra;
-  unsigned int i;
+  unsigned int i, r;
+  tree t;
 
+  t = build_pointer_type (void_type_node);
 #ifdef FUNCTION_OUTGOING_VALUE
   if (outgoing)
-    rcontext = FUNCTION_OUTGOING_VALUE (build_pointer_type (void_type_node),
-				        current_function_decl);
+    rcontext = FUNCTION_OUTGOING_VALUE (t, current_function_decl);
   else
 #endif
-    rcontext = FUNCTION_VALUE (build_pointer_type (void_type_node),
-			       current_function_decl);
+    rcontext = FUNCTION_VALUE (t, current_function_decl);
 
 #ifdef STATIC_CHAIN_REGNUM
   if (outgoing)
@@ -3023,22 +3023,39 @@ eh_regs (pcontext, psp, pra, outgoing)
   if (rsp == NULL_RTX)
     {
       for (i = 0; i < FIRST_PSEUDO_REGISTER; ++i)
-	if (call_used_regs[i] && ! fixed_regs[i] && i != REGNO (rcontext))
-	  break;
+	{
+#ifdef REG_ALLOC_ORDER
+	  r = reg_alloc_order[i];
+#else
+	  r = i;
+#endif
+	  if (call_used_regs[r] && ! fixed_regs[r]
+	      && TEST_HARD_REG_BIT (reg_class_contents[(int) GENERAL_REGS], r)
+	      && r != REGNO (rcontext))
+	    break;
+	}
       if (i == FIRST_PSEUDO_REGISTER)
 	abort();
 
-      rsp = gen_rtx_REG (Pmode, i);
+      rsp = gen_rtx_REG (Pmode, r);
     }
 
   for (i = 0; i < FIRST_PSEUDO_REGISTER; ++i)
-    if (call_used_regs[i] && ! fixed_regs[i]
-	&& i != REGNO (rcontext) && i != REGNO (rsp))
-      break;
+    {
+#ifdef REG_ALLOC_ORDER
+      r = reg_alloc_order[i];
+#else
+      r = i;
+#endif
+      if (call_used_regs[r] && ! fixed_regs[r]
+	  && TEST_HARD_REG_BIT (reg_class_contents[(int) GENERAL_REGS], r)
+	  && r != REGNO (rcontext) && r != REGNO (rsp))
+	break;
+    }
   if (i == FIRST_PSEUDO_REGISTER)
     abort();
 
-  rra = gen_rtx_REG (Pmode, i);
+  rra = gen_rtx_REG (Pmode, r);
 
   *pcontext = rcontext;
   *psp = rsp;

Re: pa reload problem

[Richard Henderson]

On Sun, Dec 10, 2000 at 12:57:03AM +1100, Alan Modra wrote:
> Grrr, that allocates r1 first, which loses on 32-bit hppa.

Err, elaborate on "loses".

>       When allocating registers, do so according to REG_ALLOC_ORDER and
> 	choose GENERAL_REGS.

The problem with this is that you've broken the ABI for all
extant targets.


r~

Re: pa reload problem

[Alan Modra]

On Sat, 9 Dec 2000, Richard Henderson wrote:

> On Sun, Dec 10, 2000 at 12:57:03AM +1100, Alan Modra wrote:
> > Grrr, that allocates r1 first, which loses on 32-bit hppa.
> 
> Err, elaborate on "loses".

Like so

/src/parisc/gcc-20001208/gcc/libgcc2.c: In function `__throw':
/src/parisc/gcc-20001208/gcc/libgcc2.c:4121: Unable to find a register to
spill in class `R1_REGS'.
/src/parisc/gcc-20001208/gcc/libgcc2.c:4121: This is the insn:

(insn 161 190 163 (set (reg/f:SI 21 %r21)
        (label_ref:SI 169)) 79 {*pa.md:2287} (nil)
    (expr_list:REG_EQUIV (label_ref:SI 169)
        (insn_list:REG_LABEL 169 (nil))))

> >       When allocating registers, do so according to REG_ALLOC_ORDER and
> > 	choose GENERAL_REGS.
> 
> The problem with this is that you've broken the ABI for all
> extant targets.

Pity.  It looked like a nice solution to me.

-- 
Linuxcare.  Support for the Revolution.

Re: pa reload problem

[Jeffrey A Law]

  In message <20001208225552.A4476@redhat.com>you write:
  > On Fri, Dec 08, 2000 at 09:05:27PM -0700, Jeffrey A Law wrote:
  > > Unfortunately, I can't actually test it yet to see what problems removing
  > > that old hack will expose...
  > 
  > Heh.  Perhaps you could forward it to the hppa-linux folk and
  > see what happens there.  Mayhap their linker is differently
  > crash-prone.
I wouldn't count on it :-)  And in general PA linux just isn't ready for
the kind of testing I need to do for this patch.

What I really need to do is backport it into devo or 2000r1 where I can
beat on the PIC & shared library code a little harder than I can on the
external tree right now due to libstdc++-v3 issues.

jeff

Re: pa reload problem

[John David Anglin]

>   In message <20001208225552.A4476@redhat.com>you write:
>   > On Fri, Dec 08, 2000 at 09:05:27PM -0700, Jeffrey A Law wrote:
>   > > Unfortunately, I can't actually test it yet to see what problems removing
>   > > that old hack will expose...
>   > 
>   > Heh.  Perhaps you could forward it to the hppa-linux folk and
>   > see what happens there.  Mayhap their linker is differently
>   > crash-prone.
> I wouldn't count on it :-)  And in general PA linux just isn't ready for
> the kind of testing I need to do for this patch.
> 
> What I really need to do is backport it into devo or 2000r1 where I can
> beat on the PIC & shared library code a little harder than I can on the
> external tree right now due to libstdc++-v3 issues.

This thread started when I attempted a bootstrap with "-g -O3 -fPIC".  The
full bootstrap gives a pretty good check of PIC code generation at least
for non shared code.  For the moment, I disable the new ABI and v3 for
my builds.  You might try "--enable-shared" as well.

I was going to deleting the hack and adding a general_operand test in
local-alloc to see if that would fix the reload problem with "-fPIC".
However, for the moment, there are still problems with a "normal" build
at "-O3".  With only the yesterdays CVS source and the patch below
to prevent rename registers walking of the return pointer, I get the
following failure building libstdc++ v2:

/xxx/gnu/gcc-2.97/objdir/gcc/g++ -B/xxx/gnu/gcc-2.97/objdir/gcc/ -nostdinc++ -isystem /xxx/gnu/gcc-2.97/libstdc++ -isystem /xxx/gnu/gcc-2.97/libstdc++/std -isystem /xxx/gnu/gcc-2.97/libstdc++/stl -isystem /xxx/gnu/gcc-2.97/libio -isystem /xxx/gnu/gcc-2.97/objdir/hppa1.1-hp-hpux10.20/libio -L/xxx/gnu/gcc-2.97/objdir/hppa1.1-hp-hpux10.20/libstdc++ -B/usr/local/hppa1.1-hp-hpux10.20/bin/ -B/usr/local/hppa1.1-hp-hpux10.20/lib/ -isystem /usr/local/hppa1.1-hp-hpux10.20/include -c -O3 -fno-implicit-templates -I../../../libstdc++ -I../../../libstdc++/stl -I../libio -I../../../libstdc++/../libio -I../../../libstdc++/../include -I../../../libstdc++/../gcc -nostdinc++  ../../../libstdc++/exception.cc
../../../libstdc++/exception.cc: In function `void __check_eh_spec(int, const 
   void**)':
../../../libstdc++/exception.cc:363: Internal error: Segmentation fault.
Please submit a full bug report.
See <URL:http://www.gnu.org/software/gcc/bugs.html> for instructions.
make[1]: *** [exception.o] Error 1
make[1]: Leaving directory `/xxx/gnu/gcc-2.97/objdir/hppa1.1-hp-hpux10.20/libstdc++'
make: *** [all-target-libstdc++] Error 2

The seg fault occurs in make_edge called from make_label_edge.  It occurs
because BLOCK_FOR_INSN (label) is 0.  The label either has been deleted or
never emitted.  I believe the reference comes from a note.  Didn't have
this problem last week.  If anybody is aware of a change that might have
affected this, let me know.

Progress has been slow in debugging this because there are problems with
the current version of gdb.  Register printouts and even the location
of seg faults is sometimes wrong.  Have to use adb to check results (yuck).
Need to get the current cvs version of gdb working properly on the pa again.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)

2000-12-08  John David Anglin  <dave@hiauly1.hia.nrc.ca>

	* pa.h: Define RETURN_POINTER_REGNUM and EPILOGUE_USES.

--- pa.h.orig	Wed Dec  6 20:21:25 2000
+++ pa.h	Fri Dec  8 16:28:58 2000
@@ -480,6 +480,9 @@
 #define INITIAL_FRAME_POINTER_OFFSET(VAR) \
   do {(VAR) = - compute_frame_size (get_frame_size (), 0);} while (0)
 
+/* Register in which the return pointer/address is passed to a function.  */
+#define RETURN_POINTER_REGNUM 2
+
 /* Base register for access to arguments of the function.  */
 #define ARG_POINTER_REGNUM 3
 
@@ -890,6 +893,11 @@
    functions that have frame pointers.
    No definition is equivalent to always zero.  */
 
+/* Define this macro as a C expression that is nonzero for registers
+   used by the epilogue or the `return' pattern.  */
+
+#define EPILOGUE_USES(REGNO) ((REGNO) == RETURN_POINTER_REGNUM)
+
 extern int may_call_alloca;
 
 #define EXIT_IGNORE_STACK	\

Re: pa reload problem

[Michael Meissner]

On Fri, Dec 08, 2000 at 03:45:27PM -0800, Richard Henderson wrote:
> On Fri, Dec 08, 2000 at 05:05:21PM -0500, John David Anglin wrote:
> > I am wondering if side_effects_p should return 1 if it encounters
> > an UNSPEC.
> 
> No, because an UNSPEC has no side effects.

To be pedantic, an UNSPEC by itself has no side effects, but obviously an
UNSPEC could have arguments that use PRE_DEC, PRE_INC, POST_MODIFY, etc.

> As I said before, the simplest thing is to check for general_operand
> before creating the REG_EQUAL note.  Any other solution requires reload
> to Do Things to the INSN_CODE during rematerialization.

-- 
Michael Meissner, Red Hat, Inc.
PMB 198, 174 Littleton Road #3, Westford, Massachusetts 01886, USA
Work:	  meissner@redhat.com		phone: +1 978-486-9304
Non-work: meissner@spectacle-pond.org	fax:   +1 978-692-4482

Re: pa reload problem

[Alan Modra]

On Mon, 11 Dec 2000, John David Anglin wrote:

> Progress has been slow in debugging this because there are problems with
> the current version of gdb.

You're not kidding!  I'm rewriting the gdb hppa target dependent code to
use the gdb multi-arch scheme...

>  Register printouts and even the location
> of seg faults is sometimes wrong.  Have to use adb to check results (yuck).
> Need to get the current cvs version of gdb working properly on the pa again.

-- 
Linuxcare.  Support for the Revolution.

Re: pa reload problem

[John David Anglin]

> I was going to deleting the hack and adding a general_operand test in
> local-alloc to see if that would fix the reload problem with "-fPIC".
> However, for the moment, there are still problems with a "normal" build
> at "-O3".  With only the yesterdays CVS source and the patch below
> to prevent rename registers walking of the return pointer, I get the
> following failure building libstdc++ v2:
> 
> /xxx/gnu/gcc-2.97/objdir/gcc/g++ -B/xxx/gnu/gcc-2.97/objdir/gcc/ -nostdinc++ -isystem /xxx/gnu/gcc-2.97/libstdc++ -isystem /xxx/gnu/gcc-2.97/libstdc++/std -isystem /xxx/gnu/gcc-2.97/libstdc++/stl -isystem /xxx/gnu/gcc-2.97/libio -isystem /xxx/gnu/gcc-2.97/objdir/hppa1.1-hp-hpux10.20/libio -L/xxx/gnu/gcc-2.97/objdir/hppa1.1-hp-hpux10.20/libstdc++ -B/usr/local/hppa1.1-hp-hpux10.20/bin/ -B/usr/local/hppa1.1-hp-hpux10.20/lib/ -isystem /usr/local/hppa1.1-hp-hpux10.20/include -c -O3 -fno-implicit-templates -I../../../libstdc++ -I../../../libstdc++/stl -I../libio -I../../../libstdc++/../libio -I../../../libstdc++/../include -I../../../libstdc++/../gcc -nostdinc++  ../../../libstdc++/exception.cc
> ../../../libstdc++/exception.cc: In function `void __check_eh_spec(int, const 
>    void**)':
> ../../../libstdc++/exception.cc:363: Internal error: Segmentation fault.

I have made some progress in locating this bug but still don't have a
complete understanding of the problem.  The problem is that a code_label
insn is "incorrectly" deleted in the loop pass.  Here is the rtl from
the gcse pass:

[snip]

(code_label 1158 1268 1439 124 "" "" [3 uses])

(note 1439 1158 262 [bb 12] NOTE_INSN_BASIC_BLOCK -1347440721)

(note 262 1439 1159 85 NOTE_INSN_EH_REGION_END -1347440721)

[snip]

(insn 788 787 789 (set (reg/f:SI 209)
        (high:SI (label_ref:SI 1158))) 87 {*pa.md:2435} (nil)
    (expr_list:REG_EQUAL (high:SI (label_ref:SI 1158))
        (insn_list:REG_LABEL 1158 (nil))))

(insn 789 788 791 (set (reg/f:SI 208)
        (lo_sum:SI (reg/f:SI 209)
            (label_ref:SI 1158))) 90 {*pa.md:2467} (nil)
    (insn_list:REG_LABEL 1158 (expr_list:REG_EQUAL (label_ref:SI 1158)
            (nil))))

Here is the rtl after loop:

Loop from 576 to 1288: 105 real insns.
Continue at insn 1278.

[snip]

Insn 788: regno 209 (life 2), move-insn savings 2  moved to 1530
Insn 789: regno 208 (life 1), move-insn forces 788 savings 1  moved to 1532

[snip]

(insn 1531 1528 1532 (set (reg/f:SI 337)
        (high:SI (label_ref:SI 1158))) -1 (nil)
    (expr_list:REG_LABEL (code_label/v 1158 1268 1439 124 "" "" [0 uses])
        (nil)))

(insn 1532 1531 1535 (set (reg/f:SI 208)
        (lo_sum:SI (reg/f:SI 337)
            (label_ref:SI 1158))) -1 (nil)
    (expr_list:REG_EQUAL (label_ref:SI 1158)
        (expr_list:REG_LABEL (code_label/v 1158 1268 1439 124 "" "" [0 uses])
            (nil))))

The code_label 1158 is deleted by delete_trivially_dead_insns apparently 
because the preceding call to loop_optimize in toplev.c has reduced the
number of uses to 1.  Maybe somebody can see how this occurs.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)

Re: pa reload problem

[Jeffrey A Law]

  In message <200012140048.TAA02603@hiauly1.hia.nrc.ca>you write:
  > I have made some progress in locating this bug but still don't have a
  > complete understanding of the problem.  The problem is that a code_label
  > insn is "incorrectly" deleted in the loop pass.  Here is the rtl from
  > the gcse pass:
  > 
  > [snip]
  > 
  > (code_label 1158 1268 1439 124 "" "" [3 uses])
  > 
  > (note 1439 1158 262 [bb 12] NOTE_INSN_BASIC_BLOCK -1347440721)
  > 
  > (note 262 1439 1159 85 NOTE_INSN_EH_REGION_END -1347440721)
  > 
  > [snip]
  > 
  > (insn 788 787 789 (set (reg/f:SI 209)
  >         (high:SI (label_ref:SI 1158))) 87 {*pa.md:2435} (nil)
  >     (expr_list:REG_EQUAL (high:SI (label_ref:SI 1158))
  >         (insn_list:REG_LABEL 1158 (nil))))
  > 
  > (insn 789 788 791 (set (reg/f:SI 208)
  >         (lo_sum:SI (reg/f:SI 209)
  >             (label_ref:SI 1158))) 90 {*pa.md:2467} (nil)
  >     (insn_list:REG_LABEL 1158 (expr_list:REG_EQUAL (label_ref:SI 1158)
  >             (nil))))
  > 
  > Here is the rtl after loop:
  > 
  > Loop from 576 to 1288: 105 real insns.
  > Continue at insn 1278.
  > 
  > [snip]
  > 
  > Insn 788: regno 209 (life 2), move-insn savings 2  moved to 1530
  > Insn 789: regno 208 (life 1), move-insn forces 788 savings 1  moved to 1532
  > 
  > [snip]
  > 
  > (insn 1531 1528 1532 (set (reg/f:SI 337)
  >         (high:SI (label_ref:SI 1158))) -1 (nil)
  >     (expr_list:REG_LABEL (code_label/v 1158 1268 1439 124 "" "" [0 uses])
  >         (nil)))
  > 
  > (insn 1532 1531 1535 (set (reg/f:SI 208)
  >         (lo_sum:SI (reg/f:SI 337)
  >             (label_ref:SI 1158))) -1 (nil)
  >     (expr_list:REG_EQUAL (label_ref:SI 1158)
  >         (expr_list:REG_LABEL (code_label/v 1158 1268 1439 124 "" "" [0 uses
  > ])
  >             (nil))))
  > 
  > The code_label 1158 is deleted by delete_trivially_dead_insns apparently 
  > because the preceding call to loop_optimize in toplev.c has reduced the
  > number of uses to 1.  Maybe somebody can see how this occurs.
Seems to me we have a reference counting problem.  There is clearly 
a reference to label 1158 (insn 1531/1532), but it's reference count
is zero.

Seems to me that if you find that reference counting bug that you'll
fix this problem.

jeff

Re: pa reload problem

[John David Anglin]

>   In message <200012140048.TAA02603@hiauly1.hia.nrc.ca>you write:
>   > I have made some progress in locating this bug but still don't have a
>   > complete understanding of the problem.  The problem is that a code_label
>   > insn is "incorrectly" deleted in the loop pass.  Here is the rtl from
>   > the gcse pass:
>   > 
>   > [snip]
>   > 
>   > (code_label 1158 1268 1439 124 "" "" [3 uses])
>   > 
>   > (note 1439 1158 262 [bb 12] NOTE_INSN_BASIC_BLOCK -1347440721)
>   > 
>   > (note 262 1439 1159 85 NOTE_INSN_EH_REGION_END -1347440721)
>   > 
>   > [snip]
>   > 
>   > (insn 788 787 789 (set (reg/f:SI 209)
>   >         (high:SI (label_ref:SI 1158))) 87 {*pa.md:2435} (nil)
>   >     (expr_list:REG_EQUAL (high:SI (label_ref:SI 1158))
>   >         (insn_list:REG_LABEL 1158 (nil))))
>   > 
>   > (insn 789 788 791 (set (reg/f:SI 208)
>   >         (lo_sum:SI (reg/f:SI 209)
>   >             (label_ref:SI 1158))) 90 {*pa.md:2467} (nil)
>   >     (insn_list:REG_LABEL 1158 (expr_list:REG_EQUAL (label_ref:SI 1158)
>   >             (nil))))
>   > 
>   > Here is the rtl after loop:
>   > 
>   > Loop from 576 to 1288: 105 real insns.
>   > Continue at insn 1278.
>   > 
>   > [snip]
>   > 
>   > Insn 788: regno 209 (life 2), move-insn savings 2  moved to 1530
>   > Insn 789: regno 208 (life 1), move-insn forces 788 savings 1  moved to 1532
>   > 
>   > [snip]
>   > 
>   > (insn 1531 1528 1532 (set (reg/f:SI 337)
>   >         (high:SI (label_ref:SI 1158))) -1 (nil)
>   >     (expr_list:REG_LABEL (code_label/v 1158 1268 1439 124 "" "" [0 uses])
>   >         (nil)))
>   > 
>   > (insn 1532 1531 1535 (set (reg/f:SI 208)
>   >         (lo_sum:SI (reg/f:SI 337)
>   >             (label_ref:SI 1158))) -1 (nil)
>   >     (expr_list:REG_EQUAL (label_ref:SI 1158)
>   >         (expr_list:REG_LABEL (code_label/v 1158 1268 1439 124 "" "" [0 uses
>   > ])
>   >             (nil))))
>   > 
>   > The code_label 1158 is deleted by delete_trivially_dead_insns apparently 
>   > because the preceding call to loop_optimize in toplev.c has reduced the
>   > number of uses to 1.  Maybe somebody can see how this occurs.
> Seems to me we have a reference counting problem.  There is clearly 
> a reference to label 1158 (insn 1531/1532), but it's reference count
> is zero.

That's what I thought.  It would appear to occur when the above two insns
get moved out of a loop.  The count gets decremented when the old insns
are deleted but not incremented when the new insns are created.  The puzzle
is why the count is zero rather than 1.  Maybe what happens is the two
insns in the loop are deleted first, then the label is deleted?  Notice
that insn 788 is moved to insn 1530.  However, the insn becomes 1531 and
the reg changes to 337 from 208.

I will try to step through the code later.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)

Re: pa reload problem

[John David Anglin]

> On Fri, Dec 08, 2000 at 08:07:23PM -0500, John David Anglin wrote:
> > It is my impression that the MEM would pass as a general_operand unless
> > the volatile flag is set.  It will pass GO_IF_LEGITIMATE_ADDRESS.  Thus,
> > the general_operand test doesn't look like it will work.
> 
> Your GO_IF_LEGITIMATE_ADDRESS will accept the UNSPEC?
> The mind boggles.  Why, then, is this strange beast its own insn?

I have done some more testing.

Modifying GO_IF_LEGITIMATE_ADDRESS and adding a general_operand test
to validate_equiv_mem doesn't look like it will work.  The fundamental
problem is that the MEM must pass the GO_IF_LEGITIMATE_ADDRESS test during
reload.  However, this is when we would like the UNSPEC to fail the
general_operand test.

Preventing the substitution of equivalent MEMs increases register pressure. 
Thus, it is desirable to allow substitutions where possible.  On the other
hand, the PA instruction can't handle all substitution possibilities.
Thus, even if all the different insns that handle MEMs in one way or
another were combined into a single insn, reload could still produce
something that wouldn't work.

For a given mode, pa.md has several different insns that for example
load memory to a register.  These exist as different insns because
the predicates (and code) differ.  Thus, it is not possible to substitute
the MEM from one insn into another.

I was able to modify print_operand to handle the UNSPEC beast and delete
the `ldw RT' insns for loading a PIC address.  However, I am not certain
that I can work all the insns into a single insn.  I am most concerned
about the index insns `{ldwx|ldw}'.

The simplest solution that I can think of is not to allow equivalent
MEMs on the PA at all (DONT_ALLOW_EQUIV_MEM?).  Since the problem doesn't
seem to happen that frequently given the number of registers available,
possibly this this is a reasonable approach.  What do you think?

Dave
-- 
J. David Anglin                                  dave.anglin@nrc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)

Re: pa reload problem

[Jeffrey A Law]

  In message <200012141640.LAA03285@hiauly1.hia.nrc.ca>you write:
  > That's what I thought.  It would appear to occur when the above two insns
  > get moved out of a loop.  The count gets decremented when the old insns
  > are deleted but not incremented when the new insns are created.  The puzzle
  > is why the count is zero rather than 1.  Maybe what happens is the two
  > insns in the loop are deleted first, then the label is deleted?  Notice
  > that insn 788 is moved to insn 1530.  However, the insn becomes 1531 and
  > the reg changes to 337 from 208.
In this case we typically will bump the number of uses before we delete
the insns that way the label doesn't go away unexpectedly.

jeff

Re: pa reload problem

[John David Anglin]

>   In message <200012141640.LAA03285@hiauly1.hia.nrc.ca>you write:
>   > That's what I thought.  It would appear to occur when the above two insns
>   > get moved out of a loop.  The count gets decremented when the old insns
>   > are deleted but not incremented when the new insns are created.  The puzzle
>   > is why the count is zero rather than 1.  Maybe what happens is the two
>   > insns in the loop are deleted first, then the label is deleted?  Notice
>   > that insn 788 is moved to insn 1530.  However, the insn becomes 1531 and
>   > the reg changes to 337 from 208.
> In this case we typically will bump the number of uses before we delete
> the insns that way the label doesn't go away unexpectedly.

As far I can tell, move_movables doesn't do this when it moves an insn
out of a loop.  It does this by deleting the old insn and creating a new
one.  It calls add_label_notes if notes need to be added to the new insn.

I sent a patch to add_label_notes for review which I think fixes the
problem.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)

Re: pa reload problem

[John David Anglin]

> > Your GO_IF_LEGITIMATE_ADDRESS will accept the UNSPEC?
> > The mind boggles.  Why, then, is this strange beast its own insn?
> 
> I have done some more testing.

As I do more work trying to build the pa port under hpux 10.20 with
`-fPIC -O3', I have begun to understand why this strange beast is its
own insn.  The machine definition which I am testing currently has
the UNSPEC integrated with other move_operands.  However, in doing this,
I have encountered another reload problem.

Here is the relevant rtl from the lreg pass:

;; Function init_all_optabs

...

(note 223 107 225 ("insn-opinit.c") 48 -1347440721)

(insn 225 223 1961 (set (reg/f:SI 611)
        (plus:SI (reg:SI 19 %r19)
            (high:SI (symbol_ref:SI ("target_flags"))))) 80 {*pa.md:2326} (nil)
    (nil))

...

(note 1063 1013 1067 ("insn-opinit.c") 130 -1347440721)

(insn 1067 1063 2002 (set (reg/f:SI 403)
        (mem/u:SI (lo_sum:SI (reg/f:SI 611)
                (unspec:SI[ 
                        (symbol_ref:SI ("target_flags"))
                    ]  0)) 0)) 69 {*pa.md:2099} (nil)
    (expr_list:REG_EQUIV (mem/u:SI (lo_sum:SI (reg/f:SI 611)
                (unspec:SI[ 
                        (symbol_ref:SI ("target_flags"))
                    ]  0)) 0)
        (expr_list:REG_DEAD (reg/f:SI 611)
            (nil))))

...

(note 2006 1059 1069 ("insn-opinit.c") 130 -1347440721)

(insn 1069 2006 2007 (set (reg:SI 404)
        (mem/f:SI (reg/f:SI 403) 4)) 69 {*pa.md:2099} (insn_list 1067 (nil))
    (expr_list:REG_EQUIV (mem/f:SI (reg/f:SI 403) 4)
        (nil)))

...

(note 1085 1926 1091 ("insn-opinit.c") 132 -1347440721)

(insn 1091 1085 1094 (set (reg:SI 411)
        (mem/f:SI (reg/f:SI 403) 4)) 69 {*pa.md:2099} (nil)
    (expr_list:REG_EQUIV (mem/f:SI (reg/f:SI 403) 4)
        (nil)))

...

(note 2168 1597 1607 ("insn-opinit.c") 194 -1347440721)

(insn 1607 2168 2169 (set (reg:SI 538)
        (mem/f:SI (reg/f:SI 403) 4)) 69 {*pa.md:2099} (insn_list:REG_DEP_ANTI 1497 (nil))
    (expr_list:REG_EQUIV (mem/f:SI (reg/f:SI 403) 4)
        (nil)))

...

(note 1621 1931 1627 ("insn-opinit.c") 196 -1347440721)

(insn 1627 1621 1630 (set (reg:SI 544)
        (mem/f:SI (reg/f:SI 403) 4)) 69 {*pa.md:2099} (nil)
    (expr_list:REG_DEAD (reg/f:SI 403)
        (nil)))

Note that register 611 is dead at insn 1067.  However, we still have REG_EQUIV
notes which implicitly use register 611 on insns 1091 and 1607 (not sure
why there isn't a note on 1627).  This leads to disaster at insn 1607
when the hard register assigned to 611 gets reused for other purposes
in the intervening code.

Is the REG_DEAD note for register 611 in the right place?

Dave
-- 
J. David Anglin                                  dave.anglin@nrc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)

Re: pa reload problem

[Jeffrey A Law]

  In message <200012301905.OAA19130@hiauly1.hia.nrc.ca>you write:
  > Note that register 611 is dead at insn 1067.
Yes.


  > However, we still have REG_EQUIV
  > notes which implicitly use register 611 on insns 1091 and 1607 (not sure
  > why there isn't a note on 1627).
Err, no, that's not true.  There is no idea of an "implicit" use like
this.  

  > This leads to disaster at insn 1607
  > when the hard register assigned to 611 gets reused for other purposes
  > in the intervening code.
This indicates a reload bug to me.


  > Is the REG_DEAD note for register 611 in the right place?
Yes.
jeff