diff mbox

Use subreg_regno instead of subreg_regno_offset

Message ID B32F41F678A04703869112BBB1967A97@Vista
State New
Headers show

Commit Message

Anatoly Sokolov Nov. 2, 2015, 10:29 p.m. UTC 
Hello.

----- Original Message ----- 
From: "Bernd Schmidt" <bschmidt@redhat.com>
Sent: Tuesday, October 27, 2015 1:50 PM

> On 10/26/2015 11:46 PM, Anatoliy Sokolov wrote:
>>    This patch change code 'REGNO (subreg) + subreg_regno_offset (...)'
>> with subreg_regno (subreg).
>
>
>> 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)))
>>           {
>
> Isn't this wrong? subreg_regno wants to be called with a SUBREG, but here 
> we already had subreg = SUBREG_REG (*pat).
>

Fixed.

>> @@ -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.
>

No promblens here. At this point op0 is equivalent orig_op0. New value to 
op0 can be assigned later.

> 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

Ok. In any case, a revised patch.

Anatoly.

Comments

Bernd Schmidt Nov. 5, 2015, 10:33 a.m. UTC | #1 
On 11/02/2015 11:29 PM, Anatoly Sokolov wrote:
>
>>> @@ -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.
>>
>
> No promblens here. At this point op0 is equivalent orig_op0. New value
> to op0 can be assigned later.

Err, what? Before the quoted code we have
         rtx op0 = orig_op0;
and then
            op0 = SUBREG_REG (op0);
Are you overlooking this line?

> +  else if (REG_P (reg) +       && HARD_REGISTER_P (reg))

I don't see how this would even compile.

> -      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);

This looks like you edited the patch? The endregno assignment is on its 
own line after this.

NAK, a final one as far as I'm concerned.


Bernd
diff mbox

Patch

Index: gcc/caller-save.c
===================================================================
--- gcc/caller-save.c	(revision 229560)
+++ 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 229560)
+++ gcc/df-scan.c	(working copy)
@@ -2587,8 +2587,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 229560)
+++ 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 (*pat),
 				  GET_MODE (subreg));
 	      return pat;
 	    }
Index: gcc/reload.c
===================================================================
--- gcc/reload.c	(revision 229560)
+++ gcc/reload.c	(working copy)
@@ -2253,10 +2253,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);
@@ -2266,10 +2263,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);
@@ -5519,12 +5513,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)
@@ -5534,12 +5523,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.
@@ -6544,14 +6528,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);
     }