All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2] target-mips: clean-up in BIT_INSV
@ 2013-05-13  1:35 Petar Jovanovic
  2013-05-17 17:41 ` Aurelien Jarno
  0 siblings, 1 reply; 2+ messages in thread
From: Petar Jovanovic @ 2013-05-13  1:35 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, petar.jovanovic, aurelien

From: Petar Jovanovic <petar.jovanovic@imgtec.com>

This is a small follow-up change to "fix incorrect behaviour for INSV".

It includes two minor modifications:

- sizefilter is constant so it can be moved inside of the block,
- several lines of the code are replaced with a call to deposit64.

No functional change.

Signed-off-by: Petar Jovanovic <petar.jovanovic@imgtec.com>
---
 v2:

 - version one was based on Aurelien comments,
 - version two includes update (use of deposit64 helper) per Peter
   Maydell's suggestion.

 target-mips/dsp_helper.c |   17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/target-mips/dsp_helper.c b/target-mips/dsp_helper.c
index 9212789..af2aa05 100644
--- a/target-mips/dsp_helper.c
+++ b/target-mips/dsp_helper.c
@@ -19,6 +19,7 @@
 
 #include "cpu.h"
 #include "helper.h"
+#include "qemu/bitops.h"
 
 /* As the byte ordering doesn't matter, i.e. all columns are treated
    identically, these unions can be used directly.  */
@@ -2900,13 +2901,13 @@ target_ulong helper_bitrev(target_ulong rt)
     return (target_ulong)rd;
 }
 
-#define BIT_INSV(name, posfilter, sizefilter, ret_type)         \
+#define BIT_INSV(name, posfilter, ret_type)                     \
 target_ulong helper_##name(CPUMIPSState *env, target_ulong rs,  \
                            target_ulong rt)                     \
 {                                                               \
     uint32_t pos, size, msb, lsb;                               \
-    target_ulong filter;                                        \
-    target_ulong temp, temprs, temprt;                          \
+    uint32_t const sizefilter = 0x3F;                           \
+    target_ulong temp;                                          \
     target_ulong dspc;                                          \
                                                                 \
     dspc = env->active_tc.DSPControl;                           \
@@ -2921,18 +2922,14 @@ target_ulong helper_##name(CPUMIPSState *env, target_ulong rs,  \
         return rt;                                              \
     }                                                           \
                                                                 \
-    filter = ((int64_t)0x01 << size) - 1;                       \
-    filter = filter << pos;                                     \
-    temprs = (rs << pos) & filter;                              \
-    temprt = rt & ~filter;                                      \
-    temp = temprs | temprt;                                     \
+    temp = deposit64(rt, pos, size, rs);                        \
                                                                 \
     return (target_long)(ret_type)temp;                         \
 }
 
-BIT_INSV(insv, 0x1F, 0x3F, int32_t);
+BIT_INSV(insv, 0x1F, int32_t);
 #ifdef TARGET_MIPS64
-BIT_INSV(dinsv, 0x7F, 0x3F, target_long);
+BIT_INSV(dinsv, 0x7F, target_long);
 #endif
 
 #undef BIT_INSV
-- 
1.7.9.5

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

* Re: [Qemu-devel] [PATCH v2] target-mips: clean-up in BIT_INSV
  2013-05-13  1:35 [Qemu-devel] [PATCH v2] target-mips: clean-up in BIT_INSV Petar Jovanovic
@ 2013-05-17 17:41 ` Aurelien Jarno
  0 siblings, 0 replies; 2+ messages in thread
From: Aurelien Jarno @ 2013-05-17 17:41 UTC (permalink / raw)
  To: Petar Jovanovic; +Cc: peter.maydell, qemu-devel, petar.jovanovic

On Mon, May 13, 2013 at 03:35:37AM +0200, Petar Jovanovic wrote:
> From: Petar Jovanovic <petar.jovanovic@imgtec.com>
> 
> This is a small follow-up change to "fix incorrect behaviour for INSV".
> 
> It includes two minor modifications:
> 
> - sizefilter is constant so it can be moved inside of the block,
> - several lines of the code are replaced with a call to deposit64.
> 
> No functional change.
> 
> Signed-off-by: Petar Jovanovic <petar.jovanovic@imgtec.com>
> ---
>  v2:
> 
>  - version one was based on Aurelien comments,
>  - version two includes update (use of deposit64 helper) per Peter
>    Maydell's suggestion.
> 
>  target-mips/dsp_helper.c |   17 +++++++----------
>  1 file changed, 7 insertions(+), 10 deletions(-)

This version is indeed better. I have applied it in my queue for after
1.6, replacing the previous version. Thanks.

> diff --git a/target-mips/dsp_helper.c b/target-mips/dsp_helper.c
> index 9212789..af2aa05 100644
> --- a/target-mips/dsp_helper.c
> +++ b/target-mips/dsp_helper.c
> @@ -19,6 +19,7 @@
>  
>  #include "cpu.h"
>  #include "helper.h"
> +#include "qemu/bitops.h"
>  
>  /* As the byte ordering doesn't matter, i.e. all columns are treated
>     identically, these unions can be used directly.  */
> @@ -2900,13 +2901,13 @@ target_ulong helper_bitrev(target_ulong rt)
>      return (target_ulong)rd;
>  }
>  
> -#define BIT_INSV(name, posfilter, sizefilter, ret_type)         \
> +#define BIT_INSV(name, posfilter, ret_type)                     \
>  target_ulong helper_##name(CPUMIPSState *env, target_ulong rs,  \
>                             target_ulong rt)                     \
>  {                                                               \
>      uint32_t pos, size, msb, lsb;                               \
> -    target_ulong filter;                                        \
> -    target_ulong temp, temprs, temprt;                          \
> +    uint32_t const sizefilter = 0x3F;                           \
> +    target_ulong temp;                                          \
>      target_ulong dspc;                                          \
>                                                                  \
>      dspc = env->active_tc.DSPControl;                           \
> @@ -2921,18 +2922,14 @@ target_ulong helper_##name(CPUMIPSState *env, target_ulong rs,  \
>          return rt;                                              \
>      }                                                           \
>                                                                  \
> -    filter = ((int64_t)0x01 << size) - 1;                       \
> -    filter = filter << pos;                                     \
> -    temprs = (rs << pos) & filter;                              \
> -    temprt = rt & ~filter;                                      \
> -    temp = temprs | temprt;                                     \
> +    temp = deposit64(rt, pos, size, rs);                        \
>                                                                  \
>      return (target_long)(ret_type)temp;                         \
>  }
>  
> -BIT_INSV(insv, 0x1F, 0x3F, int32_t);
> +BIT_INSV(insv, 0x1F, int32_t);
>  #ifdef TARGET_MIPS64
> -BIT_INSV(dinsv, 0x7F, 0x3F, target_long);
> +BIT_INSV(dinsv, 0x7F, target_long);
>  #endif
>  
>  #undef BIT_INSV
> -- 
> 1.7.9.5
> 
> 

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

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

end of thread, other threads:[~2013-05-17 17:41 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-13  1:35 [Qemu-devel] [PATCH v2] target-mips: clean-up in BIT_INSV Petar Jovanovic
2013-05-17 17:41 ` Aurelien Jarno

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.