From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45754) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XrSV1-0005FA-F4 for qemu-devel@nongnu.org; Thu, 20 Nov 2014 09:14:26 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XrSUt-0007o5-VR for qemu-devel@nongnu.org; Thu, 20 Nov 2014 09:14:19 -0500 Message-ID: <546DF731.2060303@suse.de> Date: Thu, 20 Nov 2014 15:14:09 +0100 From: Alexander Graf MIME-Version: 1.0 References: <1415828764-10582-1-git-send-email-tommusta@gmail.com> <1415828764-10582-3-git-send-email-tommusta@gmail.com> In-Reply-To: <1415828764-10582-3-git-send-email-tommusta@gmail.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [2.3 V2 PATCH 2/6] target-ppc: Fix Floating Point Move Instructions That Set CR1 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Tom Musta , qemu-devel@nongnu.org, qemu-ppc@nongnu.org On 12.11.14 22:46, Tom Musta wrote: > The Floating Point Move instructions (fmr., fabs., fnabs., fneg., > and fcpsgn.) incorrectly copy FPSCR[FPCC] instead of [FX,FEX,VX,OX]. > Furthermore, the current code does this via a call to gen_compute_fprf, > which is awkward since these instructions do not actually set FPRF. > > Change the code to use the gen_set_cr1_from_fpscr utility. > > Signed-off-by: Tom Musta > --- > target-ppc/translate.c | 50 ++++++++++++++++++++++++++++------------------- > 1 files changed, 30 insertions(+), 20 deletions(-) > > diff --git a/target-ppc/translate.c b/target-ppc/translate.c > index 910ce56..2d79e39 100644 > --- a/target-ppc/translate.c > +++ b/target-ppc/translate.c > @@ -2077,6 +2077,21 @@ static void gen_srd(DisasContext *ctx) > } > #endif > > +#if defined(TARGET_PPC64) > +static void gen_set_cr1_from_fpscr(DisasContext *ctx) > +{ > + TCGv_i32 tmp = tcg_temp_new_i32(); > + tcg_gen_trunc_tl_i32(tmp, cpu_fpscr); > + tcg_gen_shri_i32(cpu_crf[1], tmp, 28); > + tcg_temp_free_i32(tmp); > +} > +#else > +static void gen_set_cr1_from_fpscr(DisasContext *ctx) > +{ > + tcg_gen_shri_tl(cpu_crf[1], cpu_fpscr, 28); > +} > +#endif > + > /*** Floating-Point arithmetic ***/ > #define _GEN_FLOAT_ACB(name, op, op1, op2, isfloat, set_fprf, type) \ > static void gen_f##name(DisasContext *ctx) \ > @@ -2370,7 +2385,9 @@ static void gen_fabs(DisasContext *ctx) > } > tcg_gen_andi_i64(cpu_fpr[rD(ctx->opcode)], cpu_fpr[rB(ctx->opcode)], > ~(1ULL << 63)); > - gen_compute_fprf(cpu_fpr[rD(ctx->opcode)], 0, Rc(ctx->opcode) != 0); > + if (unlikely(Rc(ctx->opcode))) { > + gen_set_cr1_from_fpscr(ctx); > + } I don't quite understand this. We set cr1 based on fpscr, but we don't recalculate the respective fpscr bits? Wouldn't we get outdated comparison data? Alex