diff mbox

Remove redundant test for global_regs

Message ID 55E03337.7020205@post.ru
State New
Headers show

Commit Message

Anatoly Sokolov Aug. 28, 2015, 10:08 a.m. UTC
Hi.

The fixed_reg_set contain all fixed and global registers. This patch change code 
"fixed_regs[r] || global_regs[r]" with "TEST_HARD_REG_BIT (fixed_reg_set, r)".

Bootstrapped and reg-tested on x86_64-unknown-linux-gnu and
powerpc64le-unknown-linux-gnu.

OK for trunk?

2015-08-24  Anatoly Sokolov  <aesok@post.ru>

	* cse.c (FIXED_REGNO_P): Don't check global_regs. Use fixed_reg_set
	instead of fixed_regs.
	* df-problems.c (can_move_insns_across): Ditto.
	* postreload.c (reload_combine_recognize_pattern): Ditto.
	* recog.c (peep2_find_free_register): Ditto.
	* regcprop.c (copy_value): Ditto.
	* regrename.c (check_new_reg_p, rename_chains): Ditto.
	* sel-sched.c (init_regs_for_mode, mark_unavailable_hard_regs): Ditto.



Anatoly Sokolov.

Comments

Georg-Johann Lay Sept. 2, 2015, 11:02 p.m. UTC | #1
Anatoliy Sokolov schrieb:
> Hi.
> 
> The fixed_reg_set contain all fixed and global registers. This patch 
> change code "fixed_regs[r] || global_regs[r]" with "TEST_HARD_REG_BIT 
> (fixed_reg_set, r)".

Even though technically a no-op change

   TEST_HARD_REG_BIT (fixed_reg_set, r)

appears to be a test for r being a fixed register and not for being 
fixed-or-global register, so the old style

   fixed_regs[r] || global_regs[r]

seems to be less error prone.

Johann

> Bootstrapped and reg-tested on x86_64-unknown-linux-gnu and
> powerpc64le-unknown-linux-gnu.
> 
> OK for trunk?
> 
> 2015-08-24  Anatoly Sokolov  <aesok@post.ru>
> 
>     * cse.c (FIXED_REGNO_P): Don't check global_regs. Use fixed_reg_set
>     instead of fixed_regs.
>     * df-problems.c (can_move_insns_across): Ditto.
>     * postreload.c (reload_combine_recognize_pattern): Ditto.
>     * recog.c (peep2_find_free_register): Ditto.
>     * regcprop.c (copy_value): Ditto.
>     * regrename.c (check_new_reg_p, rename_chains): Ditto.
>     * sel-sched.c (init_regs_for_mode, mark_unavailable_hard_regs): Ditto.
> 
> Index: gcc/cse.c
> ===================================================================
> --- gcc/cse.c    (revision 226953)
> +++ gcc/cse.c    (working copy)
> @@ -463,7 +463,7 @@
>     A reg wins if it is either the frame pointer or designated as 
> fixed.  */
>  #define FIXED_REGNO_P(N)  \
>    ((N) == FRAME_POINTER_REGNUM || (N) == HARD_FRAME_POINTER_REGNUM \
> -   || fixed_regs[N] || global_regs[N])
> +   || TEST_HARD_REG_BIT (fixed_reg_set, N))
> 
>  /* Compute cost of X, as stored in the `cost' field of a table_elt.  Fixed
>     hard registers and pointers into the frame are the cheapest with a cost
> Index: gcc/df-problems.c
> ===================================================================
> --- gcc/df-problems.c    (revision 226953)
> +++ gcc/df-problems.c    (working copy)
> @@ -3871,8 +3871,7 @@
>        EXECUTE_IF_SET_IN_BITMAP (merge_set, 0, i, bi)
>      {
>        if (i < FIRST_PSEUDO_REGISTER
> -          && ! fixed_regs[i]
> -          && ! global_regs[i])
> +          && ! TEST_HARD_REG_BIT (fixed_reg_set, i))
>          {
>            fail = 1;
>            break;
> Index: gcc/postreload.c
> ===================================================================
> --- gcc/postreload.c    (revision 226953)
> +++ gcc/postreload.c    (working copy)
> @@ -1144,7 +1144,7 @@
>            && reg_state[i].store_ruid <= reg_state[regno].use_ruid
>            && (call_used_regs[i] || df_regs_ever_live_p (i))
>            && (!frame_pointer_needed || i != HARD_FRAME_POINTER_REGNUM)
> -          && !fixed_regs[i] && !global_regs[i]
> +          && !TEST_HARD_REG_BIT (fixed_reg_set, i)
>            && hard_regno_nregs[i][GET_MODE (reg)] == 1
>            && targetm.hard_regno_scratch_ok (i))
>          {
> Index: gcc/recog.c
> ===================================================================
> --- gcc/recog.c    (revision 226953)
> +++ gcc/recog.c    (working copy)
> @@ -3162,17 +3162,11 @@
>        for (j = 0; success && j < hard_regno_nregs[regno][mode]; j++)
>      {
>        /* Don't allocate fixed registers.  */
> -      if (fixed_regs[regno + j])
> +      if (TEST_HARD_REG_BIT (fixed_reg_set, regno + j))
>          {
>            success = 0;
>            break;
>          }
> -      /* Don't allocate global registers.  */
> -      if (global_regs[regno + j])
> -        {
> -          success = 0;
> -          break;
> -        }
>        /* Make sure the register is of the right class.  */
>        if (! TEST_HARD_REG_BIT (reg_class_contents[cl], regno + j))
>          {
> Index: gcc/regcprop.c
> ===================================================================
> --- gcc/regcprop.c    (revision 226953)
> +++ gcc/regcprop.c    (working copy)
> @@ -315,7 +315,7 @@
>    /* Do not propagate copies to fixed or global registers, patterns
>       can be relying to see particular fixed register or users can
>       expect the chosen global register in asm.  */
> -  if (fixed_regs[dr] || global_regs[dr])
> +  if (TEST_HARD_REG_BIT (fixed_reg_set, dr))
>      return;
> 
>    /* If SRC and DEST overlap, don't record anything.  */
> Index: gcc/regrename.c
> ===================================================================
> --- gcc/regrename.c    (revision 226953)
> +++ gcc/regrename.c    (working copy)
> @@ -311,8 +311,7 @@
> 
>    for (i = nregs - 1; i >= 0; --i)
>      if (TEST_HARD_REG_BIT (this_unavailable, new_reg + i)
> -    || fixed_regs[new_reg + i]
> -    || global_regs[new_reg + i]
> +    || TEST_HARD_REG_BIT (fixed_reg_set, new_reg + i)
>      /* Can't use regs which aren't saved by the prologue.  */
>      || (! df_regs_ever_live_p (new_reg + i)
>          && ! call_used_regs[new_reg + i])
> @@ -440,7 +439,7 @@
>        if (this_head->cannot_rename)
>      continue;
> 
> -      if (fixed_regs[reg] || global_regs[reg]
> +      if (TEST_HARD_REG_BIT (fixed_reg_set, reg)
>        || (!HARD_FRAME_POINTER_IS_FRAME_POINTER && frame_pointer_needed
>            && reg == HARD_FRAME_POINTER_REGNUM)
>        || (HARD_FRAME_POINTER_REGNUM && frame_pointer_needed
> Index: gcc/sel-sched.c
> ===================================================================
> --- gcc/sel-sched.c    (revision 226953)
> +++ gcc/sel-sched.c    (working copy)
> @@ -1089,8 +1089,7 @@
>        nregs = hard_regno_nregs[cur_reg][mode];
> 
>        for (i = nregs - 1; i >= 0; --i)
> -        if (fixed_regs[cur_reg + i]
> -                || global_regs[cur_reg + i]
> +        if (TEST_HARD_REG_BIT (fixed_reg_set, cur_reg + i)
>              /* Can't use regs which aren't saved by
>                 the prologue.  */
>              || !TEST_HARD_REG_BIT (sel_hrd.regs_ever_used, cur_reg + i)
> @@ -1189,8 +1188,7 @@
> 
>    /* Stop if the original register is one of the fixed_regs, 
> global_regs or
>       frame pointer, or we could not discover its class.  */
> -  if (fixed_regs[regno]
> -      || global_regs[regno]
> +  if (TEST_HARD_REG_BIT (fixed_reg_set, regno)
>        || (!HARD_FRAME_POINTER_IS_FRAME_POINTER && frame_pointer_needed
>        && regno == HARD_FRAME_POINTER_REGNUM)
>        || (HARD_FRAME_POINTER_REGNUM && frame_pointer_needed
> 
> 
> Anatoly Sokolov.
>
Jeff Law Sept. 4, 2015, 8:22 p.m. UTC | #2
On 09/02/2015 05:02 PM, Georg-Johann Lay wrote:
> Anatoliy Sokolov schrieb:
>> Hi.
>>
>> The fixed_reg_set contain all fixed and global registers. This patch
>> change code "fixed_regs[r] || global_regs[r]" with "TEST_HARD_REG_BIT
>> (fixed_reg_set, r)".
>
> Even though technically a no-op change
>
>    TEST_HARD_REG_BIT (fixed_reg_set, r)
>
> appears to be a test for r being a fixed register and not for being
> fixed-or-global register, so the old style
>
>    fixed_regs[r] || global_regs[r]
>
> seems to be less error prone.
Perhaps, but it's always been the case that a global register must 
always be considered fixed.   It'd be fairly difficult to implement 
global registers with the current semantics without those registers 
being fixed.

This invariant is currently enforced by globalize_reg.  I wouldn't be at 
all surprised if there are places that are currently just testing 
fixed_regs.




>
> Johann
>
>> Bootstrapped and reg-tested on x86_64-unknown-linux-gnu and
>> powerpc64le-unknown-linux-gnu.
>>
>> OK for trunk?
>>
>> 2015-08-24  Anatoly Sokolov  <aesok@post.ru>
>>
>>     * cse.c (FIXED_REGNO_P): Don't check global_regs. Use fixed_reg_set
>>     instead of fixed_regs.
>>     * df-problems.c (can_move_insns_across): Ditto.
>>     * postreload.c (reload_combine_recognize_pattern): Ditto.
>>     * recog.c (peep2_find_free_register): Ditto.
>>     * regcprop.c (copy_value): Ditto.
>>     * regrename.c (check_new_reg_p, rename_chains): Ditto.
>>     * sel-sched.c (init_regs_for_mode, mark_unavailable_hard_regs):
Perhaps a better choice would be to use a macro that implies the 
register is neither fixed nor global, but only checks if its fixed (with 
an appropriate comment indicating why this is safe).  THen in 
globalize_reg reference add a comment indicating the macro must be 
changed if we ever want to support global registers that are not fixed.


>> Ditto.
>>
>> Index: gcc/cse.c
>> ===================================================================
>> --- gcc/cse.c    (revision 226953)
>> +++ gcc/cse.c    (working copy)
>> @@ -463,7 +463,7 @@
>>     A reg wins if it is either the frame pointer or designated as
>> fixed.  */
>>  #define FIXED_REGNO_P(N)  \
>>    ((N) == FRAME_POINTER_REGNUM || (N) == HARD_FRAME_POINTER_REGNUM \
>> -   || fixed_regs[N] || global_regs[N])
>> +   || TEST_HARD_REG_BIT (fixed_reg_set, N))
So why not continue to test fixed_regs here (ie, just drop the 
global_regs test)?  It's a single memory reference and a test against zero.

Using TEST_HARD_REG_BIT likely still hits memory, but then on many 
architectures you're then going to have to do masking/shifting to get 
the bit you want to look at.  That seems to me like a step backwards.

Jeff
Anatoly Sokolov Sept. 8, 2015, midnight UTC | #3
----- Original Message ----- 
From: "Jeff Law" <law@redhat.com>
Sent: Friday, September 04, 2015 11:22 PM

Hello.


>>>
>>> Index: gcc/cse.c
>>> ===================================================================
>>> --- gcc/cse.c (revision 226953)
>>> +++ gcc/cse.c (working copy)
>>> @@ -463,7 +463,7 @@
>>> A reg wins if it is either the frame pointer or designated as
>>> fixed. */
>>> #define FIXED_REGNO_P(N) \
>>> ((N) == FRAME_POINTER_REGNUM || (N) == HARD_FRAME_POINTER_REGNUM \
>>> - || fixed_regs[N] || global_regs[N])
>>> + || TEST_HARD_REG_BIT (fixed_reg_set, N))
> So why not continue to test fixed_regs here (ie, just drop the global_regs 
> test)? It's a single memory reference and a test against zero.
>
> Using TEST_HARD_REG_BIT likely still hits memory, but then on many 
> architectures you're then going to have to do masking/shifting to get the 
> bit you want to look at. That seems to me like a step backwards.

The fixed_regs array duplicate information from the fixed_reg_set. Мore
practical to use an HARD_REG_SET as there are many useful function as
hard_reg_set_subset_p, range_in_hard_reg_set_p, etc. I propose
to remove fixed_regs array use from GCC midle end and allow to use it in
TARGET_CONDITIONAL_REGISTER_USAGE target hook only. This will isolate
the bit the back end interface from the rest of GCC and simplify 
implementation
FIXED_REGISTERS target macro as target hook.

Anatoly.
diff mbox

Patch

Index: gcc/cse.c
===================================================================
--- gcc/cse.c	(revision 226953)
+++ gcc/cse.c	(working copy)
@@ -463,7 +463,7 @@ 
     A reg wins if it is either the frame pointer or designated as fixed.  */
  #define FIXED_REGNO_P(N)  \
    ((N) == FRAME_POINTER_REGNUM || (N) == HARD_FRAME_POINTER_REGNUM \
-   || fixed_regs[N] || global_regs[N])
+   || TEST_HARD_REG_BIT (fixed_reg_set, N))

  /* Compute cost of X, as stored in the `cost' field of a table_elt.  Fixed
     hard registers and pointers into the frame are the cheapest with a cost
Index: gcc/df-problems.c
===================================================================
--- gcc/df-problems.c	(revision 226953)
+++ gcc/df-problems.c	(working copy)
@@ -3871,8 +3871,7 @@ 
        EXECUTE_IF_SET_IN_BITMAP (merge_set, 0, i, bi)
  	{
  	  if (i < FIRST_PSEUDO_REGISTER
-	      && ! fixed_regs[i]
-	      && ! global_regs[i])
+	      && ! TEST_HARD_REG_BIT (fixed_reg_set, i))
  	    {
  	      fail = 1;
  	      break;
Index: gcc/postreload.c
===================================================================
--- gcc/postreload.c	(revision 226953)
+++ gcc/postreload.c	(working copy)
@@ -1144,7 +1144,7 @@ 
  		  && reg_state[i].store_ruid <= reg_state[regno].use_ruid
  		  && (call_used_regs[i] || df_regs_ever_live_p (i))
  		  && (!frame_pointer_needed || i != HARD_FRAME_POINTER_REGNUM)
-		  && !fixed_regs[i] && !global_regs[i]
+		  && !TEST_HARD_REG_BIT (fixed_reg_set, i)
  		  && hard_regno_nregs[i][GET_MODE (reg)] == 1
  		  && targetm.hard_regno_scratch_ok (i))
  		{
Index: gcc/recog.c
===================================================================
--- gcc/recog.c	(revision 226953)
+++ gcc/recog.c	(working copy)
@@ -3162,17 +3162,11 @@ 
        for (j = 0; success && j < hard_regno_nregs[regno][mode]; j++)
  	{
  	  /* Don't allocate fixed registers.  */
-	  if (fixed_regs[regno + j])
+	  if (TEST_HARD_REG_BIT (fixed_reg_set, regno + j))
  	    {
  	      success = 0;
  	      break;
  	    }
-	  /* Don't allocate global registers.  */
-	  if (global_regs[regno + j])
-	    {
-	      success = 0;
-	      break;
-	    }
  	  /* Make sure the register is of the right class.  */
  	  if (! TEST_HARD_REG_BIT (reg_class_contents[cl], regno + j))
  	    {
Index: gcc/regcprop.c
===================================================================
--- gcc/regcprop.c	(revision 226953)
+++ gcc/regcprop.c	(working copy)
@@ -315,7 +315,7 @@ 
    /* Do not propagate copies to fixed or global registers, patterns
       can be relying to see particular fixed register or users can
       expect the chosen global register in asm.  */
-  if (fixed_regs[dr] || global_regs[dr])
+  if (TEST_HARD_REG_BIT (fixed_reg_set, dr))
      return;

    /* If SRC and DEST overlap, don't record anything.  */
Index: gcc/regrename.c
===================================================================
--- gcc/regrename.c	(revision 226953)
+++ gcc/regrename.c	(working copy)
@@ -311,8 +311,7 @@ 

    for (i = nregs - 1; i >= 0; --i)
      if (TEST_HARD_REG_BIT (this_unavailable, new_reg + i)
-	|| fixed_regs[new_reg + i]
-	|| global_regs[new_reg + i]
+	|| TEST_HARD_REG_BIT (fixed_reg_set, new_reg + i)
  	/* Can't use regs which aren't saved by the prologue.  */
  	|| (! df_regs_ever_live_p (new_reg + i)
  	    && ! call_used_regs[new_reg + i])
@@ -440,7 +439,7 @@ 
        if (this_head->cannot_rename)
  	continue;

-      if (fixed_regs[reg] || global_regs[reg]
+      if (TEST_HARD_REG_BIT (fixed_reg_set, reg)
  	  || (!HARD_FRAME_POINTER_IS_FRAME_POINTER && frame_pointer_needed
  	      && reg == HARD_FRAME_POINTER_REGNUM)
  	  || (HARD_FRAME_POINTER_REGNUM && frame_pointer_needed
Index: gcc/sel-sched.c
===================================================================
--- gcc/sel-sched.c	(revision 226953)
+++ gcc/sel-sched.c	(working copy)
@@ -1089,8 +1089,7 @@ 
        nregs = hard_regno_nregs[cur_reg][mode];

        for (i = nregs - 1; i >= 0; --i)
-        if (fixed_regs[cur_reg + i]
-                || global_regs[cur_reg + i]
+        if (TEST_HARD_REG_BIT (fixed_reg_set, cur_reg + i)
              /* Can't use regs which aren't saved by
                 the prologue.  */
              || !TEST_HARD_REG_BIT (sel_hrd.regs_ever_used, cur_reg + i)
@@ -1189,8 +1188,7 @@ 

    /* Stop if the original register is one of the fixed_regs, global_regs or
       frame pointer, or we could not discover its class.  */
-  if (fixed_regs[regno]
-      || global_regs[regno]
+  if (TEST_HARD_REG_BIT (fixed_reg_set, regno)
        || (!HARD_FRAME_POINTER_IS_FRAME_POINTER && frame_pointer_needed
  	  && regno == HARD_FRAME_POINTER_REGNUM)
        || (HARD_FRAME_POINTER_REGNUM && frame_pointer_needed