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

Sandipan Das posted 1 patch 6 years, 5 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20170930113610.28608-1-sandipan@linux.vnet.ibm.com
Test checkpatch passed
Test docker passed
Test s390x passed
There is a newer version of this series
target/ppc/helper.h     |  4 ++--
target/ppc/int_helper.c | 10 ++++++++--
target/ppc/translate.c  | 16 ++++++++++++++--
3 files changed, 24 insertions(+), 6 deletions(-)
[Qemu-devel] [PATCH] target/ppc: Fix carry flag setting for shift algebraic instructions
Posted by Sandipan Das 6 years, 5 months ago
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 <sandipan@linux.vnet.ibm.com>
---
 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;
+    }
     return (target_long)ret;
 }
 
 #if defined(TARGET_PPC64)
 target_ulong helper_srad(CPUPPCState *env, target_ulong value,
-                         target_ulong shift)
+                         target_ulong shift, target_ulong is_isa300)
 {
     int64_t ret;
 
@@ -257,6 +260,9 @@ target_ulong helper_srad(CPUPPCState *env, target_ulong value,
         ret = (int64_t)value >> 63;
         env->ca = (ret != 0);
     }
+    if (is_isa300) {
+        env->ca32 = env->ca;
+    }
     return ret;
 }
 #endif
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 606b605ba0..9b39f51447 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -2166,8 +2166,11 @@ static void gen_slw(DisasContext *ctx)
 /* sraw & sraw. */
 static void gen_sraw(DisasContext *ctx)
 {
+    TCGv t0;
+
+    t0 = tcg_const_tl(is_isa300(ctx));
     gen_helper_sraw(cpu_gpr[rA(ctx->opcode)], cpu_env,
-                    cpu_gpr[rS(ctx->opcode)], cpu_gpr[rB(ctx->opcode)]);
+                    cpu_gpr[rS(ctx->opcode)], cpu_gpr[rB(ctx->opcode)], t0);
     if (unlikely(Rc(ctx->opcode) != 0))
         gen_set_Rc0(ctx, cpu_gpr[rA(ctx->opcode)]);
 }
@@ -2192,6 +2195,9 @@ static void gen_srawi(DisasContext *ctx)
         tcg_gen_setcondi_tl(TCG_COND_NE, cpu_ca, cpu_ca, 0);
         tcg_gen_sari_tl(dst, dst, sh);
     }
+    if (is_isa300(ctx)) {
+        tcg_gen_mov_tl(cpu_ca32, cpu_ca);
+    }
     if (unlikely(Rc(ctx->opcode) != 0)) {
         gen_set_Rc0(ctx, dst);
     }
@@ -2245,8 +2251,11 @@ static void gen_sld(DisasContext *ctx)
 /* srad & srad. */
 static void gen_srad(DisasContext *ctx)
 {
+    TCGv t0;
+
+    t0 = tcg_const_tl(is_isa300(ctx));
     gen_helper_srad(cpu_gpr[rA(ctx->opcode)], cpu_env,
-                    cpu_gpr[rS(ctx->opcode)], cpu_gpr[rB(ctx->opcode)]);
+                    cpu_gpr[rS(ctx->opcode)], cpu_gpr[rB(ctx->opcode)], t0);
     if (unlikely(Rc(ctx->opcode) != 0))
         gen_set_Rc0(ctx, cpu_gpr[rA(ctx->opcode)]);
 }
@@ -2269,6 +2278,9 @@ static inline void gen_sradi(DisasContext *ctx, int n)
         tcg_gen_setcondi_tl(TCG_COND_NE, cpu_ca, cpu_ca, 0);
         tcg_gen_sari_tl(dst, src, sh);
     }
+    if (is_isa300(ctx)) {
+        tcg_gen_mov_tl(cpu_ca32, cpu_ca);
+    }
     if (unlikely(Rc(ctx->opcode) != 0)) {
         gen_set_Rc0(ctx, dst);
     }
-- 
2.13.5


Re: [Qemu-devel] [PATCH] target/ppc: Fix carry flag setting for shift algebraic instructions
Posted by Richard Henderson 6 years, 5 months ago
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 <sandipan@linux.vnet.ibm.com>
> ---
>  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.


r~

Re: [Qemu-devel] [PATCH] target/ppc: Fix carry flag setting for shift algebraic instructions
Posted by David Gibson 6 years, 5 months ago
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 <sandipan@linux.vnet.ibm.com>
> > ---
> >  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