* [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
* [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 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
* 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
* 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
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).