From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55652) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aP0Kc-0000co-OR for qemu-devel@nongnu.org; Thu, 28 Jan 2016 23:06:48 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aP0Kb-0002lU-6L for qemu-devel@nongnu.org; Thu, 28 Jan 2016 23:06:46 -0500 Date: Fri, 29 Jan 2016 14:17:17 +1100 From: David Gibson Message-ID: <20160129031717.GJ23043@voom.redhat.com> References: <1453650086-92705-1-git-send-email-jrtc27@jrtc27.com> <1453650086-92705-3-git-send-email-jrtc27@jrtc27.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="2fjX3cMESU3XgGmZ" Content-Disposition: inline In-Reply-To: <1453650086-92705-3-git-send-email-jrtc27@jrtc27.com> Subject: Re: [Qemu-devel] [Qemu-ppc] [PATCH 2/2] target-ppc: mcrfs should always update FEX/VX and only clear exception bits List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: James Clarke Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org --2fjX3cMESU3XgGmZ Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sun, Jan 24, 2016 at 03:41:26PM +0000, James Clarke wrote: > Signed-off-by: James Clarke So, first, for a patch making a subtle behavioural change like this a detailed commit message is absolutely essential. In this case I can take the description from 0/2, but in future please include rationale's like that in the individual patches, so they'll appear in the git history without extra work on my part. But.. there's a more serious bug here, so I've backed this out of ppc-for-2.6... [snip] > @@ -2501,17 +2501,24 @@ static void gen_mcrfs(DisasContext *ctx) > { > TCGv tmp =3D tcg_temp_new(); > int bfa; > + int nibble; > + int shift; > =20 > if (unlikely(!ctx->fpu_enabled)) { > gen_exception(ctx, POWERPC_EXCP_FPU); > return; > } > - bfa =3D 4 * (7 - crfS(ctx->opcode)); > - tcg_gen_shri_tl(tmp, cpu_fpscr, bfa); > + bfa =3D crfS(ctx->opcode); > + nibble =3D 7 - bfa; > + shift =3D 4 * nibble; > + tcg_gen_shri_tl(tmp, cpu_fpscr, shift); > tcg_gen_trunc_tl_i32(cpu_crf[crfD(ctx->opcode)], tmp); > - tcg_temp_free(tmp); > tcg_gen_andi_i32(cpu_crf[crfD(ctx->opcode)], cpu_crf[crfD(ctx->opcod= e)], 0xf); > - tcg_gen_andi_tl(cpu_fpscr, cpu_fpscr, ~(0xF << bfa)); > + /* Only the exception bits (including FX) should be cleared if read = */ > + tcg_gen_andi_tl(tmp, cpu_fpscr, ~((0xF << shift) & FP_EX_CLEAR_BITS)= ); > + /* FEX and VX need to be updated, so don't set fpscr directly */ > + gen_helper_store_fpscr(cpu_env, tmp, 1 << nibble); This doesn't compile. For 64-bit targets we get: CC ppc64-softmmu/target-ppc/translate.o /home/dwg/src/qemu/target-ppc/translate.c: In function =E2=80=98gen_mcrfs= =E2=80=99: /home/dwg/src/qemu/target-ppc/translate.c:2520:42: error: passing argument = 3 of =E2=80=98gen_helper_store_fpscr=E2=80=99 makes pointe r from integer without a cast [-Werror=3Dint-conversion] gen_helper_store_fpscr(cpu_env, tmp, 1 << nibble); ^ In file included from /home/dwg/src/qemu/include/exec/helper-gen.h:59:0, from /home/dwg/src/qemu/tcg/tcg-op.h:27, from /home/dwg/src/qemu/target-ppc/translate.c:23: /home/dwg/src/qemu/target-ppc/helper.h:56:58: note: expected =E2=80=98TCGv_= i32 {aka struct TCGv_i32_d *}=E2=80=99 but argument is of=20 type =E2=80=98int=E2=80=99 For 32-bit targets it's worse: CC ppcemb-softmmu/target-ppc/translate.o /home/dwg/src/qemu/target-ppc/translate.c: In function =E2=80=98gen_mcrfs= =E2=80=99: /home/dwg/src/qemu/target-ppc/translate.c:2520:37: error: passing argument = 2 of =E2=80=98gen_helper_store_fpscr=E2=80=99 from incompa tible pointer type [-Werror=3Dincompatible-pointer-types] gen_helper_store_fpscr(cpu_env, tmp, 1 << nibble); ^ In file included from /home/dwg/src/qemu/include/exec/helper-gen.h:59:0, from /home/dwg/src/qemu/tcg/tcg-op.h:27, from /home/dwg/src/qemu/target-ppc/translate.c:23: /home/dwg/src/qemu/target-ppc/helper.h:56:58: note: expected =E2=80=98TCGv_= i64 {aka struct TCGv_i64_d *}=E2=80=99 but argument is of=20 type =E2=80=98TCGv_i32 {aka struct TCGv_i32_d *}=E2=80=99 /home/dwg/src/qemu/target-ppc/translate.c:2520:42: error: passing argument = 3 of =E2=80=98gen_helper_store_fpscr=E2=80=99 makes pointe r from integer without a cast [-Werror=3Dint-conversion] gen_helper_store_fpscr(cpu_env, tmp, 1 << nibble); ^ In file included from /home/dwg/src/qemu/include/exec/helper-gen.h:59:0, from /home/dwg/src/qemu/tcg/tcg-op.h:27, from /home/dwg/src/qemu/target-ppc/translate.c:23: /home/dwg/src/qemu/target-ppc/helper.h:56:58: note: expected =E2=80=98TCGv_= i32 {aka struct TCGv_i32_d *}=E2=80=99 but argument is of=20 type =E2=80=98int=E2=80=99 --=20 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 --2fjX3cMESU3XgGmZ Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJWqtm9AAoJEGw4ysog2bOSAIsP/RvfsypycO0tiQjFMktze163 e4/R3TfTqtZv3XaaDJIvBxiAMOSDXi4L1Q7am7Aa5ykvEj2xn53aIyOsmGP1R3OQ 0/k64/RkBpssxjj+3/bi5jWnrjwPEkiCkrrZ0wB5Ig00xkIxge9gXi53Xd8kUred Z+RYtVgO7Ymh1eMrzNtGkRIttHklmn0E3eOU6gRGVpl4gXXXJ9GgGRelChzmwM4s MlwkxLCdauDV9MM1unXRyvWZyDlOIjQ+1h9xQMaUglDFTpexY/All2qTk7KD+9cc 5qBWFA8lz/Krj6Dzg6XwVWyW3+mPQDqkOJbrqPqyDeFNLS4B11lluRJG2cUhHKQJ BurWblLmc9omz0tlNOZpuNTcRlPjq36U20ePlcIinrQ2wv3Dr+0PMWZrEk5EWRM1 SHugXnmRFMhPm8geABMtl6qH9Jlq9TYwwf70jmambgoCM9pyazcjtX98xuwhLx/I ErrIEzvIG7G7v5+Qdhfy2hQ2qjpND/EGKb3LF1VFXr7ChyjW2xmzwLU1qfupKkGA XfSvJeZwl03Eu2h7+rTcD/AumwYACOE7BjdIdGT/w7UVNvp/DRmxTKQJ1ExgTK89 8haWBUM13oqr+5VM4GayjeD473l11D2CcGafUCzaGi70NPZgrITA2SQR3uaaWKJg kM+Im4QN1ylKcoEk5TXb =JBy+ -----END PGP SIGNATURE----- --2fjX3cMESU3XgGmZ--