Re: [Qemu-ppc] [Qemu-devel] [PATCH] target/ppc: Fix carry flag setting for shift algebraic instructions

[David Gibson]

On Mon, Oct 02, 2017 at 08:19:10AM -0400, Richard Henderson wrote:
> On 09/30/2017 07:36 AM, Sandipan Das wrote:
> > For POWER ISA v3.0, the XER bit CA32 needs to be set by the shift
> > right algebraic instructions whenever the CA bit is to be set. This
> > change affects the following instructions:
> >   * Shift Right Algebraic Word (sraw[.])
> >   * Shift Right Algebraic Word Immediate (srawi[.])
> >   * Shift Right Algebraic Doubleword (srad[.])
> >   * Shift Right Algebraic Doubleword Immediate (sradi[.])
> > 
> > Signed-off-by: Sandipan Das <address@hidden>
> > ---
> >  target/ppc/helper.h     |  4 ++--
> >  target/ppc/int_helper.c | 10 ++++++++--
> >  target/ppc/translate.c  | 16 ++++++++++++++--
> >  3 files changed, 24 insertions(+), 6 deletions(-)
> > 
> > diff --git a/target/ppc/helper.h b/target/ppc/helper.h
> > index bb6a94a8b3..069d65ad7b 100644
> > --- a/target/ppc/helper.h
> > +++ b/target/ppc/helper.h
> > @@ -40,12 +40,12 @@ DEF_HELPER_4(divwe, tl, env, tl, tl, i32)
> >  
> >  DEF_HELPER_FLAGS_1(popcntb, TCG_CALL_NO_RWG_SE, tl, tl)
> >  DEF_HELPER_FLAGS_2(cmpb, TCG_CALL_NO_RWG_SE, tl, tl, tl)
> > -DEF_HELPER_3(sraw, tl, env, tl, tl)
> > +DEF_HELPER_4(sraw, tl, env, tl, tl, tl)
> >  #if defined(TARGET_PPC64)
> >  DEF_HELPER_FLAGS_2(cmpeqb, TCG_CALL_NO_RWG_SE, i32, tl, tl)
> >  DEF_HELPER_FLAGS_1(popcntw, TCG_CALL_NO_RWG_SE, tl, tl)
> >  DEF_HELPER_FLAGS_2(bpermd, TCG_CALL_NO_RWG_SE, i64, i64, i64)
> > -DEF_HELPER_3(srad, tl, env, tl, tl)
> > +DEF_HELPER_4(srad, tl, env, tl, tl, tl)
> >  DEF_HELPER_0(darn32, tl)
> >  DEF_HELPER_0(darn64, tl)
> >  #endif
> > diff --git a/target/ppc/int_helper.c b/target/ppc/int_helper.c
> > index da4e1a62c9..4f270eb49d 100644
> > --- a/target/ppc/int_helper.c
> > +++ b/target/ppc/int_helper.c
> > @@ -210,7 +210,7 @@ target_ulong helper_cmpb(target_ulong rs, target_ulong 
> > rb)
> >  
> >  /* shift right arithmetic helper */
> >  target_ulong helper_sraw(CPUPPCState *env, target_ulong value,
> > -                         target_ulong shift)
> > +                         target_ulong shift, target_ulong is_isa300)
> >  {
> >      int32_t ret;
> >  
> > @@ -231,12 +231,15 @@ target_ulong helper_sraw(CPUPPCState *env, 
> > target_ulong value,
> >          ret = (int32_t)value >> 31;
> >          env->ca = (ret != 0);
> >      }
> > +    if (is_isa300) {
> > +        env->ca32 = env->ca;
> > +    }
> 
> You do not need to pass a new parameter, because ca32 does not exist prior to
> ISA 3.00, and thus any code running ISA 2.x does not care about its value.
> 
> You only need set ca32 when ca is set.  Move those assignments
> adjacent.

Ah, good point.

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature