diff mbox

target-mips: clean-up in BIT_INSV

Message ID 1368120517-84396-1-git-send-email-petar.jovanovic@rt-rk.com
State New
Headers show

Commit Message

Petar Jovanovic May 9, 2013, 5:28 p.m. UTC
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,
- (int64_t)0x01 is replaced with 1LL for ease of reading.

No functional change.

Signed-off-by: Petar Jovanovic <petar.jovanovic@imgtec.com>
---
 target-mips/dsp_helper.c |    9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

Comments

Aurelien Jarno May 9, 2013, 5:42 p.m. UTC | #1
On Thu, May 09, 2013 at 07:28:37PM +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,
> - (int64_t)0x01 is replaced with 1LL for ease of reading.
> 
> No functional change.
> 
> Signed-off-by: Petar Jovanovic <petar.jovanovic@imgtec.com>
> ---
>  target-mips/dsp_helper.c |    9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/target-mips/dsp_helper.c b/target-mips/dsp_helper.c
> index 9212789..426a3b6 100644
> --- a/target-mips/dsp_helper.c
> +++ b/target-mips/dsp_helper.c
> @@ -2900,11 +2900,12 @@ 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;                               \
> +    uint32_t const sizefilter = 0x3F;                           \
>      target_ulong filter;                                        \
>      target_ulong temp, temprs, temprt;                          \
>      target_ulong dspc;                                          \
> @@ -2921,7 +2922,7 @@ target_ulong helper_##name(CPUMIPSState *env, target_ulong rs,  \
>          return rt;                                              \
>      }                                                           \
>                                                                  \
> -    filter = ((int64_t)0x01 << size) - 1;                       \
> +    filter = (1LL << size) - 1;                                 \
>      filter = filter << pos;                                     \
>      temprs = (rs << pos) & filter;                              \
>      temprt = rt & ~filter;                                      \
> @@ -2930,9 +2931,9 @@ target_ulong helper_##name(CPUMIPSState *env, target_ulong 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

Thanks, queued for 1.6.
Peter Maydell May 9, 2013, 5:54 p.m. UTC | #2
On 9 May 2013 18:28, Petar Jovanovic <petar.jovanovic@rt-rk.com> wrote:
> @@ -2921,7 +2922,7 @@ target_ulong helper_##name(CPUMIPSState *env, target_ulong rs,  \
>          return rt;                                              \
>      }                                                           \
>                                                                  \
> -    filter = ((int64_t)0x01 << size) - 1;                       \
> +    filter = (1LL << size) - 1;                                 \
>      filter = filter << pos;                                     \
>      temprs = (rs << pos) & filter;                              \
>      temprt = rt & ~filter;                                      \

This section of code is pretty much hand-coding
temp = deposit64(rt, pos, size, rs);
isn't it?

-- PMM
Petar Jovanovic May 13, 2013, 12:17 a.m. UTC | #3
True. Nice catch. This will simplify this section even more.
I will send an updated patch with this change included.

Petar
diff mbox

Patch

diff --git a/target-mips/dsp_helper.c b/target-mips/dsp_helper.c
index 9212789..426a3b6 100644
--- a/target-mips/dsp_helper.c
+++ b/target-mips/dsp_helper.c
@@ -2900,11 +2900,12 @@  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;                               \
+    uint32_t const sizefilter = 0x3F;                           \
     target_ulong filter;                                        \
     target_ulong temp, temprs, temprt;                          \
     target_ulong dspc;                                          \
@@ -2921,7 +2922,7 @@  target_ulong helper_##name(CPUMIPSState *env, target_ulong rs,  \
         return rt;                                              \
     }                                                           \
                                                                 \
-    filter = ((int64_t)0x01 << size) - 1;                       \
+    filter = (1LL << size) - 1;                                 \
     filter = filter << pos;                                     \
     temprs = (rs << pos) & filter;                              \
     temprt = rt & ~filter;                                      \
@@ -2930,9 +2931,9 @@  target_ulong helper_##name(CPUMIPSState *env, target_ulong 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