[Qemu-devel] [PATCH v2 0/2] target-arm: inline abs, 64-bit negate

qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed

* [Qemu-devel] [PATCH v2 0/2] target-arm: inline abs, 64-bit negate
@ 2012-10-19 13:06 Peter Maydell
  2012-10-19 13:06 ` [Qemu-devel] [PATCH v2 1/2] target-arm: Use TCG operation for Neon 64 bit negation Peter Maydell
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Peter Maydell @ 2012-10-19 13:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: patches

Two minor patches which inline some operations rather than using helper
functions. The 64 bit negate is a no-brainer since there's a simple
TCG op for it. For abs we implement in terms of movcond:
 movi_i32 tmp6,$0x0
 neg_i32 tmp7,tmp5
 movcond_i32 tmp5,tmp5,tmp6,tmp5,tmp7,gt
which the x86-64 backend turns into:
 0x603b53a7:  mov    %ebp,%ebx
 0x603b53a9:  neg    %ebx
 0x603b53ab:  mov    %ebp,%r12d
 0x603b53ae:  test   %ebp,%ebp
 0x603b53b0:  cmovle %ebx,%r12d

Not sure why it felt the need to use an extra move there, but
for ARM this isn't in a particularly performance critical bit
of the instruction set (it's a Neon operation) so I'm not too
worried.

(A fully native TCG abs op would be able to use the fact that
neg sets flags to avoid the test as well.)

Changes v1->v2:
 drop debug printf from patch 2 (thanks to malc for spotting that)

Peter Maydell (2):
  target-arm: Use TCG operation for Neon 64 bit negation
  target-arm: Implement abs_i32 inline rather than as a helper

 target-arm/helper.c      |    5 -----
 target-arm/helper.h      |    2 --
 target-arm/neon_helper.c |    6 ------
 target-arm/translate.c   |   15 ++++++++++++---
 4 files changed, 12 insertions(+), 16 deletions(-)

-- 
1.7.9.5

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Qemu-devel] [PATCH v2 1/2] target-arm: Use TCG operation for Neon 64 bit negation
  2012-10-19 13:06 [Qemu-devel] [PATCH v2 0/2] target-arm: inline abs, 64-bit negate Peter Maydell
@ 2012-10-19 13:06 ` Peter Maydell
  2012-10-19 22:05   ` Aurelien Jarno
  2012-10-19 13:06 ` [Qemu-devel] [PATCH v2 2/2] target-arm: Implement abs_i32 inline rather than as a helper Peter Maydell
  2012-10-19 22:05 ` [Qemu-devel] [PATCH v2 0/2] target-arm: inline abs, 64-bit negate Aurelien Jarno
  2 siblings, 1 reply; 6+ messages in thread
From: Peter Maydell @ 2012-10-19 13:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: patches

Use the TCG operation to do Neon 64 bit negations rather than calling
a helper routine for it.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/helper.h      |    1 -
 target-arm/neon_helper.c |    6 ------
 target-arm/translate.c   |    4 +++-
 3 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/target-arm/helper.h b/target-arm/helper.h
index 8b9adf1..fa3472f 100644
--- a/target-arm/helper.h
+++ b/target-arm/helper.h
@@ -339,7 +339,6 @@ DEF_HELPER_2(neon_mull_s16, i64, i32, i32)
 
 DEF_HELPER_1(neon_negl_u16, i64, i64)
 DEF_HELPER_1(neon_negl_u32, i64, i64)
-DEF_HELPER_1(neon_negl_u64, i64, i64)
 
 DEF_HELPER_2(neon_qabs_s8, i32, env, i32)
 DEF_HELPER_2(neon_qabs_s16, i32, env, i32)
diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c
index 8bb5129..616cf95 100644
--- a/target-arm/neon_helper.c
+++ b/target-arm/neon_helper.c
@@ -1665,12 +1665,6 @@ uint64_t HELPER(neon_negl_u32)(uint64_t x)
     return low | ((uint64_t)high << 32);
 }
 
-/* FIXME:  There should be a native op for this.  */
-uint64_t HELPER(neon_negl_u64)(uint64_t x)
-{
-    return -x;
-}
-
 /* Saturating sign manipulation.  */
 /* ??? Make these use NEON_VOP1 */
 #define DO_QABS8(x) do { \
diff --git a/target-arm/translate.c b/target-arm/translate.c
index daccb15..d33f94c 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -4184,7 +4184,9 @@ static inline void gen_neon_negl(TCGv_i64 var, int size)
     switch (size) {
     case 0: gen_helper_neon_negl_u16(var, var); break;
     case 1: gen_helper_neon_negl_u32(var, var); break;
-    case 2: gen_helper_neon_negl_u64(var, var); break;
+    case 2:
+        tcg_gen_neg_i64(var, var);
+        break;
     default: abort();
     }
 }
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH v2 1/2] target-arm: Use TCG operation for Neon 64 bit negation
  2012-10-19 13:06 ` [Qemu-devel] [PATCH v2 1/2] target-arm: Use TCG operation for Neon 64 bit negation Peter Maydell
@ 2012-10-19 22:05   ` Aurelien Jarno
  0 siblings, 0 replies; 6+ messages in thread
From: Aurelien Jarno @ 2012-10-19 22:05 UTC (permalink / raw)
  To: Peter Maydell; +Cc: qemu-devel, patches

On Fri, Oct 19, 2012 at 02:06:58PM +0100, Peter Maydell wrote:
> Use the TCG operation to do Neon 64 bit negations rather than calling
> a helper routine for it.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/helper.h      |    1 -
>  target-arm/neon_helper.c |    6 ------
>  target-arm/translate.c   |    4 +++-
>  3 files changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/target-arm/helper.h b/target-arm/helper.h
> index 8b9adf1..fa3472f 100644
> --- a/target-arm/helper.h
> +++ b/target-arm/helper.h
> @@ -339,7 +339,6 @@ DEF_HELPER_2(neon_mull_s16, i64, i32, i32)
>  
>  DEF_HELPER_1(neon_negl_u16, i64, i64)
>  DEF_HELPER_1(neon_negl_u32, i64, i64)
> -DEF_HELPER_1(neon_negl_u64, i64, i64)
>  
>  DEF_HELPER_2(neon_qabs_s8, i32, env, i32)
>  DEF_HELPER_2(neon_qabs_s16, i32, env, i32)
> diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c
> index 8bb5129..616cf95 100644
> --- a/target-arm/neon_helper.c
> +++ b/target-arm/neon_helper.c
> @@ -1665,12 +1665,6 @@ uint64_t HELPER(neon_negl_u32)(uint64_t x)
>      return low | ((uint64_t)high << 32);
>  }
>  
> -/* FIXME:  There should be a native op for this.  */
> -uint64_t HELPER(neon_negl_u64)(uint64_t x)
> -{
> -    return -x;
> -}
> -
>  /* Saturating sign manipulation.  */
>  /* ??? Make these use NEON_VOP1 */
>  #define DO_QABS8(x) do { \
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index daccb15..d33f94c 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -4184,7 +4184,9 @@ static inline void gen_neon_negl(TCGv_i64 var, int size)
>      switch (size) {
>      case 0: gen_helper_neon_negl_u16(var, var); break;
>      case 1: gen_helper_neon_negl_u32(var, var); break;
> -    case 2: gen_helper_neon_negl_u64(var, var); break;
> +    case 2:
> +        tcg_gen_neg_i64(var, var);
> +        break;
>      default: abort();
>      }
>  }

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>


-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Qemu-devel] [PATCH v2 2/2] target-arm: Implement abs_i32 inline rather than as a helper
  2012-10-19 13:06 [Qemu-devel] [PATCH v2 0/2] target-arm: inline abs, 64-bit negate Peter Maydell
  2012-10-19 13:06 ` [Qemu-devel] [PATCH v2 1/2] target-arm: Use TCG operation for Neon 64 bit negation Peter Maydell
@ 2012-10-19 13:06 ` Peter Maydell
  2012-10-19 22:05   ` Aurelien Jarno
  2012-10-19 22:05 ` [Qemu-devel] [PATCH v2 0/2] target-arm: inline abs, 64-bit negate Aurelien Jarno
  2 siblings, 1 reply; 6+ messages in thread
From: Peter Maydell @ 2012-10-19 13:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: patches

Implement abs_i32 inline (with movcond) rather than using a helper
function.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/helper.c    |    5 -----
 target-arm/helper.h    |    1 -
 target-arm/translate.c |   11 +++++++++--
 3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 58340bd..0601760 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -1562,11 +1562,6 @@ uint32_t HELPER(rbit)(uint32_t x)
     return x;
 }
 
-uint32_t HELPER(abs)(uint32_t x)
-{
-    return ((int32_t)x < 0) ? -x : x;
-}
-
 #if defined(CONFIG_USER_ONLY)
 
 void do_interrupt (CPUARMState *env)
diff --git a/target-arm/helper.h b/target-arm/helper.h
index fa3472f..60812de 100644
--- a/target-arm/helper.h
+++ b/target-arm/helper.h
@@ -13,7 +13,6 @@ DEF_HELPER_2(double_saturate, i32, env, s32)
 DEF_HELPER_FLAGS_2(sdiv, TCG_CALL_CONST | TCG_CALL_PURE, s32, s32, s32)
 DEF_HELPER_FLAGS_2(udiv, TCG_CALL_CONST | TCG_CALL_PURE, i32, i32, i32)
 DEF_HELPER_FLAGS_1(rbit, TCG_CALL_CONST | TCG_CALL_PURE, i32, i32)
-DEF_HELPER_FLAGS_1(abs, TCG_CALL_CONST | TCG_CALL_PURE, i32, i32)
 
 #define PAS_OP(pfx)  \
     DEF_HELPER_3(pfx ## add8, i32, i32, i32, ptr) \
diff --git a/target-arm/translate.c b/target-arm/translate.c
index d33f94c..25433da 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -462,8 +462,15 @@ static void gen_sar(TCGv dest, TCGv t0, TCGv t1)
     tcg_temp_free_i32(tmp1);
 }
 
-/* FIXME:  Implement this natively.  */
-#define tcg_gen_abs_i32(t0, t1) gen_helper_abs(t0, t1)
+static void tcg_gen_abs_i32(TCGv dest, TCGv src)
+{
+    TCGv c0 = tcg_const_i32(0);
+    TCGv tmp = tcg_temp_new_i32();
+    tcg_gen_neg_i32(tmp, src);
+    tcg_gen_movcond_i32(TCG_COND_GT, dest, src, c0, src, tmp);
+    tcg_temp_free_i32(c0);
+    tcg_temp_free_i32(tmp);
+}
 
 static void shifter_out_im(TCGv var, int shift)
 {
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH v2 2/2] target-arm: Implement abs_i32 inline rather than as a helper
  2012-10-19 13:06 ` [Qemu-devel] [PATCH v2 2/2] target-arm: Implement abs_i32 inline rather than as a helper Peter Maydell
@ 2012-10-19 22:05   ` Aurelien Jarno
  0 siblings, 0 replies; 6+ messages in thread
From: Aurelien Jarno @ 2012-10-19 22:05 UTC (permalink / raw)
  To: Peter Maydell; +Cc: qemu-devel, patches

On Fri, Oct 19, 2012 at 02:06:59PM +0100, Peter Maydell wrote:
> Implement abs_i32 inline (with movcond) rather than using a helper
> function.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/helper.c    |    5 -----
>  target-arm/helper.h    |    1 -
>  target-arm/translate.c |   11 +++++++++--
>  3 files changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 58340bd..0601760 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -1562,11 +1562,6 @@ uint32_t HELPER(rbit)(uint32_t x)
>      return x;
>  }
>  
> -uint32_t HELPER(abs)(uint32_t x)
> -{
> -    return ((int32_t)x < 0) ? -x : x;
> -}
> -
>  #if defined(CONFIG_USER_ONLY)
>  
>  void do_interrupt (CPUARMState *env)
> diff --git a/target-arm/helper.h b/target-arm/helper.h
> index fa3472f..60812de 100644
> --- a/target-arm/helper.h
> +++ b/target-arm/helper.h
> @@ -13,7 +13,6 @@ DEF_HELPER_2(double_saturate, i32, env, s32)
>  DEF_HELPER_FLAGS_2(sdiv, TCG_CALL_CONST | TCG_CALL_PURE, s32, s32, s32)
>  DEF_HELPER_FLAGS_2(udiv, TCG_CALL_CONST | TCG_CALL_PURE, i32, i32, i32)
>  DEF_HELPER_FLAGS_1(rbit, TCG_CALL_CONST | TCG_CALL_PURE, i32, i32)
> -DEF_HELPER_FLAGS_1(abs, TCG_CALL_CONST | TCG_CALL_PURE, i32, i32)
>  
>  #define PAS_OP(pfx)  \
>      DEF_HELPER_3(pfx ## add8, i32, i32, i32, ptr) \
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index d33f94c..25433da 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -462,8 +462,15 @@ static void gen_sar(TCGv dest, TCGv t0, TCGv t1)
>      tcg_temp_free_i32(tmp1);
>  }
>  
> -/* FIXME:  Implement this natively.  */
> -#define tcg_gen_abs_i32(t0, t1) gen_helper_abs(t0, t1)
> +static void tcg_gen_abs_i32(TCGv dest, TCGv src)
> +{
> +    TCGv c0 = tcg_const_i32(0);
> +    TCGv tmp = tcg_temp_new_i32();
> +    tcg_gen_neg_i32(tmp, src);
> +    tcg_gen_movcond_i32(TCG_COND_GT, dest, src, c0, src, tmp);
> +    tcg_temp_free_i32(c0);
> +    tcg_temp_free_i32(tmp);
> +}
>  
>  static void shifter_out_im(TCGv var, int shift)
>  {


Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH v2 0/2] target-arm: inline abs, 64-bit negate
  2012-10-19 13:06 [Qemu-devel] [PATCH v2 0/2] target-arm: inline abs, 64-bit negate Peter Maydell
  2012-10-19 13:06 ` [Qemu-devel] [PATCH v2 1/2] target-arm: Use TCG operation for Neon 64 bit negation Peter Maydell
  2012-10-19 13:06 ` [Qemu-devel] [PATCH v2 2/2] target-arm: Implement abs_i32 inline rather than as a helper Peter Maydell
@ 2012-10-19 22:05 ` Aurelien Jarno
  2 siblings, 0 replies; 6+ messages in thread
From: Aurelien Jarno @ 2012-10-19 22:05 UTC (permalink / raw)
  To: Peter Maydell; +Cc: qemu-devel, patches

On Fri, Oct 19, 2012 at 02:06:57PM +0100, Peter Maydell wrote:
> Two minor patches which inline some operations rather than using helper
> functions. The 64 bit negate is a no-brainer since there's a simple
> TCG op for it. For abs we implement in terms of movcond:
>  movi_i32 tmp6,$0x0
>  neg_i32 tmp7,tmp5
>  movcond_i32 tmp5,tmp5,tmp6,tmp5,tmp7,gt
> which the x86-64 backend turns into:
>  0x603b53a7:  mov    %ebp,%ebx
>  0x603b53a9:  neg    %ebx
>  0x603b53ab:  mov    %ebp,%r12d
>  0x603b53ae:  test   %ebp,%ebp
>  0x603b53b0:  cmovle %ebx,%r12d
> 
> Not sure why it felt the need to use an extra move there, but

Difficult to say without saying the whole TB, but likely because 
both %ebp and %r12d are not dead after the movcond, and are used
separately.

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2012-10-19 22:05 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-19 13:06 [Qemu-devel] [PATCH v2 0/2] target-arm: inline abs, 64-bit negate Peter Maydell
2012-10-19 13:06 ` [Qemu-devel] [PATCH v2 1/2] target-arm: Use TCG operation for Neon 64 bit negation Peter Maydell
2012-10-19 22:05   ` Aurelien Jarno
2012-10-19 13:06 ` [Qemu-devel] [PATCH v2 2/2] target-arm: Implement abs_i32 inline rather than as a helper Peter Maydell
2012-10-19 22:05   ` Aurelien Jarno
2012-10-19 22:05 ` [Qemu-devel] [PATCH v2 0/2] target-arm: inline abs, 64-bit negate Aurelien Jarno

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).