Message ID | 1439919329-98555-1-git-send-email-petar.jovanovic@rt-rk.com |
---|---|
State | New |
Headers | show |
On 18/08/2015 18:35, Petar Jovanovic wrote: > From: Petar Jovanovic <petar.jovanovic@imgtec.com> > > Instructions recip.{s|d} and rsqrt.{s|d} do not require 64-bit FPU neither > they require any particular mode for its FPU. This patch removes the checks > that may break a program that uses these instructions. This seems to be correct starting from MIPS32R2, but I'm not sure about older cores. Do we really want to remove the restrictions for them as well? > @@ -9839,7 +9837,6 @@ static void gen_farith (DisasContext *ctx, enum fopcode op1, > opn = "movn.d"; > break; > case OPC_RECIP_D: > - check_cp1_64bitmode(ctx); I think this needs check_cp1_registers() now, i.e. check for odd fpu register access when Status.FR = 0. > { > TCGv_i64 fp0 = tcg_temp_new_i64(); > > @@ -9851,7 +9848,6 @@ static void gen_farith (DisasContext *ctx, enum fopcode op1, > opn = "recip.d"; > break; > case OPC_RSQRT_D: > - check_cp1_64bitmode(ctx); same Thanks, Leon
-----Original Message----- From: Leon Alrae [mailto:leon.alrae@imgtec.com] Sent: Tuesday, August 25, 2015 7:58 PM To: Petar Jovanovic <petar.jovanovic@rt-rk.com>; qemu-devel@nongnu.org Cc: petar.jovanovic@imgtec.com; aurelien@aurel32.net Subject: Re: [Qemu-devel] [PATCH] target-mips: remove wrong checks for recip.fmt and rsqrt.fmt On 18/08/2015 18:35, Petar Jovanovic wrote: > From: Petar Jovanovic <petar.jovanovic@imgtec.com> > > Instructions recip.{s|d} and rsqrt.{s|d} do not require 64-bit FPU > neither they require any particular mode for its FPU. This patch > removes the checks that may break a program that uses these instructions. > This seems to be correct starting from MIPS32R2, but I'm not sure about older cores. Do we really want to remove the restrictions for them as well? IMHO, this restriction is wrong. So, yes, I believe we should remove it. Whether we need to add a different restriction is a good question, but I am inclined to think we may not need any here. > @@ -9839,7 +9837,6 @@ static void gen_farith (DisasContext *ctx, enum fopcode op1, > opn = "movn.d"; > break; > case OPC_RECIP_D: > - check_cp1_64bitmode(ctx); > I think this needs check_cp1_registers() now, i.e. check for odd fpu register access when Status.FR = 0. This would raise a "reserved instruction" exception. I am not aware that any MIPS CPU implementation would throw an exception for e.g. "recip.d $f21,$f11" (let me know if that is not the case), and I do not think MIPS documentation obliges us to throw an exception either. We may do that to make this irregular case more transparent though. Regards, Petar
On 25/08/2015 23:40, Petar Jovanovic wrote: >> @@ -9839,7 +9837,6 @@ static void gen_farith (DisasContext *ctx, enum > fopcode op1, >> opn = "movn.d"; >> break; >> case OPC_RECIP_D: >> - check_cp1_64bitmode(ctx); > >> I think this needs check_cp1_registers() now, i.e. check for odd fpu > register access when Status.FR = 0. > > This would raise a "reserved instruction" exception. I am not aware that any > MIPS CPU implementation would throw an exception for e.g. "recip.d > $f21,$f11" (let me know if that is not the case), and I do not think MIPS > documentation obliges us to throw an exception either. MIPS documentation says that this operation is "UNPREDICTABLE" -- software can never depend on a result and in QEMU we usually raise RI in such cases in other *.D instructions which is quite handy (it usually indicates the "forgot to set Status.FR bit" bug in the guest). Leon
diff --git a/target-mips/translate.c b/target-mips/translate.c index 98cf72d..dcecfa0 100644 --- a/target-mips/translate.c +++ b/target-mips/translate.c @@ -9294,7 +9294,6 @@ static void gen_farith (DisasContext *ctx, enum fopcode op1, opn = "movn.s"; break; case OPC_RECIP_S: - check_cop1x(ctx); { TCGv_i32 fp0 = tcg_temp_new_i32(); @@ -9306,7 +9305,6 @@ static void gen_farith (DisasContext *ctx, enum fopcode op1, opn = "recip.s"; break; case OPC_RSQRT_S: - check_cop1x(ctx); { TCGv_i32 fp0 = tcg_temp_new_i32(); @@ -9839,7 +9837,6 @@ static void gen_farith (DisasContext *ctx, enum fopcode op1, opn = "movn.d"; break; case OPC_RECIP_D: - check_cp1_64bitmode(ctx); { TCGv_i64 fp0 = tcg_temp_new_i64(); @@ -9851,7 +9848,6 @@ static void gen_farith (DisasContext *ctx, enum fopcode op1, opn = "recip.d"; break; case OPC_RSQRT_D: - check_cp1_64bitmode(ctx); { TCGv_i64 fp0 = tcg_temp_new_i64();