[PATCH 0/4] target/m68k: fix FPSR quotient byte for fmod and frem

[PATCH 0/4] target/m68k: fix FPSR quotient byte for fmod and frem

[Mark Cave-Ayland]

This series is taken from my MacOS branch and fixes setting the FPSR quotient
byte for the fmod and frem instructions which was causing the MacOS _Pack5
SANE trancendentals implementation to return incorrect values for sin() and
cos().

The first 2 patches update make_quotient() to take separate sign and value
parameters so that it can be used for both floatx80_modrem() and a manual
division calculation, whilst the last 2 patches fix the value of the quotient
byte for both the fmod and frem instructions. This series has been validated
against the fmod and frem results using a custom test program which has been
run on a real 68040 Mac.

Many thanks to the mac68k folk for their help debugging this, in particular
SolraBizna for narrowing down the issue to the transcendentals and writing
the custom test program, and also treellama and Cat7 for running the test
program on real hardware and uploading the generated results file for
further analysis.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


Mark Cave-Ayland (4):
  target/m68k: pass quotient directly into make_quotient()
  target/m68k: pass sign directly into make_quotient()
  target/m68k: fix FPSR quotient byte for fmod instruction
  target/m68k: fix FPSR quotient byte for frem instruction

 target/m68k/fpu_helper.c | 43 ++++++++++++++++++++++++----------------
 1 file changed, 26 insertions(+), 17 deletions(-)

-- 
2.30.2

[PATCH 1/4] target/m68k: pass quotient directly into make_quotient()

[Mark Cave-Ayland]

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 target/m68k/fpu_helper.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/target/m68k/fpu_helper.c b/target/m68k/fpu_helper.c
index fdc4937e29..0932c464fd 100644
--- a/target/m68k/fpu_helper.c
+++ b/target/m68k/fpu_helper.c
@@ -515,16 +515,10 @@ uint32_t HELPER(fmovemd_ld_postinc)(CPUM68KState *env, uint32_t addr,
     return fmovem_postinc(env, addr, mask, cpu_ld_float64_ra);
 }
 
-static void make_quotient(CPUM68KState *env, floatx80 val)
+static void make_quotient(CPUM68KState *env, int32_t quotient)
 {
-    int32_t quotient;
     int sign;
 
-    if (floatx80_is_any_nan(val)) {
-        return;
-    }
-
-    quotient = floatx80_to_int32(val, &env->fp_status);
     sign = quotient < 0;
     if (sign) {
         quotient = -quotient;
@@ -538,14 +532,22 @@ void HELPER(fmod)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1)
 {
     res->d = floatx80_mod(val1->d, val0->d, &env->fp_status);
 
-    make_quotient(env, res->d);
+    if (floatx80_is_any_nan(res->d)) {
+        return;
+    }
+
+    make_quotient(env, floatx80_to_int32(res->d, &env->fp_status));
 }
 
 void HELPER(frem)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1)
 {
     res->d = floatx80_rem(val1->d, val0->d, &env->fp_status);
 
-    make_quotient(env, res->d);
+    if (floatx80_is_any_nan(res->d)) {
+        return;
+    }
+
+    make_quotient(env, floatx80_to_int32(res->d, &env->fp_status));
 }
 
 void HELPER(fgetexp)(CPUM68KState *env, FPReg *res, FPReg *val)
-- 
2.30.2

[PATCH 2/4] target/m68k: pass sign directly into make_quotient()

[Mark Cave-Ayland]

This enables the quotient parameter to be changed from int32_t to uint32_t and
also allows the extra sign logic in make_quotient() to be removed.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 target/m68k/fpu_helper.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/target/m68k/fpu_helper.c b/target/m68k/fpu_helper.c
index 0932c464fd..ae839785fa 100644
--- a/target/m68k/fpu_helper.c
+++ b/target/m68k/fpu_helper.c
@@ -515,15 +515,8 @@ uint32_t HELPER(fmovemd_ld_postinc)(CPUM68KState *env, uint32_t addr,
     return fmovem_postinc(env, addr, mask, cpu_ld_float64_ra);
 }
 
-static void make_quotient(CPUM68KState *env, int32_t quotient)
+static void make_quotient(CPUM68KState *env, int sign, uint32_t quotient)
 {
-    int sign;
-
-    sign = quotient < 0;
-    if (sign) {
-        quotient = -quotient;
-    }
-
     quotient = (sign << 7) | (quotient & 0x7f);
     env->fpsr = (env->fpsr & ~FPSR_QT_MASK) | (quotient << FPSR_QT_SHIFT);
 }
@@ -536,7 +529,8 @@ void HELPER(fmod)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1)
         return;
     }
 
-    make_quotient(env, floatx80_to_int32(res->d, &env->fp_status));
+    make_quotient(env, extractFloatx80Sign(res->d),
+                  floatx80_to_int32(res->d, &env->fp_status));
 }
 
 void HELPER(frem)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1)
@@ -547,7 +541,8 @@ void HELPER(frem)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1)
         return;
     }
 
-    make_quotient(env, floatx80_to_int32(res->d, &env->fp_status));
+    make_quotient(env, extractFloatx80Sign(res->d),
+                  floatx80_to_int32(res->d, &env->fp_status));
 }
 
 void HELPER(fgetexp)(CPUM68KState *env, FPReg *res, FPReg *val)
-- 
2.30.2

[PATCH 3/4] target/m68k: fix FPSR quotient byte for fmod instruction

[Mark Cave-Ayland]

The FPSR quotient byte should be set to the value of the quotient and not the
result. Switch from using floatx80_mod() to floatx80_modrem() which returns
the quotient as a uint64_t which can be used for the quotient byte.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 target/m68k/fpu_helper.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/target/m68k/fpu_helper.c b/target/m68k/fpu_helper.c
index ae839785fa..18594a35af 100644
--- a/target/m68k/fpu_helper.c
+++ b/target/m68k/fpu_helper.c
@@ -523,14 +523,17 @@ static void make_quotient(CPUM68KState *env, int sign, uint32_t quotient)
 
 void HELPER(fmod)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1)
 {
-    res->d = floatx80_mod(val1->d, val0->d, &env->fp_status);
+    uint64_t quotient;
+    int sign = extractFloatx80Sign(val1->d) ^ extractFloatx80Sign(val0->d);
+
+    res->d = floatx80_modrem(val1->d, val0->d, true, &quotient,
+                             &env->fp_status);
 
     if (floatx80_is_any_nan(res->d)) {
         return;
     }
 
-    make_quotient(env, extractFloatx80Sign(res->d),
-                  floatx80_to_int32(res->d, &env->fp_status));
+    make_quotient(env, sign, quotient);
 }
 
 void HELPER(frem)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1)
-- 
2.30.2

[PATCH 4/4] target/m68k: fix FPSR quotient byte for frem instruction

[Mark Cave-Ayland]

The FPSR quotient byte should be set to the value of the quotient and not the
result. Manually calculate the quotient in the frem helper in round to nearest
even mode (note this is different from the quotient calculated internally for
fmod), and use it to set the quotient byte accordingly.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Fixes: https://gitlab.com/qemu-project/qemu/-/issues/1314
---
 target/m68k/fpu_helper.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/target/m68k/fpu_helper.c b/target/m68k/fpu_helper.c
index 18594a35af..ce6f2cd2b9 100644
--- a/target/m68k/fpu_helper.c
+++ b/target/m68k/fpu_helper.c
@@ -538,14 +538,23 @@ void HELPER(fmod)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1)
 
 void HELPER(frem)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1)
 {
+    FPReg fp_quot;
+    float_status fp_status;
+
+    /* Calculate quotient directly using round to nearest mode */
+    set_float_rounding_mode(float_round_nearest_even, &fp_status);
+    set_floatx80_rounding_precision(
+        get_floatx80_rounding_precision(&env->fp_status), &fp_status);
+    fp_quot.d = floatx80_div(val1->d, val0->d, &fp_status);
+
     res->d = floatx80_rem(val1->d, val0->d, &env->fp_status);
 
-    if (floatx80_is_any_nan(res->d)) {
+    if (floatx80_is_any_nan(fp_quot.d)) {
         return;
     }
 
-    make_quotient(env, extractFloatx80Sign(res->d),
-                  floatx80_to_int32(res->d, &env->fp_status));
+    make_quotient(env, extractFloatx80Sign(fp_quot.d),
+                  abs(floatx80_to_int32(fp_quot.d, &env->fp_status)));
 }
 
 void HELPER(fgetexp)(CPUM68KState *env, FPReg *res, FPReg *val)
-- 
2.30.2

Re: [PATCH 1/4] target/m68k: pass quotient directly into make_quotient()

[Laurent Vivier]

Le 01/01/2023 à 15:43, Mark Cave-Ayland a écrit :
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>   target/m68k/fpu_helper.c | 20 +++++++++++---------
>   1 file changed, 11 insertions(+), 9 deletions(-)
> 
>

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Re: [PATCH 2/4] target/m68k: pass sign directly into make_quotient()

[Laurent Vivier]

Le 01/01/2023 à 15:43, Mark Cave-Ayland a écrit :
> This enables the quotient parameter to be changed from int32_t to uint32_t and
> also allows the extra sign logic in make_quotient() to be removed.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>   target/m68k/fpu_helper.c | 15 +++++----------
>   1 file changed, 5 insertions(+), 10 deletions(-)
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Re: [PATCH 3/4] target/m68k: fix FPSR quotient byte for fmod instruction

[Laurent Vivier]

Le 01/01/2023 à 15:43, Mark Cave-Ayland a écrit :
> The FPSR quotient byte should be set to the value of the quotient and not the
> result. Switch from using floatx80_mod() to floatx80_modrem() which returns
> the quotient as a uint64_t which can be used for the quotient byte.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>   target/m68k/fpu_helper.c | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Re: [PATCH 2/4] target/m68k: pass sign directly into make_quotient()

[Laurent Vivier]

Le 01/01/2023 à 15:43, Mark Cave-Ayland a écrit :
> This enables the quotient parameter to be changed from int32_t to uint32_t and
> also allows the extra sign logic in make_quotient() to be removed.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>   target/m68k/fpu_helper.c | 15 +++++----------
>   1 file changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/target/m68k/fpu_helper.c b/target/m68k/fpu_helper.c
> index 0932c464fd..ae839785fa 100644
> --- a/target/m68k/fpu_helper.c
> +++ b/target/m68k/fpu_helper.c
> @@ -515,15 +515,8 @@ uint32_t HELPER(fmovemd_ld_postinc)(CPUM68KState *env, uint32_t addr,
>       return fmovem_postinc(env, addr, mask, cpu_ld_float64_ra);
>   }
>   
> -static void make_quotient(CPUM68KState *env, int32_t quotient)
> +static void make_quotient(CPUM68KState *env, int sign, uint32_t quotient)
>   {
> -    int sign;
> -
> -    sign = quotient < 0;
> -    if (sign) {
> -        quotient = -quotient;
> -    }
> -
>       quotient = (sign << 7) | (quotient & 0x7f);
>       env->fpsr = (env->fpsr & ~FPSR_QT_MASK) | (quotient << FPSR_QT_SHIFT);
>   }
> @@ -536,7 +529,8 @@ void HELPER(fmod)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1)
>           return;
>       }
>   
> -    make_quotient(env, floatx80_to_int32(res->d, &env->fp_status));
> +    make_quotient(env, extractFloatx80Sign(res->d),
> +                  floatx80_to_int32(res->d, &env->fp_status));
>   }
>   
>   void HELPER(frem)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1)
> @@ -547,7 +541,8 @@ void HELPER(frem)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1)
>           return;
>       }
>   
> -    make_quotient(env, floatx80_to_int32(res->d, &env->fp_status));
> +    make_quotient(env, extractFloatx80Sign(res->d),
> +                  floatx80_to_int32(res->d, &env->fp_status));
>   }
>   
>   void HELPER(fgetexp)(CPUM68KState *env, FPReg *res, FPReg *val)

I think you need an "abs(floatx80_to_int32())" in both cases as you do in PATCH 4

Thanks,
Laurent

Re: [PATCH 4/4] target/m68k: fix FPSR quotient byte for frem instruction

[Laurent Vivier]

Le 01/01/2023 à 15:43, Mark Cave-Ayland a écrit :
> The FPSR quotient byte should be set to the value of the quotient and not the
> result. Manually calculate the quotient in the frem helper in round to nearest
> even mode (note this is different from the quotient calculated internally for
> fmod), and use it to set the quotient byte accordingly.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Fixes: https://gitlab.com/qemu-project/qemu/-/issues/1314
> ---
>   target/m68k/fpu_helper.c | 15 ++++++++++++---
>   1 file changed, 12 insertions(+), 3 deletions(-)
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Re: [PATCH 2/4] target/m68k: pass sign directly into make_quotient()

[Richard Henderson]

On 1/1/23 09:26, Laurent Vivier wrote:
> Le 01/01/2023 à 15:43, Mark Cave-Ayland a écrit :
>> This enables the quotient parameter to be changed from int32_t to uint32_t and
>> also allows the extra sign logic in make_quotient() to be removed.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>>   target/m68k/fpu_helper.c | 15 +++++----------
>>   1 file changed, 5 insertions(+), 10 deletions(-)
>>
>> diff --git a/target/m68k/fpu_helper.c b/target/m68k/fpu_helper.c
>> index 0932c464fd..ae839785fa 100644
>> --- a/target/m68k/fpu_helper.c
>> +++ b/target/m68k/fpu_helper.c
>> @@ -515,15 +515,8 @@ uint32_t HELPER(fmovemd_ld_postinc)(CPUM68KState *env, uint32_t addr,
>>       return fmovem_postinc(env, addr, mask, cpu_ld_float64_ra);
>>   }
>> -static void make_quotient(CPUM68KState *env, int32_t quotient)
>> +static void make_quotient(CPUM68KState *env, int sign, uint32_t quotient)
>>   {
>> -    int sign;
>> -
>> -    sign = quotient < 0;
>> -    if (sign) {
>> -        quotient = -quotient;
>> -    }
>> -
>>       quotient = (sign << 7) | (quotient & 0x7f);
>>       env->fpsr = (env->fpsr & ~FPSR_QT_MASK) | (quotient << FPSR_QT_SHIFT);
>>   }
>> @@ -536,7 +529,8 @@ void HELPER(fmod)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg 
>> *val1)
>>           return;
>>       }
>> -    make_quotient(env, floatx80_to_int32(res->d, &env->fp_status));
>> +    make_quotient(env, extractFloatx80Sign(res->d),
>> +                  floatx80_to_int32(res->d, &env->fp_status));
>>   }
>>   void HELPER(frem)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1)
>> @@ -547,7 +541,8 @@ void HELPER(frem)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg 
>> *val1)
>>           return;
>>       }
>> -    make_quotient(env, floatx80_to_int32(res->d, &env->fp_status));
>> +    make_quotient(env, extractFloatx80Sign(res->d),
>> +                  floatx80_to_int32(res->d, &env->fp_status));
>>   }
>>   void HELPER(fgetexp)(CPUM68KState *env, FPReg *res, FPReg *val)
> 
> I think you need an "abs(floatx80_to_int32())" in both cases as you do in PATCH 4

Or in fact

     sign = extractFloatx80Sign(res);
     quot = floatx80_to_int32(floatx80_abs(res->d), status);
     make_quotient(env, sign, quot);


r~

Re: [PATCH 2/4] target/m68k: pass sign directly into make_quotient()

[Mark Cave-Ayland]

On 01/01/2023 17:26, Laurent Vivier wrote:

> Le 01/01/2023 à 15:43, Mark Cave-Ayland a écrit :
>> This enables the quotient parameter to be changed from int32_t to uint32_t and
>> also allows the extra sign logic in make_quotient() to be removed.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>>   target/m68k/fpu_helper.c | 15 +++++----------
>>   1 file changed, 5 insertions(+), 10 deletions(-)
>>
>> diff --git a/target/m68k/fpu_helper.c b/target/m68k/fpu_helper.c
>> index 0932c464fd..ae839785fa 100644
>> --- a/target/m68k/fpu_helper.c
>> +++ b/target/m68k/fpu_helper.c
>> @@ -515,15 +515,8 @@ uint32_t HELPER(fmovemd_ld_postinc)(CPUM68KState *env, 
>> uint32_t addr,
>>       return fmovem_postinc(env, addr, mask, cpu_ld_float64_ra);
>>   }
>> -static void make_quotient(CPUM68KState *env, int32_t quotient)
>> +static void make_quotient(CPUM68KState *env, int sign, uint32_t quotient)
>>   {
>> -    int sign;
>> -
>> -    sign = quotient < 0;
>> -    if (sign) {
>> -        quotient = -quotient;
>> -    }
>> -
>>       quotient = (sign << 7) | (quotient & 0x7f);
>>       env->fpsr = (env->fpsr & ~FPSR_QT_MASK) | (quotient << FPSR_QT_SHIFT);
>>   }
>> @@ -536,7 +529,8 @@ void HELPER(fmod)(CPUM68KState *env, FPReg *res, FPReg *val0, 
>> FPReg *val1)
>>           return;
>>       }
>> -    make_quotient(env, floatx80_to_int32(res->d, &env->fp_status));
>> +    make_quotient(env, extractFloatx80Sign(res->d),
>> +                  floatx80_to_int32(res->d, &env->fp_status));
>>   }
>>   void HELPER(frem)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1)
>> @@ -547,7 +541,8 @@ void HELPER(frem)(CPUM68KState *env, FPReg *res, FPReg *val0, 
>> FPReg *val1)
>>           return;
>>       }
>> -    make_quotient(env, floatx80_to_int32(res->d, &env->fp_status));
>> +    make_quotient(env, extractFloatx80Sign(res->d),
>> +                  floatx80_to_int32(res->d, &env->fp_status));
>>   }
>>   void HELPER(fgetexp)(CPUM68KState *env, FPReg *res, FPReg *val)
> 
> I think you need an "abs(floatx80_to_int32())" in both cases as you do in PATCH 4

Ah yes that's probably true - I suspect I didn't notice because the static tests fail 
immediately until patches 3 and 4.


ATB,

Mark.

Re: [PATCH 2/4] target/m68k: pass sign directly into make_quotient()

[Mark Cave-Ayland]

On 01/01/2023 18:53, Richard Henderson wrote:

> On 1/1/23 09:26, Laurent Vivier wrote:
>> Le 01/01/2023 à 15:43, Mark Cave-Ayland a écrit :
>>> This enables the quotient parameter to be changed from int32_t to uint32_t and
>>> also allows the extra sign logic in make_quotient() to be removed.
>>>
>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>> ---
>>>   target/m68k/fpu_helper.c | 15 +++++----------
>>>   1 file changed, 5 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/target/m68k/fpu_helper.c b/target/m68k/fpu_helper.c
>>> index 0932c464fd..ae839785fa 100644
>>> --- a/target/m68k/fpu_helper.c
>>> +++ b/target/m68k/fpu_helper.c
>>> @@ -515,15 +515,8 @@ uint32_t HELPER(fmovemd_ld_postinc)(CPUM68KState *env, 
>>> uint32_t addr,
>>>       return fmovem_postinc(env, addr, mask, cpu_ld_float64_ra);
>>>   }
>>> -static void make_quotient(CPUM68KState *env, int32_t quotient)
>>> +static void make_quotient(CPUM68KState *env, int sign, uint32_t quotient)
>>>   {
>>> -    int sign;
>>> -
>>> -    sign = quotient < 0;
>>> -    if (sign) {
>>> -        quotient = -quotient;
>>> -    }
>>> -
>>>       quotient = (sign << 7) | (quotient & 0x7f);
>>>       env->fpsr = (env->fpsr & ~FPSR_QT_MASK) | (quotient << FPSR_QT_SHIFT);
>>>   }
>>> @@ -536,7 +529,8 @@ void HELPER(fmod)(CPUM68KState *env, FPReg *res, FPReg *val0, 
>>> FPReg *val1)
>>>           return;
>>>       }
>>> -    make_quotient(env, floatx80_to_int32(res->d, &env->fp_status));
>>> +    make_quotient(env, extractFloatx80Sign(res->d),
>>> +                  floatx80_to_int32(res->d, &env->fp_status));
>>>   }
>>>   void HELPER(frem)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1)
>>> @@ -547,7 +541,8 @@ void HELPER(frem)(CPUM68KState *env, FPReg *res, FPReg *val0, 
>>> FPReg *val1)
>>>           return;
>>>       }
>>> -    make_quotient(env, floatx80_to_int32(res->d, &env->fp_status));
>>> +    make_quotient(env, extractFloatx80Sign(res->d),
>>> +                  floatx80_to_int32(res->d, &env->fp_status));
>>>   }
>>>   void HELPER(fgetexp)(CPUM68KState *env, FPReg *res, FPReg *val)
>>
>> I think you need an "abs(floatx80_to_int32())" in both cases as you do in PATCH 4
> 
> Or in fact
> 
>      sign = extractFloatx80Sign(res);
>      quot = floatx80_to_int32(floatx80_abs(res->d), status);
>      make_quotient(env, sign, quot);

Thanks for the suggestion. Just out of curiosity, how does moving the abs to before 
the integer conversion make a difference here? Is it because floatx80_to_int32() can 
fail in some circumstances because of the sign of the result?


ATB,

Mark.

Re: [PATCH 2/4] target/m68k: pass sign directly into make_quotient()

[Richard Henderson]

On 1/2/23 02:10, Mark Cave-Ayland wrote:
>>> I think you need an "abs(floatx80_to_int32())" in both cases as you do in PATCH 4
>>
>> Or in fact
>>
>>      sign = extractFloatx80Sign(res);
>>      quot = floatx80_to_int32(floatx80_abs(res->d), status);
>>      make_quotient(env, sign, quot);
> 
> Thanks for the suggestion. Just out of curiosity, how does moving the abs to before the 
> integer conversion make a difference here? Is it because floatx80_to_int32() can fail in 
> some circumstances because of the sign of the result?

It's a simple and operation on floats, instead of cmp+cmov on integers.

I think it's a touch clearer as well, having just saved the fp sign, you discard it before 
moving on to the next thing.


r~