diff mbox

target-mips: remove wrong checks for recip.fmt and rsqrt.fmt

Message ID 1439919329-98555-1-git-send-email-petar.jovanovic@rt-rk.com
State New
Headers show

Commit Message

Petar Jovanovic Aug. 18, 2015, 5:35 p.m. UTC
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.

Signed-off-by: Petar Jovanovic <petar.jovanovic@imgtec.com>
---
 target-mips/translate.c |    4 ----
 1 file changed, 4 deletions(-)

Comments

Leon Alrae Aug. 25, 2015, 5:57 p.m. UTC | #1
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
Petar Jovanovic Aug. 25, 2015, 10:40 p.m. UTC | #2
-----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
Leon Alrae Aug. 26, 2015, 11:53 a.m. UTC | #3
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 mbox

Patch

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();