Message ID | alpine.DEB.1.10.1411120328290.2881@tp.orcam.me.uk |
---|---|
State | New |
Headers | show |
Hi Maciej, Please consistently use "target-mips: " when that's what you're touching. (For hw/mips/ it's less consistent what to use.) Leon, please sanitize subjects before sending them out, it makes them easier to skim in git-log and cgit. Thanks, Andreas
On Wed, 12 Nov 2014, Andreas Färber wrote: > Please consistently use "target-mips: " when that's what you're > touching. (For hw/mips/ it's less consistent what to use.) Sure. What about MIPS changes that span files contained within target-mips/ and elsewhere? I have such changes in my queue. Maciej
On 12/11/2014 18:46, Maciej W. Rozycki wrote: > On Wed, 12 Nov 2014, Andreas Färber wrote: > >> Please consistently use "target-mips: " when that's what you're >> touching. (For hw/mips/ it's less consistent what to use.) > > Sure. What about MIPS changes that span files contained within > target-mips/ and elsewhere? I have such changes in my queue. It might be a good idea to split these changes into separate patches to have more precise indication about touched subsystem (even though all the changes were done in MIPS context). For example "target-mips" and "linux-user" rather than just "mips". Regards, Leon
On Thu, 13 Nov 2014, Leon Alrae wrote: > It might be a good idea to split these changes into separate patches to > have more precise indication about touched subsystem (even though all > the changes were done in MIPS context). For example "target-mips" and > "linux-user" rather than just "mips". I split changes as applicable wherever I can, but at least some of these changes cannot be split without losing the sense of the change or even without causing compilation breakage. Well, I plan to submit those last, so it'll be a while yet and we can think about it when the time comes. Thanks for your input. Maciej
On 12/11/2014 15:21, Maciej W. Rozycki wrote: > Fix microMIPS MOVE16 and MOVEP instructions on 64-bit processors by > using register addition operations. > > This copies the approach taken with MIPS16 MOVE instructions (I8_MOV32R > and I8_MOVR32 opcodes) and follows the observation that OPC_ADDU expands > to tcg_gen_mov_tl whenever `rt' is 0 and `rs' is not, therefore copying > `rs' to `rd' verbatim. This is not the case with OPC_ADDIU where a > sign-extension from bit #31 is made, unless in the uninteresting case of > `rs' being 0, losing the upper 32 bits of the value copied for any > proper 64-bit values. > > This also serves as an optimization as one op is produced in generated > code rather than two (again, unless `rs' is 0, where it doesn't change > anything). > > Signed-off-by: Maciej W. Rozycki <macro@codesourcery.com> > --- > This is rather obvious, but I also pushed it through full bare-iron GCC > regression testing with an o32 big-endian microMIPS multilib. That > includes several instances of both instructions. No changes in results > were observed with the patch applied compared to the original version. > > I wonder if all these move operations shouldn't actually be switched to > using OPC_OR that is agnostic to the machine word size regardless of the > operand selection. But that is something to consider separately. > > So meanwhile, please apply. > > Maciej > > qemu-umips-move.diff > Index: qemu-git-trunk/target-mips/translate.c > =================================================================== > --- qemu-git-trunk.orig/target-mips/translate.c 2014-11-02 17:57:16.998924336 +0000 > +++ qemu-git-trunk/target-mips/translate.c 2014-11-02 17:57:19.498930155 +0000 > @@ -13492,8 +13492,8 @@ static int decode_micromips_opc (CPUMIPS > rs = rs_rt_enc[enc_rs]; > rt = rs_rt_enc[enc_rt]; > > - gen_arith_imm(ctx, OPC_ADDIU, rd, rs, 0); > - gen_arith_imm(ctx, OPC_ADDIU, re, rt, 0); > + gen_arith(ctx, OPC_ADDU, rd, rs, 0); > + gen_arith(ctx, OPC_ADDU, re, rt, 0); > } > break; > case LBU16: > @@ -13574,7 +13574,7 @@ static int decode_micromips_opc (CPUMIPS > int rd = uMIPS_RD5(ctx->opcode); > int rs = uMIPS_RS5(ctx->opcode); > > - gen_arith_imm(ctx, OPC_ADDIU, rd, rs, 0); > + gen_arith(ctx, OPC_ADDU, rd, rs, 0); > } > break; > case ANDI16: Reviewed-by: Leon Alrae <leon.alrae@imgtec.com> All the patches up to this one have been applied to mips-next branch (available at git://github.com/lalrae/qemu.git), thanks. I'll go through the remaining soon. Regards, Leon
On Mon, 24 Nov 2014, Leon Alrae wrote: > All the patches up to this one have been applied to mips-next branch > (available at git://github.com/lalrae/qemu.git), thanks. I'll go through > the remaining soon. Thanks. I am now back from a week's vacation and will continue posting outstanding changes. There will be changes made to generic code, specifically soft-float, and consequently other platforms' code, to suit the MIPS implementation of IEEE 754-2008 NaN handling recommendation. Regrettably I see the delivery of the 2.2 release has been delayed and I do hope the new estimate of Dec 5th will stand so that the changes can make their way to trunk in a timely manner. I have some other stuff beyond 2008-NaN support as well, but I'll be giving the latter a priority as I know you look forward to seeing it and the rest is valuable, but a bit less important (and furthermore it relies on some changes to ABI configuration that we may need to discuss before we find a satisfactory solution). Maciej
Index: qemu-git-trunk/target-mips/translate.c =================================================================== --- qemu-git-trunk.orig/target-mips/translate.c 2014-11-02 17:57:16.998924336 +0000 +++ qemu-git-trunk/target-mips/translate.c 2014-11-02 17:57:19.498930155 +0000 @@ -13492,8 +13492,8 @@ static int decode_micromips_opc (CPUMIPS rs = rs_rt_enc[enc_rs]; rt = rs_rt_enc[enc_rt]; - gen_arith_imm(ctx, OPC_ADDIU, rd, rs, 0); - gen_arith_imm(ctx, OPC_ADDIU, re, rt, 0); + gen_arith(ctx, OPC_ADDU, rd, rs, 0); + gen_arith(ctx, OPC_ADDU, re, rt, 0); } break; case LBU16: @@ -13574,7 +13574,7 @@ static int decode_micromips_opc (CPUMIPS int rd = uMIPS_RD5(ctx->opcode); int rs = uMIPS_RS5(ctx->opcode); - gen_arith_imm(ctx, OPC_ADDIU, rd, rs, 0); + gen_arith(ctx, OPC_ADDU, rd, rs, 0); } break; case ANDI16:
Fix microMIPS MOVE16 and MOVEP instructions on 64-bit processors by using register addition operations. This copies the approach taken with MIPS16 MOVE instructions (I8_MOV32R and I8_MOVR32 opcodes) and follows the observation that OPC_ADDU expands to tcg_gen_mov_tl whenever `rt' is 0 and `rs' is not, therefore copying `rs' to `rd' verbatim. This is not the case with OPC_ADDIU where a sign-extension from bit #31 is made, unless in the uninteresting case of `rs' being 0, losing the upper 32 bits of the value copied for any proper 64-bit values. This also serves as an optimization as one op is produced in generated code rather than two (again, unless `rs' is 0, where it doesn't change anything). Signed-off-by: Maciej W. Rozycki <macro@codesourcery.com> --- This is rather obvious, but I also pushed it through full bare-iron GCC regression testing with an o32 big-endian microMIPS multilib. That includes several instances of both instructions. No changes in results were observed with the patch applied compared to the original version. I wonder if all these move operations shouldn't actually be switched to using OPC_OR that is agnostic to the machine word size regardless of the operand selection. But that is something to consider separately. So meanwhile, please apply. Maciej qemu-umips-move.diff