This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH][AArch64] Use std::swap instead of manually swapping in aarch64-ldpstp.md


On Wed, Feb 04, 2015 at 12:18:29PM +0000, Kyrill Tkachov wrote:
> Hi all,
> 
> This patch makes use of std::swap in every peephole2 of 
> aarch64-ldp-stp.md instead of manually swapping rtxen.
> No functional change, just a cleanup.
> Bootstrapped and tested on aarch64.
> 
> I'm proposing this for next stage1 together with the other AArch64 patch 
> that
> moves a couple of places of aarch64.c to use std::swap
> (https://gcc.gnu.org/ml/gcc-patches/2014-11/msg01426.html)

Both of these are obvious patches that it would be hard to object to.
If you put them in now, then 6.0 -> 5.n backports will be easier.

Remember, the obvious rule is:

  "Obvious fixes can be committed without prior approval. Just check in 
   the fix and copy it to gcc-patches. A good test to determine whether a fix
   is obvious: "will the person who objects to my work the most be able to find
   a fault with my fix?" If the fix is later found to be faulty, it can always
   be rolled back.  We don't want to get overly restrictive about checkin
   policies."

Can you really see somebody objecting to this clean up? We have plenty of
uses of std::swap already, you've made the same changes to the arm port,
and other targets are also moving (and committing the patches as
obvious!)

But, as you asked - this looks fine, but you'll need Marcus or
Richard to OK it.

Thanks,
James

> 
> Thanks,
> Kyrill
> 
> 2015-02-04  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
> 
>      * config/aarch64/aarch64-ldpstp.md: Use std::swap instead of
>      manual swaps in all peepholes.

> commit d1f937c2ed28d3d1bab09536b93906ba6fe99fa9
> Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
> Date:   Mon Feb 2 14:44:54 2015 +0000
> 
>     [AArch64] Use std::swap in aarch64-ldpstp.md
> 
> diff --git a/gcc/config/aarch64/aarch64-ldpstp.md b/gcc/config/aarch64/aarch64-ldpstp.md
> index af8d50e..8d6d882 100644
> --- a/gcc/config/aarch64/aarch64-ldpstp.md
> +++ b/gcc/config/aarch64/aarch64-ldpstp.md
> @@ -27,18 +27,14 @@ (define_peephole2
>    [(parallel [(set (match_dup 0) (match_dup 1))
>  	      (set (match_dup 2) (match_dup 3))])]
>  {
> -  rtx base, offset_1, offset_2, tmp;
> +  rtx base, offset_1, offset_2;
>  
>    extract_base_offset_in_addr (operands[1], &base, &offset_1);
>    extract_base_offset_in_addr (operands[3], &base, &offset_2);
>    if (INTVAL (offset_1) > INTVAL (offset_2))
>      {
> -      tmp = operands[0];
> -      operands[0] = operands[2];
> -      operands[2] = tmp;
> -      tmp = operands[1];
> -      operands[1] = operands[3];
> -      operands[3] = tmp;
> +      std::swap (operands[0], operands[2]);
> +      std::swap (operands[1], operands[3]);
>      }
>  })
>  
> @@ -51,18 +47,14 @@ (define_peephole2
>    [(parallel [(set (match_dup 0) (match_dup 1))
>  	      (set (match_dup 2) (match_dup 3))])]
>  {
> -  rtx base, offset_1, offset_2, tmp;
> +  rtx base, offset_1, offset_2;
>  
>    extract_base_offset_in_addr (operands[0], &base, &offset_1);
>    extract_base_offset_in_addr (operands[2], &base, &offset_2);
>    if (INTVAL (offset_1) > INTVAL (offset_2))
>      {
> -      tmp = operands[0];
> -      operands[0] = operands[2];
> -      operands[2] = tmp;
> -      tmp = operands[1];
> -      operands[1] = operands[3];
> -      operands[3] = tmp;
> +      std::swap (operands[0], operands[2]);
> +      std::swap (operands[1], operands[3]);
>      }
>  })
>  
> @@ -75,18 +67,14 @@ (define_peephole2
>    [(parallel [(set (match_dup 0) (match_dup 1))
>  	      (set (match_dup 2) (match_dup 3))])]
>  {
> -  rtx base, offset_1, offset_2, tmp;
> +  rtx base, offset_1, offset_2;
>  
>    extract_base_offset_in_addr (operands[1], &base, &offset_1);
>    extract_base_offset_in_addr (operands[3], &base, &offset_2);
>    if (INTVAL (offset_1) > INTVAL (offset_2))
>      {
> -      tmp = operands[0];
> -      operands[0] = operands[2];
> -      operands[2] = tmp;
> -      tmp = operands[1];
> -      operands[1] = operands[3];
> -      operands[3] = tmp;
> +      std::swap (operands[0], operands[2]);
> +      std::swap (operands[1], operands[3]);
>      }
>  })
>  
> @@ -99,18 +87,14 @@ (define_peephole2
>    [(parallel [(set (match_dup 0) (match_dup 1))
>  	      (set (match_dup 2) (match_dup 3))])]
>  {
> -  rtx base, offset_1, offset_2, tmp;
> +  rtx base, offset_1, offset_2;
>  
>    extract_base_offset_in_addr (operands[0], &base, &offset_1);
>    extract_base_offset_in_addr (operands[2], &base, &offset_2);
>    if (INTVAL (offset_1) > INTVAL (offset_2))
>      {
> -      tmp = operands[0];
> -      operands[0] = operands[2];
> -      operands[2] = tmp;
> -      tmp = operands[1];
> -      operands[1] = operands[3];
> -      operands[3] = tmp;
> +      std::swap (operands[0], operands[2]);
> +      std::swap (operands[1], operands[3]);
>      }
>  })
>  
> @@ -125,18 +109,14 @@ (define_peephole2
>    [(parallel [(set (match_dup 0) (sign_extend:DI (match_dup 1)))
>  	      (set (match_dup 2) (sign_extend:DI (match_dup 3)))])]
>  {
> -  rtx base, offset_1, offset_2, tmp;
> +  rtx base, offset_1, offset_2;
>  
>    extract_base_offset_in_addr (operands[1], &base, &offset_1);
>    extract_base_offset_in_addr (operands[3], &base, &offset_2);
>    if (INTVAL (offset_1) > INTVAL (offset_2))
>      {
> -      tmp = operands[0];
> -      operands[0] = operands[2];
> -      operands[2] = tmp;
> -      tmp = operands[1];
> -      operands[1] = operands[3];
> -      operands[3] = tmp;
> +      std::swap (operands[0], operands[2]);
> +      std::swap (operands[1], operands[3]);
>      }
>  })
>  
> @@ -149,18 +129,14 @@ (define_peephole2
>    [(parallel [(set (match_dup 0) (zero_extend:DI (match_dup 1)))
>  	      (set (match_dup 2) (zero_extend:DI (match_dup 3)))])]
>  {
> -  rtx base, offset_1, offset_2, tmp;
> +  rtx base, offset_1, offset_2;
>  
>    extract_base_offset_in_addr (operands[1], &base, &offset_1);
>    extract_base_offset_in_addr (operands[3], &base, &offset_2);
>    if (INTVAL (offset_1) > INTVAL (offset_2))
>      {
> -      tmp = operands[0];
> -      operands[0] = operands[2];
> -      operands[2] = tmp;
> -      tmp = operands[1];
> -      operands[1] = operands[3];
> -      operands[3] = tmp;
> +      std::swap (operands[0], operands[2]);
> +      std::swap (operands[1], operands[3]);
>      }
>  })
>  
> @@ -183,24 +159,16 @@ (define_peephole2
>    "aarch64_operands_adjust_ok_for_ldpstp (operands, true, <MODE>mode)"
>    [(const_int 0)]
>  {
> -  rtx base, offset_1, offset_2, tmp;
> +  rtx base, offset_1, offset_2;
>  
>    extract_base_offset_in_addr (operands[1], &base, &offset_1);
>    extract_base_offset_in_addr (operands[3], &base, &offset_2);
>    if (INTVAL (offset_1) > INTVAL (offset_2))
>      {
> -      tmp = operands[0];
> -      operands[0] = operands[6];
> -      operands[6] = tmp;
> -      tmp = operands[1];
> -      operands[1] = operands[7];
> -      operands[7] = tmp;
> -      tmp = operands[2];
> -      operands[2] = operands[4];
> -      operands[4] = tmp;
> -      tmp = operands[3];
> -      operands[3] = operands[5];
> -      operands[5] = tmp;
> +      std::swap (operands[0], operands[6]);
> +      std::swap (operands[1], operands[7]);
> +      std::swap (operands[2], operands[4]);
> +      std::swap (operands[3], operands[5]);
>      }
>  
>    if (aarch64_gen_adjusted_ldpstp (operands, true, <MODE>mode, UNKNOWN))
> @@ -223,24 +191,16 @@ (define_peephole2
>    "aarch64_operands_adjust_ok_for_ldpstp (operands, true, <MODE>mode)"
>    [(const_int 0)]
>  {
> -  rtx base, offset_1, offset_2, tmp;
> +  rtx base, offset_1, offset_2;
>  
>    extract_base_offset_in_addr (operands[1], &base, &offset_1);
>    extract_base_offset_in_addr (operands[3], &base, &offset_2);
>    if (INTVAL (offset_1) > INTVAL (offset_2))
>      {
> -      tmp = operands[0];
> -      operands[0] = operands[6];
> -      operands[6] = tmp;
> -      tmp = operands[1];
> -      operands[1] = operands[7];
> -      operands[7] = tmp;
> -      tmp = operands[2];
> -      operands[2] = operands[4];
> -      operands[4] = tmp;
> -      tmp = operands[3];
> -      operands[3] = operands[5];
> -      operands[5] = tmp;
> +      std::swap (operands[0], operands[6]);
> +      std::swap (operands[1], operands[7]);
> +      std::swap (operands[2], operands[4]);
> +      std::swap (operands[3], operands[5]);
>      }
>  
>    if (aarch64_gen_adjusted_ldpstp (operands, true, <MODE>mode, UNKNOWN))
> @@ -263,24 +223,16 @@ (define_peephole2
>    "aarch64_operands_adjust_ok_for_ldpstp (operands, true, SImode)"
>    [(const_int 0)]
>  {
> -  rtx base, offset_1, offset_2, tmp;
> +  rtx base, offset_1, offset_2;
>  
>    extract_base_offset_in_addr (operands[1], &base, &offset_1);
>    extract_base_offset_in_addr (operands[3], &base, &offset_2);
>    if (INTVAL (offset_1) > INTVAL (offset_2))
>      {
> -      tmp = operands[0];
> -      operands[0] = operands[6];
> -      operands[6] = tmp;
> -      tmp = operands[1];
> -      operands[1] = operands[7];
> -      operands[7] = tmp;
> -      tmp = operands[2];
> -      operands[2] = operands[4];
> -      operands[4] = tmp;
> -      tmp = operands[3];
> -      operands[3] = operands[5];
> -      operands[5] = tmp;
> +      std::swap (operands[0], operands[6]);
> +      std::swap (operands[1], operands[7]);
> +      std::swap (operands[2], operands[4]);
> +      std::swap (operands[3], operands[5]);
>      }
>  
>    if (aarch64_gen_adjusted_ldpstp (operands, true, SImode, SIGN_EXTEND))
> @@ -303,24 +255,16 @@ (define_peephole2
>    "aarch64_operands_adjust_ok_for_ldpstp (operands, true, SImode)"
>    [(const_int 0)]
>  {
> -  rtx base, offset_1, offset_2, tmp;
> +  rtx base, offset_1, offset_2;
>  
>    extract_base_offset_in_addr (operands[1], &base, &offset_1);
>    extract_base_offset_in_addr (operands[3], &base, &offset_2);
>    if (INTVAL (offset_1) > INTVAL (offset_2))
>      {
> -      tmp = operands[0];
> -      operands[0] = operands[6];
> -      operands[6] = tmp;
> -      tmp = operands[1];
> -      operands[1] = operands[7];
> -      operands[7] = tmp;
> -      tmp = operands[2];
> -      operands[2] = operands[4];
> -      operands[4] = tmp;
> -      tmp = operands[3];
> -      operands[3] = operands[5];
> -      operands[5] = tmp;
> +      std::swap (operands[0], operands[6]);
> +      std::swap (operands[1], operands[7]);
> +      std::swap (operands[2], operands[4]);
> +      std::swap (operands[3], operands[5]);
>      }
>  
>    if (aarch64_gen_adjusted_ldpstp (operands, true, SImode, ZERO_EXTEND))
> @@ -343,24 +287,16 @@ (define_peephole2
>    "aarch64_operands_adjust_ok_for_ldpstp (operands, false, <MODE>mode)"
>    [(const_int 0)]
>  {
> -  rtx base, offset_1, offset_2, tmp;
> +  rtx base, offset_1, offset_2;
>  
>    extract_base_offset_in_addr (operands[0], &base, &offset_1);
>    extract_base_offset_in_addr (operands[2], &base, &offset_2);
>    if (INTVAL (offset_1) > INTVAL (offset_2))
>      {
> -      tmp = operands[0];
> -      operands[0] = operands[6];
> -      operands[6] = tmp;
> -      tmp = operands[1];
> -      operands[1] = operands[7];
> -      operands[7] = tmp;
> -      tmp = operands[2];
> -      operands[2] = operands[4];
> -      operands[4] = tmp;
> -      tmp = operands[3];
> -      operands[3] = operands[5];
> -      operands[5] = tmp;
> +      std::swap (operands[0], operands[6]);
> +      std::swap (operands[1], operands[7]);
> +      std::swap (operands[2], operands[4]);
> +      std::swap (operands[3], operands[5]);
>      }
>  
>    if (aarch64_gen_adjusted_ldpstp (operands, false, <MODE>mode, UNKNOWN))
> @@ -383,24 +319,16 @@ (define_peephole2
>    "aarch64_operands_adjust_ok_for_ldpstp (operands, false, <MODE>mode)"
>    [(const_int 0)]
>  {
> -  rtx base, offset_1, offset_2, tmp;
> +  rtx base, offset_1, offset_2;
>  
>    extract_base_offset_in_addr (operands[0], &base, &offset_1);
>    extract_base_offset_in_addr (operands[2], &base, &offset_2);
>    if (INTVAL (offset_1) > INTVAL (offset_2))
>      {
> -      tmp = operands[0];
> -      operands[0] = operands[6];
> -      operands[6] = tmp;
> -      tmp = operands[1];
> -      operands[1] = operands[7];
> -      operands[7] = tmp;
> -      tmp = operands[2];
> -      operands[2] = operands[4];
> -      operands[4] = tmp;
> -      tmp = operands[3];
> -      operands[3] = operands[5];
> -      operands[5] = tmp;
> +      std::swap (operands[0], operands[6]);
> +      std::swap (operands[1], operands[7]);
> +      std::swap (operands[2], operands[4]);
> +      std::swap (operands[3], operands[5]);
>      }
>  
>    if (aarch64_gen_adjusted_ldpstp (operands, false, <MODE>mode, UNKNOWN))


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]