From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33911) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XUycv-0001yI-DF for qemu-devel@nongnu.org; Fri, 19 Sep 2014 09:53:39 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XUycp-0002VX-3Y for qemu-devel@nongnu.org; Fri, 19 Sep 2014 09:53:33 -0400 Received: from mx1.redhat.com ([209.132.183.28]:49929) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XUyco-0002Tr-Qr for qemu-devel@nongnu.org; Fri, 19 Sep 2014 09:53:27 -0400 Message-ID: <541C354A.2040109@redhat.com> Date: Fri, 19 Sep 2014 15:53:14 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1410793421-6453-1-git-send-email-pbonzini@redhat.com> <1410793421-6453-12-git-send-email-pbonzini@redhat.com> <541B3FD7.3090605@gmail.com> In-Reply-To: <541B3FD7.3090605@gmail.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH 11/14] ppc: store CR registers in 32 1-bit registers List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Tom Musta , qemu-devel@nongnu.org Cc: agraf@suse.de Il 18/09/2014 22:25, Tom Musta ha scritto: > This breaks what you did in patch 5, which used LE bit numbering to > perform shifts. Yeah, I change "1 << x" to "8 >> x" in this patch for the fcmp helpers, but not the others. > And it breaks other code that uses the old LE > convention. I'll fix it like this: git diff target-ppc/fpu_helper.c diff --git a/target-ppc/fpu_helper.c b/target-ppc/fpu_helper.c index da93d12..06e4559 100644 --- a/target-ppc/fpu_helper.c +++ b/target-ppc/fpu_helper.c @@ -1018,32 +1018,32 @@ uint32_t helper_ftdiv(uint64_t fra, uint64_t frb) if (unlikely(float64_is_infinity(fra) || float64_is_infinity(frb) || float64_is_zero(frb))) { - fe_flag = 1; - fg_flag = 1; + fe_flag = 8 >> CRF_EQ; + fg_flag = 8 >> CRF_GT; } else { int e_a = ppc_float64_get_unbiased_exp(fra); int e_b = ppc_float64_get_unbiased_exp(frb); if (unlikely(float64_is_any_nan(fra) || float64_is_any_nan(frb))) { - fe_flag = 1; + fe_flag = 8 >> CRF_EQ; } else if ((e_b <= -1022) || (e_b >= 1021)) { - fe_flag = 1; + fe_flag = 8 >> CRF_EQ; } else if (!float64_is_zero(fra) && (((e_a - e_b) >= 1023) || ((e_a - e_b) <= -1021) || (e_a <= -970))) { - fe_flag = 1; + fe_flag = 8 >> CRF_EQ; } if (unlikely(float64_is_zero_or_denormal(frb))) { /* XB is not zero because of the above check and */ /* so must be denormalized. */ - fg_flag = 1; + fg_flag = 8 >> CRF_GT; } } - return 0x8 | (fg_flag ? 4 : 0) | (fe_flag ? 2 : 0); + return (8 >> CRF_LT) | fg_flag | fe_flag; } uint32_t helper_ftsqrt(uint64_t frb) and similarly for ftsqrt. Paolo > There are some other places in helper where env->crf[*] was still being set. Here are the ones that I found: > > diff --git a/target-ppc/fpu_helper.c b/target-ppc/fpu_helper.c > index 3f656e5..e624f97 100644 > --- a/target-ppc/fpu_helper.c > +++ b/target-ppc/fpu_helper.c > @@ -2141,7 +2141,8 @@ void helper_##op(CPUPPCState *env, uint32_t opcode) \ > } \ > } \ > \ > - env->crf[BF(opcode)] = 0x8 | (fg_flag ? 4 : 0) | (fe_flag ? 2 : 0); \ > + ppc_set_crf(env, BF(opcode), \ > + 0x8 | (fg_flag ? 4 : 0) | (fe_flag ? 2 : 0)); \ > } > > VSX_TDIV(xstdivdp, 1, float64, VsrD(0), -1022, 1023, 52) > @@ -2195,7 +2196,8 @@ void helper_##op(CPUPPCState *env, uint32_t opcode) \ > } \ > } \ > \ > - env->crf[BF(opcode)] = 0x8 | (fg_flag ? 4 : 0) | (fe_flag ? 2 : 0); \ > + ppc_set_crf(env, BF(opcode), \ > + 0x8 | (fg_flag ? 4 : 0) | (fe_flag ? 2 : 0)); \ > } > > VSX_TSQRT(xstsqrtdp, 1, float64, VsrD(0), -1022, 52) > @@ -2358,7 +2360,7 @@ void helper_##op(CPUPPCState *env, uint32_t opcode) \ > \ > env->fpscr &= ~(0x0F << FPSCR_FPRF); \ > env->fpscr |= cc << FPSCR_FPRF; \ > - env->crf[BF(opcode)] = cc; \ > + ppc_set_crf(env, BF(opcode), cc); \ > \ > helper_float_check_status(env); \ > } > @@ -2450,7 +2452,8 @@ void helper_##op(CPUPPCState *env, uint32_t opcode) \ > \ > putVSR(xT(opcode), &xt, env); \ > if ((opcode >> (31-21)) & 1) { \ > - env->crf[6] = (all_true ? 0x8 : 0) | (all_false ? 0x2 : 0); \ > + ppc_set_crf(env, 6, \ > + (all_true ? 0x8 : 0) | (all_false ? 0x2 : 0)); \ > } \ > helper_float_check_status(env); \ > } > > > > Note that I do not have the capability of testing any of the SPE instructions. >