diff mbox

Use subreg_regno instead of subreg_regno_offset

Message ID 562EAD37.4040402@post.ru
State New
Headers show

Commit Message

Anatoly Sokolov Oct. 26, 2015, 10:46 p.m. UTC 
Hello.

   This patch change code 'REGNO (subreg) + subreg_regno_offset (...)' with 
subreg_regno (subreg).

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

OK for trunk?

2015-10-27  Anatoly Sokolov  <aesok@post.ru>

	* caller-save.c (add_stored_regs): Use subreg_regno instead of
	subreg_regno_offset.
	* df-scan.c (df_ref_record): Ditto.
	* reg-stack.c (get_true_reg): Ditto.
	* reload.c (operands_match_p, find_reloads_address_1,
	reg_overlap_mentioned_for_reload_p): Ditto.



Anatoliy.

Comments

Bernd Schmidt Oct. 27, 2015, 10:50 a.m. UTC | #1 
On 10/26/2015 11:46 PM, Anatoliy Sokolov wrote:
>    This patch change code 'REGNO (subreg) + subreg_regno_offset (...)'
> with subreg_regno (subreg).

The patch has whitespace damage that makes it difficult to apply. Please 
use text/plain attachments.

> Index: gcc/reg-stack.c
> ===================================================================
> --- gcc/reg-stack.c    (revision 229083)
> +++ gcc/reg-stack.c    (working copy)
> @@ -416,11 +416,7 @@
>         rtx subreg;
>         if (STACK_REG_P (subreg = SUBREG_REG (*pat)))
>           {
> -          int regno_off = subreg_regno_offset (REGNO (subreg),
> -                           GET_MODE (subreg),
> -                           SUBREG_BYTE (*pat),
> -                           GET_MODE (*pat));
> -          *pat = FP_MODE_REG (REGNO (subreg) + regno_off,
> +          *pat = FP_MODE_REG (subreg_regno (subreg),
>                     GET_MODE (subreg));
>             return pat;

Isn't this wrong? subreg_regno wants to be called with a SUBREG, but 
here we already had subreg = SUBREG_REG (*pat).

> @@ -5522,12 +5516,7 @@
>           op0 = SUBREG_REG (op0);
>           code0 = GET_CODE (op0);
>           if (code0 == REG && REGNO (op0) < FIRST_PSEUDO_REGISTER)
> -          op0 = gen_rtx_REG (word_mode,
> -                 (REGNO (op0) +
> -                  subreg_regno_offset (REGNO (SUBREG_REG (orig_op0)),
> -                               GET_MODE (SUBREG_REG (orig_op0)),
> -                               SUBREG_BYTE (orig_op0),
> -                               GET_MODE (orig_op0))));
> +          op0 = gen_rtx_REG (word_mode, subreg_regno (op0));
>         }

Same problem as in the reg-stack code? The existing code was using 
orig_op0 to get the subreg, you've changed it to use op0 which is 
already the SUBREG_REG.

With an x86 test you're not exercising reload, and even on other targets 
this is not a frequently used path. I've stopped reviewing here, I think 
this is a good example of the kind of cleanup patch we _shouldn't_ be 
doing. We've proved it's risky, and unless these cleanup patches were a 
preparation for functional changes, we should just leave the code alone IMO.


Bernd
diff mbox

Patch

Index: gcc/caller-save.c
===================================================================
--- gcc/caller-save.c	(revision 229083)
+++ gcc/caller-save.c	(working copy)
@@ -991,31 +991,25 @@ 
  add_stored_regs (rtx reg, const_rtx setter, void *data)
  {
    int regno, endregno, i;
-  machine_mode mode = GET_MODE (reg);
-  int offset = 0;

    if (GET_CODE (setter) == CLOBBER)
      return;

-  if (GET_CODE (reg) == SUBREG
+  if (SUBREG_P (reg)
        && REG_P (SUBREG_REG (reg))
-      && REGNO (SUBREG_REG (reg)) < FIRST_PSEUDO_REGISTER)
+      && HARD_REGISTER_P (SUBREG_REG (reg)))
      {
-      offset = subreg_regno_offset (REGNO (SUBREG_REG (reg)),
-				    GET_MODE (SUBREG_REG (reg)),
-				    SUBREG_BYTE (reg),
-				    GET_MODE (reg));
-      regno = REGNO (SUBREG_REG (reg)) + offset;
+      regno = subreg_regno (reg);
        endregno = regno + subreg_nregs (reg);
      }
-  else
+  else if (REG_P (reg)
+	   && HARD_REGISTER_P (reg))
      {
-      if (!REG_P (reg) || REGNO (reg) >= FIRST_PSEUDO_REGISTER)
-	return;
-
-      regno = REGNO (reg) + offset;
-      endregno = end_hard_regno (mode, regno);
+      regno = REGNO (reg);
+      endregno = end_hard_regno (GET_MODE (reg), regno);
      }
+  else
+    return;

    for (i = regno; i < endregno; i++)
      SET_REGNO_REG_SET ((regset) data, i);
Index: gcc/df-scan.c
===================================================================
--- gcc/df-scan.c	(revision 229083)
+++ gcc/df-scan.c	(working copy)
@@ -2588,8 +2588,7 @@ 

        if (GET_CODE (reg) == SUBREG)
  	{
-	  regno += subreg_regno_offset (regno, GET_MODE (SUBREG_REG (reg)),
-					SUBREG_BYTE (reg), GET_MODE (reg));
+	  regno = subreg_regno (reg);
  	  endregno = regno + subreg_nregs (reg);
  	}
        else
Index: gcc/reg-stack.c
===================================================================
--- gcc/reg-stack.c	(revision 229083)
+++ gcc/reg-stack.c	(working copy)
@@ -416,11 +416,7 @@ 
  	  rtx subreg;
  	  if (STACK_REG_P (subreg = SUBREG_REG (*pat)))
  	    {
-	      int regno_off = subreg_regno_offset (REGNO (subreg),
-						   GET_MODE (subreg),
-						   SUBREG_BYTE (*pat),
-						   GET_MODE (*pat));
-	      *pat = FP_MODE_REG (REGNO (subreg) + regno_off,
+	      *pat = FP_MODE_REG (subreg_regno (subreg),
  				  GET_MODE (subreg));
  	      return pat;
  	    }
Index: gcc/reload.c
===================================================================
--- gcc/reload.c	(revision 229083)
+++ gcc/reload.c	(working copy)
@@ -2256,10 +2256,7 @@ 
  	  i = REGNO (SUBREG_REG (x));
  	  if (i >= FIRST_PSEUDO_REGISTER)
  	    goto slow;
-	  i += subreg_regno_offset (REGNO (SUBREG_REG (x)),
-				    GET_MODE (SUBREG_REG (x)),
-				    SUBREG_BYTE (x),
-				    GET_MODE (x));
+	  i = subreg_regno (x);
  	}
        else
  	i = REGNO (x);
@@ -2269,10 +2266,7 @@ 
  	  j = REGNO (SUBREG_REG (y));
  	  if (j >= FIRST_PSEUDO_REGISTER)
  	    goto slow;
-	  j += subreg_regno_offset (REGNO (SUBREG_REG (y)),
-				    GET_MODE (SUBREG_REG (y)),
-				    SUBREG_BYTE (y),
-				    GET_MODE (y));
+	  j = subreg_regno (y);
  	}
        else
  	j = REGNO (y);
@@ -5522,12 +5516,7 @@ 
  	    op0 = SUBREG_REG (op0);
  	    code0 = GET_CODE (op0);
  	    if (code0 == REG && REGNO (op0) < FIRST_PSEUDO_REGISTER)
-	      op0 = gen_rtx_REG (word_mode,
-				 (REGNO (op0) +
-				  subreg_regno_offset (REGNO (SUBREG_REG (orig_op0)),
-						       GET_MODE (SUBREG_REG (orig_op0)),
-						       SUBREG_BYTE (orig_op0),
-						       GET_MODE (orig_op0))));
+	      op0 = gen_rtx_REG (word_mode, subreg_regno (op0));
  	  }

  	if (GET_CODE (op1) == SUBREG)
@@ -5537,12 +5526,7 @@ 
  	    if (code1 == REG && REGNO (op1) < FIRST_PSEUDO_REGISTER)
  	      /* ??? Why is this given op1's mode and above for
  		 ??? op0 SUBREGs we use word_mode?  */
-	      op1 = gen_rtx_REG (GET_MODE (op1),
-				 (REGNO (op1) +
-				  subreg_regno_offset (REGNO (SUBREG_REG (orig_op1)),
-						       GET_MODE (SUBREG_REG (orig_op1)),
-						       SUBREG_BYTE (orig_op1),
-						       GET_MODE (orig_op1))));
+	      op1 = gen_rtx_REG (GET_MODE (op1), subreg_regno (op1));
  	  }
  	/* Plus in the index register may be created only as a result of
  	   register rematerialization for expression like &localvar*4.  Reload it.
@@ -6547,14 +6531,16 @@ 
      return refers_to_mem_for_reload_p (in);
    else if (GET_CODE (x) == SUBREG)
      {
-      regno = REGNO (SUBREG_REG (x));
-      if (regno < FIRST_PSEUDO_REGISTER)
-	regno += subreg_regno_offset (REGNO (SUBREG_REG (x)),
-				      GET_MODE (SUBREG_REG (x)),
-				      SUBREG_BYTE (x),
-				      GET_MODE (x));
-      endregno = regno + (regno < FIRST_PSEUDO_REGISTER
-			  ? subreg_nregs (x) : 1);
+      if (HARD_REGISTER_P (SUBREG_REG (x)))
+	{
+	  regno = subreg_regno (x);
+	  endregno = regno + subreg_nregs (x);
+	}
+      else
+	{
+	  regno = REGNO (SUBREG_REG (x));
+	  endregno = regno + 1;
+	}

        return refers_to_regno_for_reload_p (regno, endregno, in, (rtx*) 0);
      }