Subject: m68k optimizer bug in egcs
To: None <tech-toolchain@netbsd.org>
From: Chas Williams <chas@cmf.nrl.navy.mil>
List: tech-toolchain
Date: 11/16/1998 22:42:29
i rebuilt egcs after the latest round of changes, but the optimizer bug
seen on the m68k (especially when building libgnumalloc) is still there.

i would characterize the bug (and i am not a compiler expert by any 
means) by saying it seemed to fail to eliminate some of the psuedo
registers in subroutines.  i.e. after optimizing malloc(), malloc
would have 3 psuedo registers left in its stack.  the 'correct' code
as produced by gcc 2.7... only has 1 psuedo register.  while these 
extra psuedo's shouldn't be a problem, apparently they aren't being
used correctly (so this would seem to indicate another bug)

however, i had a bit of time to play around with some of the development
releases of egcs, and saw the same optimizer bug in the 19980906 release
but by the 19980914 release, it seemed to be fixed.  the following diff
to global.c seems to fix the problem (i.e. malloc() again only had a single
psuedo register again).  i have applied it to the egcs in the dist/gcc tree,
and now i can building a working libgnumalloc on m68k-netbsd w/ -O.  the
ChangeLog entry that seems to go with this says:

Sat Sep  5 16:34:34 EDT 1998  John Wehle  (john@feith.com)

        * global.c: Update comments.
        (global_alloc): Assign allocation-numbers
        even for registers allocated by local_alloc in case
        they are later spilled and retry_global_alloc is called.
        (mark_reg_store, mark_reg_clobber,
        mark_reg_conflicts, mark_reg_death): Always record a
        conflict with a pseudo register even if it has been
        assigned to a hard register.
        (dump_conflicts): Don't list pseudo registers already assigned to
        a hard register as needing to be allocated, but do list their
        conflicts.

could some other people apply this patch and try it out?

diff -Nrc3p egcs-19980906/gcc/global.c egcs-19980914/gcc/global.c
*** egcs-19980906/gcc/global.c	Sun Jun 21 12:54:56 1998
--- egcs-19980914/gcc/global.c	Mon Sep  7 17:29:34 1998
*************** Boston, MA 02111-1307, USA.  */
*** 45,52 ****
     reg for it.  The reload pass is independent in other respects
     and it is run even when stupid register allocation is in use.
  
!    1. count the pseudo-registers still needing allocation
!    and assign allocation-numbers (allocnos) to them.
     Set up tables reg_allocno and allocno_reg to map 
     reg numbers to allocnos and vice versa.
     max_allocno gets the number of allocnos in use.
--- 45,53 ----
     reg for it.  The reload pass is independent in other respects
     and it is run even when stupid register allocation is in use.
  
!    1. Assign allocation-numbers (allocnos) to the pseudo-registers
!    still needing allocations and to the pseudo-registers currently
!    allocated by local-alloc which may be spilled by reload.
     Set up tables reg_allocno and allocno_reg to map 
     reg numbers to allocnos and vice versa.
     max_allocno gets the number of allocnos in use.
*************** Boston, MA 02111-1307, USA.  */
*** 56,68 ****
     for conflicts between allocnos and explicit hard register use
     (which includes use of pseudo-registers allocated by local_alloc).
  
!    3. for each basic block
      walk forward through the block, recording which
!     unallocated registers and which hardware registers are live.
!     Build the conflict matrix between the unallocated registers
!     and another of unallocated registers versus hardware registers.
      Also record the preferred hardware registers
!     for each unallocated one.
  
     4. Sort a table of the allocnos into order of
     desirability of the variables.
--- 57,69 ----
     for conflicts between allocnos and explicit hard register use
     (which includes use of pseudo-registers allocated by local_alloc).
  
!    3. For each basic block
      walk forward through the block, recording which
!     pseudo-registers and which hardware registers are live.
!     Build the conflict matrix between the pseudo-registers
!     and another of pseudo-registers versus hardware registers.
      Also record the preferred hardware registers
!     for each pseudo-register.
  
     4. Sort a table of the allocnos into order of
     desirability of the variables.
*************** Boston, MA 02111-1307, USA.  */
*** 70,82 ****
     5. Allocate the variables in that order; each if possible into
     a preferred register, else into another register.  */
  
! /* Number of pseudo-registers still requiring allocation
!    (not allocated by local_allocate).  */
  
  static int max_allocno;
  
  /* Indexed by (pseudo) reg number, gives the allocno, or -1
!    for pseudo registers already allocated by local_allocate.  */
  
  int *reg_allocno;
  
--- 71,82 ----
     5. Allocate the variables in that order; each if possible into
     a preferred register, else into another register.  */
  
! /* Number of pseudo-registers which are candidates for allocation. */
  
  static int max_allocno;
  
  /* Indexed by (pseudo) reg number, gives the allocno, or -1
!    for pseudo registers which are not to be allocated.  */
  
  int *reg_allocno;
  
*************** global_alloc (file)
*** 389,401 ****
      /* Note that reg_live_length[i] < 0 indicates a "constant" reg
         that we are supposed to refrain from putting in a hard reg.
         -2 means do make an allocno but don't allocate it.  */
!     if (REG_N_REFS (i) != 0 && reg_renumber[i] < 0 && REG_LIVE_LENGTH (i) != -1
  	/* Don't allocate pseudos that cross calls,
  	   if this function receives a nonlocal goto.  */
  	&& (! current_function_has_nonlocal_label
  	    || REG_N_CALLS_CROSSED (i) == 0))
        {
! 	if (reg_may_share[i] && reg_allocno[reg_may_share[i]] >= 0)
  	  reg_allocno[i] = reg_allocno[reg_may_share[i]];
  	else
  	  reg_allocno[i] = max_allocno++;
--- 389,401 ----
      /* Note that reg_live_length[i] < 0 indicates a "constant" reg
         that we are supposed to refrain from putting in a hard reg.
         -2 means do make an allocno but don't allocate it.  */
!     if (REG_N_REFS (i) != 0 && REG_LIVE_LENGTH (i) != -1
  	/* Don't allocate pseudos that cross calls,
  	   if this function receives a nonlocal goto.  */
  	&& (! current_function_has_nonlocal_label
  	    || REG_N_CALLS_CROSSED (i) == 0))
        {
! 	if (reg_renumber[i] < 0 && reg_may_share[i] && reg_allocno[reg_may_share[i]] >= 0)
  	  reg_allocno[i] = reg_allocno[reg_may_share[i]];
  	else
  	  reg_allocno[i] = max_allocno++;
*************** global_alloc (file)
*** 433,439 ****
    bzero ((char *) local_reg_live_length, sizeof local_reg_live_length);
    bzero ((char *) local_reg_n_refs, sizeof local_reg_n_refs);
    for (i = FIRST_PSEUDO_REGISTER; i < max_regno; i++)
!     if (reg_allocno[i] < 0 && reg_renumber[i] >= 0)
        {
  	int regno = reg_renumber[i];
  	int endregno = regno + HARD_REGNO_NREGS (regno, PSEUDO_REGNO_MODE (i));
--- 433,439 ----
    bzero ((char *) local_reg_live_length, sizeof local_reg_live_length);
    bzero ((char *) local_reg_n_refs, sizeof local_reg_n_refs);
    for (i = FIRST_PSEUDO_REGISTER; i < max_regno; i++)
!     if (reg_renumber[i] >= 0)
        {
  	int regno = reg_renumber[i];
  	int endregno = regno + HARD_REGNO_NREGS (regno, PSEUDO_REGNO_MODE (i));
*************** global_alloc (file)
*** 561,567 ****
  	 except for parameters marked with reg_live_length[regno] == -2.  */
  
        for (i = 0; i < max_allocno; i++)
! 	if (REG_LIVE_LENGTH (allocno_reg[allocno_order[i]]) >= 0)
  	  {
  	    /* If we have more than one register class,
  	       first try allocating in the class that is cheapest
--- 561,568 ----
  	 except for parameters marked with reg_live_length[regno] == -2.  */
  
        for (i = 0; i < max_allocno; i++)
! 	if (reg_renumber[allocno_reg[allocno_order[i]]] < 0
! 	    && REG_LIVE_LENGTH (allocno_reg[allocno_order[i]]) >= 0)
  	  {
  	    /* If we have more than one register class,
  	       first try allocating in the class that is cheapest
*************** mark_reg_store (orig_reg, setter)
*** 1364,1372 ****
  
    regno = REGNO (reg);
  
-   if (reg_renumber[regno] >= 0)
-     regno = reg_renumber[regno] /* + word */;
- 
    /* Either this is one of the max_allocno pseudo regs not allocated,
       or it is or has a hardware reg.  First handle the pseudo-regs.  */
    if (regno >= FIRST_PSEUDO_REGISTER)
--- 1365,1370 ----
*************** mark_reg_store (orig_reg, setter)
*** 1377,1384 ****
  	  record_one_conflict (regno);
  	}
      }
    /* Handle hardware regs (and pseudos allocated to hard regs).  */
!   else if (! fixed_regs[regno])
      {
        register int last = regno + HARD_REGNO_NREGS (regno, GET_MODE (reg));
        while (regno < last)
--- 1375,1386 ----
  	  record_one_conflict (regno);
  	}
      }
+ 
+   if (reg_renumber[regno] >= 0)
+     regno = reg_renumber[regno] /* + word */;
+ 
    /* Handle hardware regs (and pseudos allocated to hard regs).  */
!   if (regno < FIRST_PSEUDO_REGISTER && ! fixed_regs[regno])
      {
        register int last = regno + HARD_REGNO_NREGS (regno, GET_MODE (reg));
        while (regno < last)
*************** mark_reg_clobber (reg, setter)
*** 1420,1428 ****
  
    regno = REGNO (reg);
  
-   if (reg_renumber[regno] >= 0)
-     regno = reg_renumber[regno] /* + word */;
- 
    /* Either this is one of the max_allocno pseudo regs not allocated,
       or it is or has a hardware reg.  First handle the pseudo-regs.  */
    if (regno >= FIRST_PSEUDO_REGISTER)
--- 1422,1427 ----
*************** mark_reg_clobber (reg, setter)
*** 1433,1440 ****
  	  record_one_conflict (regno);
  	}
      }
    /* Handle hardware regs (and pseudos allocated to hard regs).  */
!   else if (! fixed_regs[regno])
      {
        register int last = regno + HARD_REGNO_NREGS (regno, GET_MODE (reg));
        while (regno < last)
--- 1432,1443 ----
  	  record_one_conflict (regno);
  	}
      }
+ 
+   if (reg_renumber[regno] >= 0)
+     regno = reg_renumber[regno] /* + word */;
+ 
    /* Handle hardware regs (and pseudos allocated to hard regs).  */
!   if (regno < FIRST_PSEUDO_REGISTER && ! fixed_regs[regno])
      {
        register int last = regno + HARD_REGNO_NREGS (regno, GET_MODE (reg));
        while (regno < last)
*************** mark_reg_conflicts (reg)
*** 1463,1471 ****
  
    regno = REGNO (reg);
  
-   if (reg_renumber[regno] >= 0)
-     regno = reg_renumber[regno];
- 
    /* Either this is one of the max_allocno pseudo regs not allocated,
       or it is or has a hardware reg.  First handle the pseudo-regs.  */
    if (regno >= FIRST_PSEUDO_REGISTER)
--- 1466,1471 ----
*************** mark_reg_conflicts (reg)
*** 1473,1480 ****
        if (reg_allocno[regno] >= 0)
  	record_one_conflict (regno);
      }
    /* Handle hardware regs (and pseudos allocated to hard regs).  */
!   else if (! fixed_regs[regno])
      {
        register int last = regno + HARD_REGNO_NREGS (regno, GET_MODE (reg));
        while (regno < last)
--- 1473,1484 ----
        if (reg_allocno[regno] >= 0)
  	record_one_conflict (regno);
      }
+ 
+   if (reg_renumber[regno] >= 0)
+     regno = reg_renumber[regno];
+ 
    /* Handle hardware regs (and pseudos allocated to hard regs).  */
!   if (regno < FIRST_PSEUDO_REGISTER && ! fixed_regs[regno])
      {
        register int last = regno + HARD_REGNO_NREGS (regno, GET_MODE (reg));
        while (regno < last)
*************** mark_reg_death (reg)
*** 1494,1503 ****
  {
    register int regno = REGNO (reg);
  
-   /* For pseudo reg, see if it has been assigned a hardware reg.  */
-   if (reg_renumber[regno] >= 0)
-     regno = reg_renumber[regno];
- 
    /* Either this is one of the max_allocno pseudo regs not allocated,
       or it is a hardware reg.  First handle the pseudo-regs.  */
    if (regno >= FIRST_PSEUDO_REGISTER)
--- 1498,1503 ----
*************** mark_reg_death (reg)
*** 1505,1512 ****
        if (reg_allocno[regno] >= 0)
  	CLEAR_ALLOCNO_LIVE (reg_allocno[regno]);
      }
    /* Handle hardware regs (and pseudos allocated to hard regs).  */
!   else if (! fixed_regs[regno])
      {
        /* Pseudo regs already assigned hardware regs are treated
  	 almost the same as explicit hardware regs.  */
--- 1505,1517 ----
        if (reg_allocno[regno] >= 0)
  	CLEAR_ALLOCNO_LIVE (reg_allocno[regno]);
      }
+ 
+   /* For pseudo reg, see if it has been assigned a hardware reg.  */
+   if (reg_renumber[regno] >= 0)
+     regno = reg_renumber[regno];
+ 
    /* Handle hardware regs (and pseudos allocated to hard regs).  */
!   if (regno < FIRST_PSEUDO_REGISTER && ! fixed_regs[regno])
      {
        /* Pseudo regs already assigned hardware regs are treated
  	 almost the same as explicit hardware regs.  */
*************** dump_conflicts (file)
*** 1661,1670 ****
  {
    register int i;
    register int has_preferences;
!   fprintf (file, ";; %d regs to allocate:", max_allocno);
    for (i = 0; i < max_allocno; i++)
      {
        int j;
        fprintf (file, " %d", allocno_reg[allocno_order[i]]);
        for (j = 0; j < max_regno; j++)
  	if (reg_allocno[j] == allocno_order[i]
--- 1666,1685 ----
  {
    register int i;
    register int has_preferences;
!   register int nregs;
!   nregs = 0;
!   for (i = 0; i < max_allocno; i++)
!     {
!       if (reg_renumber[allocno_reg[allocno_order[i]]] >= 0)
!         continue;
!       nregs++;
!     }
!   fprintf (file, ";; %d regs to allocate:", nregs);
    for (i = 0; i < max_allocno; i++)
      {
        int j;
+       if (reg_renumber[allocno_reg[allocno_order[i]]] >= 0)
+ 	continue;
        fprintf (file, " %d", allocno_reg[allocno_order[i]]);
        for (j = 0; j < max_regno; j++)
  	if (reg_allocno[j] == allocno_order[i]