PATCH: Fix PR 33211

PATCH: Fix PR 33211

[Sandra Loosemore]

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

This bug with -mfixed-range on spu was caused by my earlier patch to 
refactor back end initialization code into run-once versus 
possibly-run-multiple-times pieces.  The bug was caused by run-once 
initialization in target-specific code modifying variables that are now 
overwritten during the run-later pieces.  While the bug was reported on 
spu, after it was brought to my attention I poked around and saw that 
the ia64 and pa back ends do exactly the same thing.

I've fixed the problem by reverting pieces of my previous patch and 
reimplementing the way it keeps track of the initial register set 
information, so that the target-specific code can keep doing exactly 
what it's been doing all along.  (It now saves a copy of the data after 
run-once initialization instead of expecting the run-once actions to 
operate directly on the duplicate set.)

I tested this patch by doing a full bootstrap and regression test on 
i686-pc-linux-gnu and MIPS testing in conjunction with my mips16 
function attributes patch (both with and without -mflip-mips16, so I 
know that doing the reinitialization multiple times does, in fact, work 
correctly).  So far I've done a bootstrap and spot testing on ia64, to 
verify that -mfixed-range really was broken, and really is fixed by this 
patch; I'm running regression tests now and they should be done by 
morning.  Unfortunately it does not seem like we have either a spu or pa 
machine set up for testing here.  Perhaps someone who does have a spu 
test setup in place can check to verify that this patch fixes the 
reported regression?

-Sandra


[-- Attachment #2: 33-register-init.log --]
[-- Type: text/x-log, Size: 688 bytes --]

2007-08-28  Sandra Loosemore  <sandra@codesourcery.com>

	gcc/
	* regclass.c (initial_fixed_regs): Revert previous change and make
	it const again.
	(initial_call_used_regs): Likewise.
	(initial_call_really_used_regs): Delete, reverting previous addition.
	(initial_reg_names): Likewise.
	(init_reg_sets): Revert previous change.
	(saved_fixed_regs): New.
	(saved_call_used_regs): New.
	(saved_call_really_used_regs): New.
	(saved_reg_names): New.
	(save_register_info): New.
	(restore_register_info): New.
	(init_reg_sets_1): Replace reset of register info with call to
	restore_register_info.
	* rtl.h (save_register_info): Declare.
	* toplev.c (backend_init): Call save_register_info.

[-- Attachment #3: 33-register-init.patch --]
[-- Type: text/x-patch, Size: 8455 bytes --]

Index: gcc/regclass.c
===================================================================
*** gcc/regclass.c	(revision 127836)
--- gcc/regclass.c	(working copy)
*************** HARD_REG_SET fixed_reg_set;
*** 81,87 ****
  
  /* Data for initializing the above.  */
  
! static char initial_fixed_regs[] = FIXED_REGISTERS;
  
  /* Indexed by hard register number, contains 1 for registers
     that are fixed use or are clobbered by function calls.
--- 81,87 ----
  
  /* Data for initializing the above.  */
  
! static const char initial_fixed_regs[] = FIXED_REGISTERS;
  
  /* Indexed by hard register number, contains 1 for registers
     that are fixed use or are clobbered by function calls.
*************** HARD_REG_SET losing_caller_save_reg_set;
*** 100,106 ****
  
  /* Data for initializing the above.  */
  
! static char initial_call_used_regs[] = CALL_USED_REGISTERS;
  
  /* This is much like call_used_regs, except it doesn't have to
     be a superset of FIXED_REGISTERS. This vector indicates
--- 100,106 ----
  
  /* Data for initializing the above.  */
  
! static const char initial_call_used_regs[] = CALL_USED_REGISTERS;
  
  /* This is much like call_used_regs, except it doesn't have to
     be a superset of FIXED_REGISTERS. This vector indicates
*************** static char initial_call_used_regs[] = C
*** 108,115 ****
     regs_invalidated_by_call.  */
  
  #ifdef CALL_REALLY_USED_REGISTERS
! static char initial_call_really_used_regs[] = CALL_REALLY_USED_REGISTERS;
! char call_really_used_regs[FIRST_PSEUDO_REGISTER];
  #endif
  
  #ifdef CALL_REALLY_USED_REGISTERS
--- 108,114 ----
     regs_invalidated_by_call.  */
  
  #ifdef CALL_REALLY_USED_REGISTERS
! char call_really_used_regs[] = CALL_REALLY_USED_REGISTERS;
  #endif
  
  #ifdef CALL_REALLY_USED_REGISTERS
*************** enum reg_class reg_class_superunion[N_RE
*** 193,203 ****
  
  /* Array containing all of the register names.  */
  
! const char * reg_names[FIRST_PSEUDO_REGISTER];
! 
! /* Data for initializing the above.  */
! 
! const char * initial_reg_names[] = REGISTER_NAMES;
  
  /* Array containing all of the register class names.  */
  
--- 191,197 ----
  
  /* Array containing all of the register names.  */
  
! const char * reg_names[] = REGISTER_NAMES;
  
  /* Array containing all of the register class names.  */
  
*************** init_reg_sets (void)
*** 306,317 ****
  	  SET_HARD_REG_BIT (reg_class_contents[i], j);
      }
  
!   memset (global_regs, 0, sizeof global_regs);
  
!   /* Processing of command-line options like -ffixed needs to know the
!      initial set of register names, so initialize that now.  */
!   gcc_assert (sizeof reg_names == sizeof initial_reg_names);
!   memcpy (reg_names, initial_reg_names, sizeof reg_names);
  }
  
  /* Initialize may_move_cost and friends for mode M.  */
--- 300,313 ----
  	  SET_HARD_REG_BIT (reg_class_contents[i], j);
      }
  
!   /* Sanity check: make sure the target macros FIXED_REGISTERS and
!      CALL_USED_REGISTERS had the right number of initializers.  */
!   gcc_assert (sizeof fixed_regs == sizeof initial_fixed_regs);
!   gcc_assert (sizeof call_used_regs == sizeof initial_call_used_regs);
  
!   memcpy (fixed_regs, initial_fixed_regs, sizeof fixed_regs);
!   memcpy (call_used_regs, initial_call_used_regs, sizeof call_used_regs);
!   memset (global_regs, 0, sizeof global_regs);
  }
  
  /* Initialize may_move_cost and friends for mode M.  */
*************** init_move_cost (enum machine_mode m)
*** 403,436 ****
  	}
  }
  
! /* After switches have been processed, which perhaps alter
!    `fixed_regs' and `call_used_regs', convert them to HARD_REG_SETs.  */
  
! static void
! init_reg_sets_1 (void)
! {
!   unsigned int i, j;
!   unsigned int /* enum machine_mode */ m;
  
    /* Sanity check:  make sure the target macros FIXED_REGISTERS and
       CALL_USED_REGISTERS had the right number of initializers.  */
!   gcc_assert (sizeof fixed_regs == sizeof initial_fixed_regs);
!   gcc_assert (sizeof call_used_regs == sizeof initial_call_used_regs);
! 
!   memcpy (fixed_regs, initial_fixed_regs, sizeof fixed_regs);
!   memcpy (call_used_regs, initial_call_used_regs, sizeof call_used_regs);
  
    /* Likewise for call_really_used_regs.  */
  #ifdef CALL_REALLY_USED_REGISTERS
    gcc_assert (sizeof call_really_used_regs
! 	      == sizeof initial_call_really_used_regs);
!   memcpy (call_really_used_regs, initial_call_really_used_regs,
  	  sizeof call_really_used_regs);
  #endif
  
    /* And similarly for reg_names.  */
!   gcc_assert (sizeof reg_names == sizeof initial_reg_names);
!   memcpy (reg_names, initial_reg_names, sizeof reg_names);
  
  #ifdef REG_ALLOC_ORDER
    for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
--- 399,466 ----
  	}
  }
  
! /* We need to save copies of some of the register information which
!    can be munged by command-line switches so we can restore it during
!    subsequent back-end reinitialization.  */
  
! static char saved_fixed_regs[FIRST_PSEUDO_REGISTER];
! static char saved_call_used_regs[FIRST_PSEUDO_REGISTER];
! #ifdef CALL_REALLY_USED_REGISTERS
! static char saved_call_really_used_regs[FIRST_PSEUDO_REGISTER];
! #endif
! static const char *saved_reg_names[FIRST_PSEUDO_REGISTER];
! 
! /* Save the register information.  */
  
+ void
+ save_register_info (void)
+ {
    /* Sanity check:  make sure the target macros FIXED_REGISTERS and
       CALL_USED_REGISTERS had the right number of initializers.  */
!   gcc_assert (sizeof fixed_regs == sizeof saved_fixed_regs);
!   gcc_assert (sizeof call_used_regs == sizeof saved_call_used_regs);
!   memcpy (saved_fixed_regs, fixed_regs, sizeof fixed_regs);
!   memcpy (saved_call_used_regs, call_used_regs, sizeof call_used_regs);
  
    /* Likewise for call_really_used_regs.  */
  #ifdef CALL_REALLY_USED_REGISTERS
    gcc_assert (sizeof call_really_used_regs
! 	      == sizeof saved_call_really_used_regs);
!   memcpy (saved_call_really_used_regs, call_really_used_regs,
  	  sizeof call_really_used_regs);
  #endif
  
    /* And similarly for reg_names.  */
!   gcc_assert (sizeof reg_names == sizeof saved_reg_names);
!   memcpy (saved_reg_names, reg_names, sizeof reg_names);
! }
! 
! /* Restore the register information.  */
! 
! static void
! restore_register_info (void)
! {
!   memcpy (fixed_regs, saved_fixed_regs, sizeof fixed_regs);
!   memcpy (call_used_regs, saved_call_used_regs, sizeof call_used_regs);
! 
! #ifdef CALL_REALLY_USED_REGISTERS
!   memcpy (call_really_used_regs, saved_call_really_used_regs,
! 	  sizeof call_really_used_regs);
! #endif
! 
!   memcpy (reg_names, saved_reg_names, sizeof reg_names);
! }
! 
! /* After switches have been processed, which perhaps alter
!    `fixed_regs' and `call_used_regs', convert them to HARD_REG_SETs.  */
! 
! static void
! init_reg_sets_1 (void)
! {
!   unsigned int i, j;
!   unsigned int /* enum machine_mode */ m;
! 
!   restore_register_info ();
  
  #ifdef REG_ALLOC_ORDER
    for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
*************** fix_register (const char *name, int fixe
*** 846,856 ****
  	}
        else
  	{
! 	  initial_fixed_regs[i] = fixed;
! 	  initial_call_used_regs[i] = call_used;
  #ifdef CALL_REALLY_USED_REGISTERS
  	  if (fixed == 0)
! 	    initial_call_really_used_regs[i] = call_used;
  #endif
  	}
      }
--- 876,886 ----
  	}
        else
  	{
! 	  fixed_regs[i] = fixed;
! 	  call_used_regs[i] = call_used;
  #ifdef CALL_REALLY_USED_REGISTERS
  	  if (fixed == 0)
! 	    call_really_used_regs[i] = call_used;
  #endif
  	}
      }
Index: gcc/rtl.h
===================================================================
*** gcc/rtl.h	(revision 127835)
--- gcc/rtl.h	(working copy)
*************** extern void globalize_reg (int);
*** 2190,2195 ****
--- 2190,2196 ----
  extern void init_reg_modes_target (void);
  extern void init_regs (void);
  extern void init_fake_stack_mems (void);
+ extern void save_register_info (void);
  extern void init_reg_sets (void);
  extern void regclass (rtx, int);
  extern void reg_scan (rtx, unsigned int);
Index: gcc/toplev.c
===================================================================
*** gcc/toplev.c	(revision 127835)
--- gcc/toplev.c	(working copy)
*************** backend_init (void)
*** 2064,2069 ****
--- 2064,2070 ----
    init_rtlanal ();
    init_inline_once ();
    init_varasm_once ();
+   save_register_info ();
  
    /* Initialize the target-specific back end pieces.  */
    backend_init_target ();

Re: PATCH: Fix PR 33211

[Andrew Pinski]

On 8/28/07, Sandra Loosemore <sandra@codesourcery.com> wrote:
>		33-register-init.log

Can I make one suggestion, make the changelog message inline to the
email so it is easier to read them?

Thanks,
Andrew Pinski

Re: PATCH: Fix PR 33211

[Andrew Pinski]

On 8/28/07, Sandra Loosemore <sandra@codesourcery.com> wrote:
> Perhaps someone who does have a spu
> test setup in place can check to verify that this patch fixes the
> reported regression?

Yes this fixes the bug for spu-elf.

Thanks,
Andrew Pinski

Re: PATCH: Fix PR 33211

[Mark Mitchell]

Andrew Pinski wrote:
> On 8/28/07, Sandra Loosemore <sandra@codesourcery.com> wrote:
>> Perhaps someone who does have a spu
>> test setup in place can check to verify that this patch fixes the
>> reported regression?
> 
> Yes this fixes the bug for spu-elf.

Sandra, please go ahead and apply this, given that it seems to make
definitive forward progress.

Thanks,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: PATCH: Fix PR 33211

[Sandra Loosemore]

Mark Mitchell wrote:
> Andrew Pinski wrote:
>> On 8/28/07, Sandra Loosemore <sandra@codesourcery.com> wrote:
>>> Perhaps someone who does have a spu
>>> test setup in place can check to verify that this patch fixes the
>>> reported regression?
>> Yes this fixes the bug for spu-elf.
> 
> Sandra, please go ahead and apply this, given that it seems to make
> definitive forward progress.

OK, committed.

-Sandra