diff mbox series

[2/9] Remove call_fixed_reg_set

Message ID mpt5zm03yjp.fsf@arm.com
State New
Headers show
Series Reduce the amount of global ABI state | expand

Commit Message

Richard Sandiford Sept. 10, 2019, 4:29 p.m. UTC 
On targets that use reload, call_fixed_reg_set is structurally:

  fixed_reg_set                           -- reginfo.c
  | (call_used_reg_set & ~have_save_mode) -- first loop in init_caller_save
  | ~have_save_insn                       -- final loop in init_caller_save

(where "have_save_mode" and "have_save_insn" are just my names).
But the final loop in init_caller_save does:

  for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
    for (j = 1; j <= MOVE_MAX_WORDS; j++)
      if (reg_save_code (i,regno_save_mode[i][j]) == -1)

This last condition ought to be true whenever:

  regno_save_mode[i][j] == VOIDmode

since either targetm.hard_regno_mode_ok (i, VOIDmode) should be
false or the VOIDmode save & restore shouldn't match any move insn.
And after the first loop, regno_save_mode[i][j] == VOIDmode whenever
!call_used_regs[i].  So the above is actually:

  fixed_reg_set
  | (call_used_reg_set & ~have_save_mode)
  | (~call_used_reg_set | ~have_save_insn)

which simplifies to:

  fixed_reg_set                        -- reginfo.c
  | ~have_save_mode                    -- first loop in init_caller_save
  | ~have_save_insn                    -- final loop in init_caller_save
  | ~call_used_reg_set                 -- final loop in init_caller_save

So:

  ~call_fixed_reg_set == (~fixed_reg_set
		          & have_save_mode
			  & have_save_insn
			  & call_used_reg_set)  [A]

All users have the form:

  (call_used_reg_set or some subset) & ~(call_fixed_reg_set | ...)

i.e.:

  (call_used_reg_set or some subset) & ~call_fixed_reg_set & ~(...)

We can therefore drop the "& call_used_reg_set" from [A], leaving:

  ~fixed_reg_set & have_save_mode & have_save_insn

This patch combines have_save_mode & have_save_insn into a single
condition "a save is possible", represented as savable_regs.
We can then substitute:

  ~call_fixed_reg_set --> ~fixed_reg_set & savable_regs
                          (registers we can actually save around calls)

The patch also sets regno_save_mode[i][j] for all registers,
in case non-default ABIs require a save when the default ABI
doesn't.  This ensures that savable_regs (like fixed_reg_set but
unlike call_fixed_reg_set) isn't affected by the ABI.  This only
becomes significant with later patches and at this point is just
a simplification.

Since init_caller_save is only called for reload targets,
the default assumption for LRA is that all registers are savable,
just like the default assumption before the patch was that
(~)call_fixed_reg_set == (~)fixed_reg_set.


2019-09-10  Richard Sandiford  <richard.sandiford@arm.com>

gcc/
	* hard-reg-set.h (target_hard_regs::x_call_fixed_reg_set): Delete.
	(target_hard_regs::x_savable_regs): New field.
	(call_fixed_reg_set): Delete.
	(savable_regs): New macro,
	* reginfo.c (globalize_reg): Don't set call_fixed_reg_set.
	(init_reg_sets_1): Likewise.  Initialize savable_regs.
	* caller-save.c (init_caller_save): Invoke HARD_REGNO_CALLER_SAVE_MODE
	for all registers.  Set savable_regs instead of call_fixed_reg_set.
	(setup_save_areas, save_call_clobbered_regs): Replace uses of
	~call_fixed_reg_set with ~fixed_reg_set & savable_regs.
	* config/sh/sh.c (output_stack_adjust): Likewise.

Comments

Jeff Law Sept. 10, 2019, 6:29 p.m. UTC | #1 
On 9/10/19 10:29 AM, Richard Sandiford wrote:
> On targets that use reload, call_fixed_reg_set is structurally:
> 
>   fixed_reg_set                           -- reginfo.c
>   | (call_used_reg_set & ~have_save_mode) -- first loop in init_caller_save
>   | ~have_save_insn                       -- final loop in init_caller_save
> 
> (where "have_save_mode" and "have_save_insn" are just my names).
> But the final loop in init_caller_save does:
> 
>   for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
>     for (j = 1; j <= MOVE_MAX_WORDS; j++)
>       if (reg_save_code (i,regno_save_mode[i][j]) == -1)
> 
> This last condition ought to be true whenever:
> 
>   regno_save_mode[i][j] == VOIDmode
> 
> since either targetm.hard_regno_mode_ok (i, VOIDmode) should be
> false or the VOIDmode save & restore shouldn't match any move insn.
> And after the first loop, regno_save_mode[i][j] == VOIDmode whenever
> !call_used_regs[i].  So the above is actually:
> 
>   fixed_reg_set
>   | (call_used_reg_set & ~have_save_mode)
>   | (~call_used_reg_set | ~have_save_insn)
> 
> which simplifies to:
> 
>   fixed_reg_set                        -- reginfo.c
>   | ~have_save_mode                    -- first loop in init_caller_save
>   | ~have_save_insn                    -- final loop in init_caller_save
>   | ~call_used_reg_set                 -- final loop in init_caller_save
> 
> So:
> 
>   ~call_fixed_reg_set == (~fixed_reg_set
> 		          & have_save_mode
> 			  & have_save_insn
> 			  & call_used_reg_set)  [A]
> 
> All users have the form:
> 
>   (call_used_reg_set or some subset) & ~(call_fixed_reg_set | ...)
> 
> i.e.:
> 
>   (call_used_reg_set or some subset) & ~call_fixed_reg_set & ~(...)
> 
> We can therefore drop the "& call_used_reg_set" from [A], leaving:
> 
>   ~fixed_reg_set & have_save_mode & have_save_insn
> 
> This patch combines have_save_mode & have_save_insn into a single
> condition "a save is possible", represented as savable_regs.
> We can then substitute:
> 
>   ~call_fixed_reg_set --> ~fixed_reg_set & savable_regs
>                           (registers we can actually save around calls)
> 
> The patch also sets regno_save_mode[i][j] for all registers,
> in case non-default ABIs require a save when the default ABI
> doesn't.  This ensures that savable_regs (like fixed_reg_set but
> unlike call_fixed_reg_set) isn't affected by the ABI.  This only
> becomes significant with later patches and at this point is just
> a simplification.
> 
> Since init_caller_save is only called for reload targets,
> the default assumption for LRA is that all registers are savable,
> just like the default assumption before the patch was that
> (~)call_fixed_reg_set == (~)fixed_reg_set.
> 
> 
> 2019-09-10  Richard Sandiford  <richard.sandiford@arm.com>
> 
> gcc/
> 	* hard-reg-set.h (target_hard_regs::x_call_fixed_reg_set): Delete.
> 	(target_hard_regs::x_savable_regs): New field.
> 	(call_fixed_reg_set): Delete.
> 	(savable_regs): New macro,
> 	* reginfo.c (globalize_reg): Don't set call_fixed_reg_set.
> 	(init_reg_sets_1): Likewise.  Initialize savable_regs.
> 	* caller-save.c (init_caller_save): Invoke HARD_REGNO_CALLER_SAVE_MODE
> 	for all registers.  Set savable_regs instead of call_fixed_reg_set.
> 	(setup_save_areas, save_call_clobbered_regs): Replace uses of
> 	~call_fixed_reg_set with ~fixed_reg_set & savable_regs.
> 	* config/sh/sh.c (output_stack_adjust): Likewise.
OK.

Given that the major targets at this point are LRA and caller-saves is
strictly for reload and of marginal value unless the target has heavily
used register classes with no callee saved regs, one could make an
argument that we should just drop caller-save.  That's clearly an
independent change, but one that I'd seriously consider.

jeff
diff mbox series

Patch

Index: gcc/hard-reg-set.h
===================================================================
--- gcc/hard-reg-set.h	2019-09-09 19:01:40.371078272 +0100
+++ gcc/hard-reg-set.h	2019-09-10 17:22:37.774467353 +0100
@@ -400,11 +400,15 @@  struct target_hard_regs {
   /* The same info as a HARD_REG_SET.  */
   HARD_REG_SET x_call_used_reg_set;
 
-  /* Contains registers that are fixed use -- i.e. in fixed_reg_set -- or
-     a function value return register or TARGET_STRUCT_VALUE_RTX or
-     STATIC_CHAIN_REGNUM.  These are the registers that cannot hold quantities
-     across calls even if we are willing to save and restore them.  */
-  HARD_REG_SET x_call_fixed_reg_set;
+  /* For targets that use reload rather than LRA, this is the set
+     of registers that we are able to save and restore around calls
+     (i.e. those for which we know a suitable mode and set of
+     load/store instructions exist).  For LRA targets it contains
+     all registers.
+
+     This is legacy information and should be removed if all targets
+     switch to LRA.  */
+  HARD_REG_SET x_savable_regs;
 
   /* Contains registers that are fixed use -- i.e. in fixed_reg_set -- but
      only if they are not merely part of that set because they are global
@@ -482,8 +486,8 @@  #define call_really_used_regs \
   (this_target_hard_regs->x_call_really_used_regs)
 #define call_used_reg_set \
   (this_target_hard_regs->x_call_used_reg_set)
-#define call_fixed_reg_set \
-  (this_target_hard_regs->x_call_fixed_reg_set)
+#define savable_regs \
+  (this_target_hard_regs->x_savable_regs)
 #define regs_invalidated_by_call \
   (this_target_hard_regs->x_regs_invalidated_by_call)
 #define no_caller_save_reg_set \
Index: gcc/reginfo.c
===================================================================
--- gcc/reginfo.c	2019-09-09 19:01:48.427021398 +0100
+++ gcc/reginfo.c	2019-09-10 17:22:37.774467353 +0100
@@ -351,7 +351,6 @@  init_reg_sets_1 (void)
 
   CLEAR_HARD_REG_SET (fixed_reg_set);
   CLEAR_HARD_REG_SET (call_used_reg_set);
-  CLEAR_HARD_REG_SET (call_fixed_reg_set);
   CLEAR_HARD_REG_SET (regs_invalidated_by_call);
 
   operand_reg_set &= accessible_reg_set;
@@ -417,7 +416,7 @@  init_reg_sets_1 (void)
 	SET_HARD_REG_BIT (regs_invalidated_by_call, i);
     }
 
-  call_fixed_reg_set = fixed_reg_set;
+  SET_HARD_REG_SET (savable_regs);
   fixed_nonglobal_reg_set = fixed_reg_set;
 
   /* Preserve global registers if called more than once.  */
@@ -428,7 +427,6 @@  init_reg_sets_1 (void)
 	  fixed_regs[i] = call_used_regs[i] = 1;
 	  SET_HARD_REG_BIT (fixed_reg_set, i);
 	  SET_HARD_REG_BIT (call_used_reg_set, i);
-	  SET_HARD_REG_BIT (call_fixed_reg_set, i);
 	}
     }
 
@@ -782,7 +780,6 @@  globalize_reg (tree decl, int i)
 
   SET_HARD_REG_BIT (fixed_reg_set, i);
   SET_HARD_REG_BIT (call_used_reg_set, i);
-  SET_HARD_REG_BIT (call_fixed_reg_set, i);
 
   reinit_regs ();
 }
Index: gcc/caller-save.c
===================================================================
--- gcc/caller-save.c	2019-09-09 18:59:20.840063365 +0100
+++ gcc/caller-save.c	2019-09-10 17:22:37.770467379 +0100
@@ -198,23 +198,12 @@  init_caller_save (void)
      we can't have the register live over calls.  */
 
   for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
-    {
-      if (call_used_regs[i]
-          && !TEST_HARD_REG_BIT (call_fixed_reg_set, i))
-	{
-	  for (j = 1; j <= MOVE_MAX_WORDS; j++)
-	    {
-	      regno_save_mode[i][j] = HARD_REGNO_CALLER_SAVE_MODE (i, j,
-								   VOIDmode);
-	      if (regno_save_mode[i][j] == VOIDmode && j == 1)
-		{
-		  SET_HARD_REG_BIT (call_fixed_reg_set, i);
-		}
-	    }
-	}
-      else
-	regno_save_mode[i][1] = VOIDmode;
-    }
+    for (j = 1; j <= MOVE_MAX_WORDS; j++)
+      {
+	regno_save_mode[i][j] = HARD_REGNO_CALLER_SAVE_MODE (i, j, VOIDmode);
+	if (regno_save_mode[i][j] == VOIDmode && j == 1)
+	  CLEAR_HARD_REG_BIT (savable_regs, i);
+      }
 
   /* The following code tries to approximate the conditions under which
      we can easily save and restore a register without scratch registers or
@@ -276,7 +265,7 @@  init_caller_save (void)
 	  regno_save_mode[i][j] = VOIDmode;
 	  if (j == 1)
 	    {
-	      SET_HARD_REG_BIT (call_fixed_reg_set, i);
+	      CLEAR_HARD_REG_BIT (savable_regs, i);
 	      if (call_used_regs[i])
 		SET_HARD_REG_BIT (no_caller_save_reg_set, i);
 	    }
@@ -455,8 +444,8 @@  setup_save_areas (void)
       if (SIBLING_CALL_P (insn) && crtl->return_rtx)
 	mark_set_regs (crtl->return_rtx, NULL_RTX, &this_insn_sets);
 
-      used_regs &= ~(call_fixed_reg_set | this_insn_sets);
-      hard_regs_to_save &= used_regs;
+      used_regs &= ~(fixed_reg_set | this_insn_sets);
+      hard_regs_to_save &= used_regs & savable_regs;
       for (regno = 0; regno < FIRST_PSEUDO_REGISTER; regno++)
 	if (TEST_HARD_REG_BIT (hard_regs_to_save, regno))
 	  {
@@ -539,8 +528,8 @@  setup_save_areas (void)
 	  if (SIBLING_CALL_P (insn) && crtl->return_rtx)
 	    mark_set_regs (crtl->return_rtx, NULL_RTX, &this_insn_sets);
 
-	  used_regs &= ~(call_fixed_reg_set | this_insn_sets);
-	  hard_regs_to_save &= used_regs;
+	  used_regs &= ~(fixed_reg_set | this_insn_sets);
+	  hard_regs_to_save &= used_regs & savable_regs;
 	  for (regno = 0; regno < FIRST_PSEUDO_REGISTER; regno++)
 	    if (TEST_HARD_REG_BIT (hard_regs_to_save, regno))
 	      {
@@ -850,9 +839,10 @@  save_call_clobbered_regs (void)
 	      note_stores (insn, mark_set_regs, &this_insn_sets);
 
 	      /* Compute which hard regs must be saved before this call.  */
-	      hard_regs_to_save &= ~(call_fixed_reg_set
+	      hard_regs_to_save &= ~(fixed_reg_set
 				     | this_insn_sets
 				     | hard_regs_saved);
+	      hard_regs_to_save &= savable_regs;
 	      get_call_reg_set_usage (insn, &call_def_reg_set,
 				      call_used_reg_set);
 	      hard_regs_to_save &= call_def_reg_set;
Index: gcc/config/sh/sh.c
===================================================================
--- gcc/config/sh/sh.c	2019-09-09 19:01:48.419021455 +0100
+++ gcc/config/sh/sh.c	2019-09-10 17:22:37.774467353 +0100
@@ -6707,7 +6707,9 @@  output_stack_adjust (int size, rtx reg,
 	    temp = -1;
 	  if (temp < 0 && ! current_function_interrupt && epilogue_p >= 0)
 	    {
-	      HARD_REG_SET temps = call_used_reg_set & ~call_fixed_reg_set;
+	      HARD_REG_SET temps = (call_used_reg_set
+				    & ~fixed_reg_set
+				    & savable_regs);
 	      if (epilogue_p > 0)
 		{
 		  int nreg = 0;