Re: [Qemu-devel] [PATCH v2 5/8] target-mips: use DSP unions for binary DSP operators

[Aurelien Jarno]

On Wed, Jan 09, 2013 at 09:16:29PM +0000, Blue Swirl wrote:
> On Wed, Jan 9, 2013 at 3:27 PM, Aurelien Jarno <address@hidden> wrote:
> > This allow to reduce the number of macros.
> >
> > Signed-off-by: Aurelien Jarno <address@hidden>
> > ---
> >  target-mips/dsp_helper.c |  384 
> > ++++++++++++++--------------------------------
> >  1 file changed, 116 insertions(+), 268 deletions(-)
> >
> > diff --git a/target-mips/dsp_helper.c b/target-mips/dsp_helper.c
> > index aed4c63..e01c8a9 100644
> > --- a/target-mips/dsp_helper.c
> > +++ b/target-mips/dsp_helper.c
> > @@ -1078,7 +1078,6 @@ static inline int32_t mipsdsp_cmpu_lt(uint32_t a, 
> > uint32_t b)
> >          b = num & MIPSDSP_LO;               \
> >      } while (0)
> >
> > -#define MIPSDSP_RETURN32(a)             ((target_long)(int32_t)a)
> >  #define MIPSDSP_RETURN32_8(a, b, c, d)  ((target_long)(int32_t) \
> >                                           (((uint32_t)a << 24) | \
> >                                           (((uint32_t)b << 16) | \
> > @@ -1111,119 +1110,127 @@ static inline int32_t mipsdsp_cmpu_lt(uint32_t a, 
> > uint32_t b)
> >  #endif
> >
> >  /** DSP Arithmetic Sub-class insns **/
> > -#define ARITH_PH(name, func)                                      \
> > -target_ulong helper_##name##_ph(target_ulong rs, target_ulong rt) \
> > -{                                                                 \
> > -    uint16_t  rsh, rsl, rth, rtl, temph, templ;                   \
> > -                                                                  \
> > -    MIPSDSP_SPLIT32_16(rs, rsh, rsl);                             \
> > -    MIPSDSP_SPLIT32_16(rt, rth, rtl);                             \
> > -                                                                  \
> > -    temph = mipsdsp_##func(rsh, rth);                             \
> > -    templ = mipsdsp_##func(rsl, rtl);                             \
> > -                                                                  \
> > -    return MIPSDSP_RETURN32_16(temph, templ);                     \
> > -}
> > -
> > -#define ARITH_PH_ENV(name, func)                                  \
> > -target_ulong helper_##name##_ph(target_ulong rs, target_ulong rt, \
> > -                                CPUMIPSState *env)                \
> > -{                                                                 \
> > -    uint16_t  rsh, rsl, rth, rtl, temph, templ;                   \
> > -                                                                  \
> > -    MIPSDSP_SPLIT32_16(rs, rsh, rsl);                             \
> > -    MIPSDSP_SPLIT32_16(rt, rth, rtl);                             \
> > -                                                                  \
> > -    temph = mipsdsp_##func(rsh, rth, env);                        \
> > -    templ = mipsdsp_##func(rsl, rtl, env);                        \
> > -                                                                  \
> > -    return MIPSDSP_RETURN32_16(temph, templ);                     \
> > -}
> > -
> > -
> > -ARITH_PH_ENV(addq, add_i16);
> > -ARITH_PH_ENV(addq_s, sat_add_i16);
> > -ARITH_PH_ENV(addu, add_u16);
> > -ARITH_PH_ENV(addu_s, sat_add_u16);
> > -
> > -ARITH_PH(addqh, rshift1_add_q16);
> > -ARITH_PH(addqh_r, rrshift1_add_q16);
> > -
> > -ARITH_PH_ENV(subq, sub_i16);
> > -ARITH_PH_ENV(subq_s, sat16_sub);
> > -ARITH_PH_ENV(subu, sub_u16_u16);
> > -ARITH_PH_ENV(subu_s, satu16_sub_u16_u16);
> > -
> > -ARITH_PH(subqh, rshift1_sub_q16);
> > -ARITH_PH(subqh_r, rrshift1_sub_q16);
> > -
> > -#undef ARITH_PH
> > -#undef ARITH_PH_ENV
> > +#define MIPSDSP32_BINOP(name, func, element)                               
> > \
> > +target_ulong helper_##name(target_ulong rs, target_ulong rt)               
> > \
> > +{                                                                          
> > \
> > +    DSP32Value ds, dt;                                                     
> > \
> > +    unsigned int i, n;                                                     
> > \
> > +                                                                           
> > \
> > +    n = sizeof(DSP32Value) / sizeof(ds.element[0]);                        
> > \
> > +    ds.sw[0] = rs;                                                         
> > \
> > +    dt.sw[0] = rt;                                                         
> > \
> > +                                                                           
> > \
> > +    for (i = 0 ; i < n ; i++) {                                            
> > \
> 
> There's an extra space before ';', please remove. Also in the other
> for loops below.

It is not something I can find in CODING_STYLE, and it is also not
caught by checkpatch.pl.

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