Patchwork [PING,ARM] cleanup prologue_use pattern

login
register
mail settings
Submitter Greta Yorsh
Date Oct. 17, 2012, 12:37 p.m.
Message ID <000401cdac64$2b9b0860$82d11920$@yorsh@arm.com>
Download mbox | patch
Permalink /patch/192031/
State New
Headers show

Comments

Greta Yorsh - Oct. 17, 2012, 12:37 p.m.
I am attaching a new version of the patch, addressing Richard's comments.

This patch renames the exiting pattern prologue_use to force_register_use,
because the pattern is used in both prologue and epilogue.

No regression on qemu for arm-none-eabi.

Ok for trunk?

Thanks,
Greta

ChangeLog

gcc/

2012-10-17  Greta Yorsh  <Greta.Yorsh@arm.com>

        * config/arm/arm.md (UNSPEC_PROLOGUE_USE): Rename this...
        (UNSPEC_REGISTER_USE): ... to this.
        (prologue_use): Rename this...
	  (force_register_use): ... to this and update output assembly.
        (epilogue) Rename gen_prologue_use to gen_force_register_use.
        * config/arm/arm.c (arm_expand_prologue): Likewise.
        (thumb1_expand_epilogue): Likewise.
        (arm_expand_epilogue): Likewise.
        (arm_expand_epilogue): Likewise.




-----Original Message-----
From: Richard Earnshaw 
Sent: 17 October 2012 11:14
To: Greta Yorsh
Cc: GCC Patches; Ramana Radhakrishnan; nickc@redhat.com;
paul@codesourcery.com
Subject: Re: [PING][Patch, ARM] cleanup prologue_use pattern

On 17/10/12 11:08, Greta Yorsh wrote:
> Ping!
>

I've been pondering why this was being asked for.  As far as I can tell 
it's just a naming issue (mention of the epilogue in the prologue).

The right thing to do is to rename the pattern to reflect the dual use 
rather than add additional patterns with identical NOP behaviour.  Can't 
you just rename the existing pattern?  Something like "force_register_use"?

R.

> Thanks,
> Greta
>
> -----Original Message-----
> From: Greta Yorsh [mailto:greta.yorsh@arm.com]
> Sent: 10 October 2012 16:14
> To: GCC Patches
> Cc: Ramana Radhakrishnan; Richard Earnshaw; nickc@redhat.com;
> paul@codesourcery.com
> Subject: [Patch, ARM] cleanup prologue_use pattern
>
> The pattern prologue_use is emitted for both prologue and epilogue.
> In particular, the assembly comment
> "@sp needed for prologue"
> is printed out for both prologue and epilogue.
>
> This patch adds a separate pattern for epilogue_use and replaces
> prologue_use with epilogue_use where appropriate.
>
> No regression on qemu for arm-none-eabi.
>
> Ok for trunk?
>
> Thanks,
> Greta
>
> 2012-09-17  Greta Yorsh  <Greta.Yorsh@arm.com>
>
>          * config/arm/arm.md (UNSPEC_EPILOGUE_USE): New unspec value.
>          (sibcall_epilogue): Use UNSPEC_EPILOGUE_USE instead of
>          UNSPEC_PROLOGUE_USE.
>          (epilogue_use): New define_insn.
>          (epilogue): Use gen_epilogue_use instead of gen_prologue_use.
>          * config/arm/arm.c (arm_expand_epilogue): Likewise.
>          (thumb1_expand_epilogue) Likewise.
>
>
> rename-prolog-use.v2.patch.txt
>
>
> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
> index dd073da..f23c2d0 100644
> --- a/gcc/config/arm/arm.c
> +++ b/gcc/config/arm/arm.c
> @@ -22581,7 +22581,7 @@ thumb1_expand_epilogue (void)
>
>     /* Emit a USE (stack_pointer_rtx), so that
>        the stack adjustment will not be deleted.  */
> -  emit_insn (gen_prologue_use (stack_pointer_rtx));
> +  emit_insn (gen_epilogue_use (stack_pointer_rtx));
>
>     if (crtl->profile || !TARGET_SCHED_PROLOG)
>       emit_insn (gen_blockage ());
> @@ -22805,7 +22805,7 @@ arm_expand_epilogue (bool really_return)
>
>             /* Emit USE(stack_pointer_rtx) to ensure that stack adjustment
is not
>                deleted.  */
> -          emit_insn (gen_prologue_use (stack_pointer_rtx));
> +          emit_insn (gen_epilogue_use (stack_pointer_rtx));
>           }
>         else
>           {
> @@ -22823,7 +22823,7 @@ arm_expand_epilogue (bool really_return)
>             emit_insn (gen_movsi (stack_pointer_rtx,
hard_frame_pointer_rtx));
>             /* Emit USE(stack_pointer_rtx) to ensure that stack adjustment
is not
>                deleted.  */
> -          emit_insn (gen_prologue_use (stack_pointer_rtx));
> +          emit_insn (gen_epilogue_use (stack_pointer_rtx));
>           }
>       }
>     else
> @@ -22841,7 +22841,7 @@ arm_expand_epilogue (bool really_return)
>                                    GEN_INT (amount)));
>             /* Emit USE(stack_pointer_rtx) to ensure that stack adjustment
is
>                not deleted.  */
> -          emit_insn (gen_prologue_use (stack_pointer_rtx));
> +          emit_insn (gen_epilogue_use (stack_pointer_rtx));
>           }
>       }
>
> diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
> index a60e659..6a910a3 100644
> --- a/gcc/config/arm/arm.md
> +++ b/gcc/config/arm/arm.md
> @@ -81,6 +81,7 @@
>                           ; instructions setting registers for EH handling
>                           ; and stack frame generation.  Operand 0 is the
>                           ; register to "use".
> +  UNSPEC_EPILOGUE_USE   ; Same for epilogue.
>     UNSPEC_CHECK_ARCH     ; Set CCs to indicate 26-bit or 32-bit mode.
>     UNSPEC_WSHUFH         ; Used by the intrinsic form of the iWMMXt
WSHUFH instruction.
>     UNSPEC_WACC           ; Used by the intrinsic form of the iWMMXt WACC
instruction.
> @@ -10610,7 +10611,7 @@
>     "TARGET_EITHER"
>     "
>     if (crtl->calls_eh_return)
> -    emit_insn (gen_prologue_use (gen_rtx_REG (Pmode, 2)));
> +    emit_insn (gen_epilogue_use (gen_rtx_REG (Pmode, 2)));
>     if (TARGET_THUMB1)
>      {
>        thumb1_expand_epilogue ();
> @@ -10644,7 +10645,7 @@
>   ;; does not think that it is unused by the sibcall branch that
>   ;; will replace the standard function epilogue.
>   (define_expand "sibcall_epilogue"
> -   [(parallel [(unspec:SI [(reg:SI LR_REGNUM)] UNSPEC_PROLOGUE_USE)
> +   [(parallel [(unspec:SI [(reg:SI LR_REGNUM)] UNSPEC_EPILOGUE_USE)
>                  (unspec_volatile [(return)] VUNSPEC_EPILOGUE)])]
>      "TARGET_32BIT"
>      "
> @@ -11267,6 +11268,12 @@
>     [(set_attr "length" "0")]
>   )
>
> +(define_insn "epilogue_use"
> +  [(unspec:SI [(match_operand:SI 0 "register_operand" "")]
UNSPEC_EPILOGUE_USE)]
> +  ""
> +  "%@ %0 needed for epilogue"
> +  [(set_attr "length" "0")]
> +)
>
>   ;; Patterns for exception handling
>
>
Richard Earnshaw - Oct. 17, 2012, 12:47 p.m.
On 17/10/12 13:37, Greta Yorsh wrote:
> I am attaching a new version of the patch, addressing Richard's comments.
>
> This patch renames the exiting pattern prologue_use to force_register_use,
> because the pattern is used in both prologue and epilogue.
>
> No regression on qemu for arm-none-eabi.
>
> Ok for trunk?
>
> Thanks,
> Greta
>
> ChangeLog
>
> gcc/
>
> 2012-10-17  Greta Yorsh  <Greta.Yorsh@arm.com>
>
>          * config/arm/arm.md (UNSPEC_PROLOGUE_USE): Rename this...
>          (UNSPEC_REGISTER_USE): ... to this.
>          (prologue_use): Rename this...
> 	  (force_register_use): ... to this and update output assembly.
>          (epilogue) Rename gen_prologue_use to gen_force_register_use.
>          * config/arm/arm.c (arm_expand_prologue): Likewise.
>          (thumb1_expand_epilogue): Likewise.
>          (arm_expand_epilogue): Likewise.
>          (arm_expand_epilogue): Likewise.
>
>
>

OK.

R.

>
> -----Original Message-----
> From: Richard Earnshaw
> Sent: 17 October 2012 11:14
> To: Greta Yorsh
> Cc: GCC Patches; Ramana Radhakrishnan; nickc@redhat.com;
> paul@codesourcery.com
> Subject: Re: [PING][Patch, ARM] cleanup prologue_use pattern
>
> On 17/10/12 11:08, Greta Yorsh wrote:
>> Ping!
>>
>
> I've been pondering why this was being asked for.  As far as I can tell
> it's just a naming issue (mention of the epilogue in the prologue).
>
> The right thing to do is to rename the pattern to reflect the dual use
> rather than add additional patterns with identical NOP behaviour.  Can't
> you just rename the existing pattern?  Something like "force_register_use"?
>
> R.
>
>> Thanks,
>> Greta
>>
>> -----Original Message-----
>> From: Greta Yorsh [mailto:greta.yorsh@arm.com]
>> Sent: 10 October 2012 16:14
>> To: GCC Patches
>> Cc: Ramana Radhakrishnan; Richard Earnshaw; nickc@redhat.com;
>> paul@codesourcery.com
>> Subject: [Patch, ARM] cleanup prologue_use pattern
>>
>> The pattern prologue_use is emitted for both prologue and epilogue.
>> In particular, the assembly comment
>> "@sp needed for prologue"
>> is printed out for both prologue and epilogue.
>>
>> This patch adds a separate pattern for epilogue_use and replaces
>> prologue_use with epilogue_use where appropriate.
>>
>> No regression on qemu for arm-none-eabi.
>>
>> Ok for trunk?
>>
>> Thanks,
>> Greta
>>
>> 2012-09-17  Greta Yorsh  <Greta.Yorsh@arm.com>
>>
>>           * config/arm/arm.md (UNSPEC_EPILOGUE_USE): New unspec value.
>>           (sibcall_epilogue): Use UNSPEC_EPILOGUE_USE instead of
>>           UNSPEC_PROLOGUE_USE.
>>           (epilogue_use): New define_insn.
>>           (epilogue): Use gen_epilogue_use instead of gen_prologue_use.
>>           * config/arm/arm.c (arm_expand_epilogue): Likewise.
>>           (thumb1_expand_epilogue) Likewise.
>>
>>
>> rename-prolog-use.v2.patch.txt
>>
>>
>> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
>> index dd073da..f23c2d0 100644
>> --- a/gcc/config/arm/arm.c
>> +++ b/gcc/config/arm/arm.c
>> @@ -22581,7 +22581,7 @@ thumb1_expand_epilogue (void)
>>
>>      /* Emit a USE (stack_pointer_rtx), so that
>>         the stack adjustment will not be deleted.  */
>> -  emit_insn (gen_prologue_use (stack_pointer_rtx));
>> +  emit_insn (gen_epilogue_use (stack_pointer_rtx));
>>
>>      if (crtl->profile || !TARGET_SCHED_PROLOG)
>>        emit_insn (gen_blockage ());
>> @@ -22805,7 +22805,7 @@ arm_expand_epilogue (bool really_return)
>>
>>              /* Emit USE(stack_pointer_rtx) to ensure that stack adjustment
> is not
>>                 deleted.  */
>> -          emit_insn (gen_prologue_use (stack_pointer_rtx));
>> +          emit_insn (gen_epilogue_use (stack_pointer_rtx));
>>            }
>>          else
>>            {
>> @@ -22823,7 +22823,7 @@ arm_expand_epilogue (bool really_return)
>>              emit_insn (gen_movsi (stack_pointer_rtx,
> hard_frame_pointer_rtx));
>>              /* Emit USE(stack_pointer_rtx) to ensure that stack adjustment
> is not
>>                 deleted.  */
>> -          emit_insn (gen_prologue_use (stack_pointer_rtx));
>> +          emit_insn (gen_epilogue_use (stack_pointer_rtx));
>>            }
>>        }
>>      else
>> @@ -22841,7 +22841,7 @@ arm_expand_epilogue (bool really_return)
>>                                     GEN_INT (amount)));
>>              /* Emit USE(stack_pointer_rtx) to ensure that stack adjustment
> is
>>                 not deleted.  */
>> -          emit_insn (gen_prologue_use (stack_pointer_rtx));
>> +          emit_insn (gen_epilogue_use (stack_pointer_rtx));
>>            }
>>        }
>>
>> diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
>> index a60e659..6a910a3 100644
>> --- a/gcc/config/arm/arm.md
>> +++ b/gcc/config/arm/arm.md
>> @@ -81,6 +81,7 @@
>>                            ; instructions setting registers for EH handling
>>                            ; and stack frame generation.  Operand 0 is the
>>                            ; register to "use".
>> +  UNSPEC_EPILOGUE_USE   ; Same for epilogue.
>>      UNSPEC_CHECK_ARCH     ; Set CCs to indicate 26-bit or 32-bit mode.
>>      UNSPEC_WSHUFH         ; Used by the intrinsic form of the iWMMXt
> WSHUFH instruction.
>>      UNSPEC_WACC           ; Used by the intrinsic form of the iWMMXt WACC
> instruction.
>> @@ -10610,7 +10611,7 @@
>>      "TARGET_EITHER"
>>      "
>>      if (crtl->calls_eh_return)
>> -    emit_insn (gen_prologue_use (gen_rtx_REG (Pmode, 2)));
>> +    emit_insn (gen_epilogue_use (gen_rtx_REG (Pmode, 2)));
>>      if (TARGET_THUMB1)
>>       {
>>         thumb1_expand_epilogue ();
>> @@ -10644,7 +10645,7 @@
>>    ;; does not think that it is unused by the sibcall branch that
>>    ;; will replace the standard function epilogue.
>>    (define_expand "sibcall_epilogue"
>> -   [(parallel [(unspec:SI [(reg:SI LR_REGNUM)] UNSPEC_PROLOGUE_USE)
>> +   [(parallel [(unspec:SI [(reg:SI LR_REGNUM)] UNSPEC_EPILOGUE_USE)
>>                   (unspec_volatile [(return)] VUNSPEC_EPILOGUE)])]
>>       "TARGET_32BIT"
>>       "
>> @@ -11267,6 +11268,12 @@
>>      [(set_attr "length" "0")]
>>    )
>>
>> +(define_insn "epilogue_use"
>> +  [(unspec:SI [(match_operand:SI 0 "register_operand" "")]
> UNSPEC_EPILOGUE_USE)]
>> +  ""
>> +  "%@ %0 needed for epilogue"
>> +  [(set_attr "length" "0")]
>> +)
>>
>>    ;; Patterns for exception handling
>>
>>
>
>
> rename_prologue_use.v3.patch.txt
>
>
> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
> index 11f0037..248ed09 100644
> --- a/gcc/config/arm/arm.c
> +++ b/gcc/config/arm/arm.c
> @@ -16671,7 +16671,7 @@ arm_expand_prologue (void)
>   		}
>   	      emit_set_insn (ip_rtx, insn);
>   	      /* Add a USE to stop propagate_one_insn() from barfing.  */
> -	      emit_insn (gen_prologue_use (ip_rtx));
> +	      emit_insn (gen_force_register_use (ip_rtx));
>   	    }
>   	}
>         else
> @@ -22581,7 +22581,7 @@ thumb1_expand_epilogue (void)
>
>     /* Emit a USE (stack_pointer_rtx), so that
>        the stack adjustment will not be deleted.  */
> -  emit_insn (gen_prologue_use (stack_pointer_rtx));
> +  emit_insn (gen_force_register_use (stack_pointer_rtx));
>
>     if (crtl->profile || !TARGET_SCHED_PROLOG)
>       emit_insn (gen_blockage ());
> @@ -22805,7 +22805,7 @@ arm_expand_epilogue (bool really_return)
>
>             /* Emit USE(stack_pointer_rtx) to ensure that stack adjustment is not
>                deleted.  */
> -          emit_insn (gen_prologue_use (stack_pointer_rtx));
> +          emit_insn (gen_force_register_use (stack_pointer_rtx));
>           }
>         else
>           {
> @@ -22823,7 +22823,7 @@ arm_expand_epilogue (bool really_return)
>             emit_insn (gen_movsi (stack_pointer_rtx, hard_frame_pointer_rtx));
>             /* Emit USE(stack_pointer_rtx) to ensure that stack adjustment is not
>                deleted.  */
> -          emit_insn (gen_prologue_use (stack_pointer_rtx));
> +          emit_insn (gen_force_register_use (stack_pointer_rtx));
>           }
>       }
>     else
> @@ -22841,7 +22841,7 @@ arm_expand_epilogue (bool really_return)
>                                    GEN_INT (amount)));
>             /* Emit USE(stack_pointer_rtx) to ensure that stack adjustment is
>                not deleted.  */
> -          emit_insn (gen_prologue_use (stack_pointer_rtx));
> +          emit_insn (gen_force_register_use (stack_pointer_rtx));
>           }
>       }
>
> diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
> index 4735ad2..6cc4e81 100644
> --- a/gcc/config/arm/arm.md
> +++ b/gcc/config/arm/arm.md
> @@ -76,7 +76,7 @@
>                           ; that points at the containing instruction.
>     UNSPEC_PRLG_STK       ; A special barrier that prevents frame accesses
>                           ; being scheduled before the stack adjustment insn.
> -  UNSPEC_PROLOGUE_USE   ; As USE insns are not meaningful after reload,
> +  UNSPEC_REGISTER_USE   ; As USE insns are not meaningful after reload,
>                           ; this unspec is used to prevent the deletion of
>                           ; instructions setting registers for EH handling
>                           ; and stack frame generation.  Operand 0 is the
> @@ -10748,7 +10748,7 @@
>     "TARGET_EITHER"
>     "
>     if (crtl->calls_eh_return)
> -    emit_insn (gen_prologue_use (gen_rtx_REG (Pmode, 2)));
> +    emit_insn (gen_force_register_use (gen_rtx_REG (Pmode, 2)));
>     if (TARGET_THUMB1)
>      {
>        thumb1_expand_epilogue ();
> @@ -10782,7 +10782,7 @@
>   ;; does not think that it is unused by the sibcall branch that
>   ;; will replace the standard function epilogue.
>   (define_expand "sibcall_epilogue"
> -   [(parallel [(unspec:SI [(reg:SI LR_REGNUM)] UNSPEC_PROLOGUE_USE)
> +   [(parallel [(unspec:SI [(reg:SI LR_REGNUM)] UNSPEC_REGISTER_USE)
>                  (unspec_volatile [(return)] VUNSPEC_EPILOGUE)])]
>      "TARGET_32BIT"
>      "
> @@ -11398,10 +11398,10 @@
>     ""
>   )
>
> -(define_insn "prologue_use"
> -  [(unspec:SI [(match_operand:SI 0 "register_operand" "")] UNSPEC_PROLOGUE_USE)]
> +(define_insn "force_register_use"
> +  [(unspec:SI [(match_operand:SI 0 "register_operand" "")] UNSPEC_REGISTER_USE)]
>     ""
> -  "%@ %0 needed for prologue"
> +  "%@ %0 needed"
>     [(set_attr "length" "0")]
>   )
>
>

Patch

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 11f0037..248ed09 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -16671,7 +16671,7 @@  arm_expand_prologue (void)
 		}
 	      emit_set_insn (ip_rtx, insn);
 	      /* Add a USE to stop propagate_one_insn() from barfing.  */
-	      emit_insn (gen_prologue_use (ip_rtx));
+	      emit_insn (gen_force_register_use (ip_rtx));
 	    }
 	}
       else
@@ -22581,7 +22581,7 @@  thumb1_expand_epilogue (void)
 
   /* Emit a USE (stack_pointer_rtx), so that
      the stack adjustment will not be deleted.  */
-  emit_insn (gen_prologue_use (stack_pointer_rtx));
+  emit_insn (gen_force_register_use (stack_pointer_rtx));
 
   if (crtl->profile || !TARGET_SCHED_PROLOG)
     emit_insn (gen_blockage ());
@@ -22805,7 +22805,7 @@  arm_expand_epilogue (bool really_return)
 
           /* Emit USE(stack_pointer_rtx) to ensure that stack adjustment is not
              deleted.  */
-          emit_insn (gen_prologue_use (stack_pointer_rtx));
+          emit_insn (gen_force_register_use (stack_pointer_rtx));
         }
       else
         {
@@ -22823,7 +22823,7 @@  arm_expand_epilogue (bool really_return)
           emit_insn (gen_movsi (stack_pointer_rtx, hard_frame_pointer_rtx));
           /* Emit USE(stack_pointer_rtx) to ensure that stack adjustment is not
              deleted.  */
-          emit_insn (gen_prologue_use (stack_pointer_rtx));
+          emit_insn (gen_force_register_use (stack_pointer_rtx));
         }
     }
   else
@@ -22841,7 +22841,7 @@  arm_expand_epilogue (bool really_return)
                                  GEN_INT (amount)));
           /* Emit USE(stack_pointer_rtx) to ensure that stack adjustment is
              not deleted.  */
-          emit_insn (gen_prologue_use (stack_pointer_rtx));
+          emit_insn (gen_force_register_use (stack_pointer_rtx));
         }
     }
 
diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index 4735ad2..6cc4e81 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -76,7 +76,7 @@ 
                         ; that points at the containing instruction.
   UNSPEC_PRLG_STK       ; A special barrier that prevents frame accesses
                         ; being scheduled before the stack adjustment insn.
-  UNSPEC_PROLOGUE_USE   ; As USE insns are not meaningful after reload,
+  UNSPEC_REGISTER_USE   ; As USE insns are not meaningful after reload,
                         ; this unspec is used to prevent the deletion of
                         ; instructions setting registers for EH handling
                         ; and stack frame generation.  Operand 0 is the
@@ -10748,7 +10748,7 @@ 
   "TARGET_EITHER"
   "
   if (crtl->calls_eh_return)
-    emit_insn (gen_prologue_use (gen_rtx_REG (Pmode, 2)));
+    emit_insn (gen_force_register_use (gen_rtx_REG (Pmode, 2)));
   if (TARGET_THUMB1)
    {
      thumb1_expand_epilogue ();
@@ -10782,7 +10782,7 @@ 
 ;; does not think that it is unused by the sibcall branch that
 ;; will replace the standard function epilogue.
 (define_expand "sibcall_epilogue"
-   [(parallel [(unspec:SI [(reg:SI LR_REGNUM)] UNSPEC_PROLOGUE_USE)
+   [(parallel [(unspec:SI [(reg:SI LR_REGNUM)] UNSPEC_REGISTER_USE)
                (unspec_volatile [(return)] VUNSPEC_EPILOGUE)])]
    "TARGET_32BIT"
    "
@@ -11398,10 +11398,10 @@ 
   ""
 )
 
-(define_insn "prologue_use"
-  [(unspec:SI [(match_operand:SI 0 "register_operand" "")] UNSPEC_PROLOGUE_USE)]
+(define_insn "force_register_use"
+  [(unspec:SI [(match_operand:SI 0 "register_operand" "")] UNSPEC_REGISTER_USE)]
   ""
-  "%@ %0 needed for prologue"
+  "%@ %0 needed"
   [(set_attr "length" "0")]
 )