Re: [Qemu-devel] [PATCH] fix fdiv instruction

[Programmingkid]

> On Jun 23, 2018, at 12:17 PM, Richard Henderson <address@hidden> wrote:
> 
> On 06/22/2018 07:22 PM, John Arbuckle wrote:
>> When the fdiv instruction divides a finite number by zero,
>> the result actually depends on the FPSCR[ZE] bit. If this
>> bit is set, the return value is zero. If it is not set
>> the result should be either positive or negative infinity.
>> The sign of this result would depend on the sign of the
>> two inputs. What currently happens is only infinity is
>> returned even if the FPSCR[ZE] bit is set. This patch
>> fixes this problem by actually checking the FPSCR[ZE] bit
>> when deciding what the answer should be.
>> 
>> fdiv is suppose to only set the FPSCR's FPRF bits during a
>> division by zero situation when the FPSCR[ZE] is not set.
>> What currently happens is these bits are always set. This
>> patch fixes this problem by checking the FPSCR[ZE] bit to
>> decide if the FPRF bits should be set. 
>> 
>> https://www.pdfdrive.net/powerpc-microprocessor-family-the-programming-environments-for-32-e3087633.html
>> This document has the information on the fdiv. Page 133 has the information 
>> on what action is executed when a division by zero situation takes place. 
>> 
>> Signed-off-by: John Arbuckle <address@hidden>
>> ---
>> target/ppc/fpu_helper.c            | 16 ++++++++++++++++
>> target/ppc/translate/fp-impl.inc.c | 28 +++++++++++++++++++++++++++-
>> 2 files changed, 43 insertions(+), 1 deletion(-)
>> 
>> diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
>> index 7714bfe0f9..de694604fb 100644
>> --- a/target/ppc/fpu_helper.c
>> +++ b/target/ppc/fpu_helper.c
>> @@ -658,6 +658,20 @@ uint64_t helper_fdiv(CPUPPCState *env, uint64_t arg1, 
>> uint64_t arg2)
>>     } else if (unlikely(float64_is_zero(farg1.d) && 
>> float64_is_zero(farg2.d))) {
>>         /* Division of zero by zero */
>>         farg1.ll = float_invalid_op_excp(env, POWERPC_EXCP_FP_VXZDZ, 1);
>> +    } else if (arg2 == 0) {
>> +        /* Division by zero */
>> +        float_zero_divide_excp(env, GETPC());
>> +        if (fpscr_ze) { /* if zero divide exception is enabled */
>> +            farg1.ll = 0;
>> +        } else {
>> +            uint64_t sign = (farg1.ll ^ farg2.ll) >> 63;
>> +            if (sign) { /* Negative sign bit */
>> +                farg1.ll = 0xfff0000000000000; /* Negative Infinity */
>> +            } else { /* Positive sign bit */
>> +                farg1.ll = 0x7ff0000000000000; /* Positive Infinity */
>> +            }
>> +            helper_compute_fprf_float64(env, farg1.d);
> 
> I don't believe you.
> (1) This is against IEEE spec,

I'm trying to implement IBM PowerPC specs. 

> (2) There is nothing about this zero result in the Power manual,

This is for PowerPC. Power and PowerPC are cousins to each other rather than 
having a child-parent relationship. Yes there are a lot of similar instructions 
between them, this does not mean they are compatible with each other.  

> (3) I do not replicate this experimentally.

I used a G3 iMac and a G5 iMac to do my testing.

> 
> #include <signal.h>
> #include <stdio.h>
> #include <fenv.h>
> #include <stdlib.h>
> 
> void handle(int sig, siginfo_t *info, void *x)
> {
>    ucontext_t *u = x;
>    printf("%f\n", u->uc_mcontext.fp_regs[0]);
>    exit(0);
> }
> 
> int main()
> {
>  struct sigaction a = { .sa_sigaction = handle, .sa_flags = SA_SIGINFO };
>  sigaction(SIGFPE, &a, NULL);
>  feenableexcept(FE_ALL_EXCEPT);

This is C99 code. There are a lot of floating point bugs with this 
implementation. I suggest all future testing be done using PowerPC assembly 
language only.

> 
>  {
>    register double f0 __asm__("32") = 2;
>    register double f1 __asm__("33") = 1;
>    register double f2 __asm__("34") = 0;
>    __asm__ volatile ("fdiv %0,%1,%2" : "+f"(f0) : "f"(f1), "f"(f2));
>  }
>  return 1;
> }
> 
> Produces the expected 2.0, i.e. the destination register is unmodified.
> Without feenableexcept, of course, the normal infinity is produced.

What compiler did you use to compile this program?

What operating system did you run this program on?

What are the specs of the system you used to test this program on?

I made a complete floating point test program that I could send you if you want 
to see how I test things.

Thank you.