Re: [Qemu-devel] [PATCH] SPARC target : Fix carry flagupdate inaddxcc and subxc

Re: [Qemu-devel] [PATCH] SPARC target : Fix carry flagupdate inaddxcc and subxc

[Blue Swirl]

[-- Attachment #1: Type: text/plain, Size: 1235 bytes --]

>As far as the V flag is concerned, mmm, I'm not really sure whether we 
>should
>change something in the sparc code. If we compare to the arm code, we don't
>take into account the fact that the carry flag is set before.
>
>We'd probably need some extensive tests and their associated expected 
>results.

I made a small test program (attached) to test the addx instruction. The 
program calculates the sum of two 64-bit values, given on the command line 
as 32-bit lower and upper parts.  Native system produces following:
$ ./addx -1 -1 0x80000000 -1
ffffffffffffffff + ffffffff80000000 = ffffffff7fffffff, NZVC: 9
while unpatched Qemu the following:
$ qemu-sparc ./addx -1 -1 0x80000000 -1
ffffffffffffffff + ffffffff80000000 = ffffffff7fffffff, NZVC: 8

So the carry flag not set. When your patch is applied, the output is 
identical:
ffffffffffffffff + ffffffff80000000 = ffffffff7fffffff, NZVC: 9

I couldn't think of a combination of values that would set the V flag when 
there is also a carry from the 32-bit addition, any suggestions?

_________________________________________________________________
FREE pop-up blocking with the new MSN Toolbar - get it now! 
http://toolbar.msn.click-url.com/go/onm00200415ave/direct/01/

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: addx.c --]
[-- Type: text/x-csrc; name="addx.c", Size: 1195 bytes --]

#include <stdlib.h>
#include <stdio.h>

void addx_test(unsigned int a, unsigned int b, unsigned int c, unsigned int d)
{
    int flags, result1, result2;

    asm ("addcc %3,%5,%1\n\t"
         "addxcc %4,%6,%2\n\t"
         "mov 0, %0\n\t"
         "bcc 1f\n\t"
         " nop\n\t"
         "mov 1, %0\n\t"
         "1: \n\t"
         "bvc 1f\n\t"
         " nop\n\t"
         "or %0, 2, %0\n\t"
         "1: \n\t"
         "bnz 1f\n\t"
         " nop\n\t"
         "or %0, 4, %0\n\t"
         "1: \n\t"
         "bpos 1f\n\t"
         " nop\n\t"
         "or %0, 8, %0\n\t"
         "1: \n\t"
         : "=r" (flags), "=r" (result1), "=r" (result2) : "r" (a), "r" (b), "r" (c), "r" (d));
    printf("%llx + %llx = %llx, NZVC: %d\n", ((long long)b << 32) | (long long)a, 
           ((long long)d << 32) | (long long)c,
           ((long long)result2 << 32) | (long long)result1,
           flags);
}

int main(int argc, const char **argv)
{
    unsigned int a, b, c, d;

    if (argc != 5)
        return 1;
    a = strtoul(argv[1], NULL, 0);
    b = strtoul(argv[2], NULL, 0);
    c = strtoul(argv[3], NULL, 0);
    d = strtoul(argv[4], NULL, 0);
    addx_test(a, b, c, d);

    return 0;
}

Re: [Qemu-devel] [PATCH] SPARC target : Fix carry flagupdate inaddxcc and subxc

[Even Rouault]

[-- Attachment #1: Type: text/plain, Size: 3968 bytes --]

Hello, 

As far as the V flag is concerned, I've taken a look at the Sparc V8 reference manual (www.sparc.org/standards/V8.pdf)

We can read at page 170 for the update of the V flag for "addcc" and "addxcc":
Vtheory = (r[rs1]<31> & operand2<31> & !result<31>) | (!r[rs1]<31> & !operand2<31> && result<31>)

Let's transform this with the name of the variables in the qemu code :
Vtheory = (src1<31> & T1<31> & !T0<31>) | (!src1<31> & !T1<31> & T0<31>)
Vtheory = ((src1 & T1 & ~T0) | (~src1 & ~T1 & T0)<31>

And we have in qemu code :
Vqemu = ((src1 ^ T1 ^ -1) & (src1 ^ T0))<31>

Now, let's transform Vqemu :
Vqemu = ((src1 ^ (T1 ^ -1)) & (src1 ^ T0))<31>
Vqemu = ((src1 ^ ~T1) & (src1 ^ T0))<31>
Vqemu = (((src1 & ~(~T1)) | (~src1 & ~T1)) & (src1 ^ T0))<31>
Vqemu = (((src1 & T1) | (~src1 & ~T1)) & (src1 ^ T0))<31>
Vqemu = ((src1 & T1 & (src1 ^ T0)) | (~src1 & ~T1 & (src1 ^ T0)))<31>
Vqemu = ((src1 & T1 & ((src1 & ~T0) | (~src1 & T0))) |
                (~src1 & ~T1 & ((src1 & ~T0) | (~src1 & T0))))<31>
Vqemu = ((src1 & T1 & src1 & ~T0) | (src1 & T1 & ~src1 & T0) |
                (~src1 & ~T1 & src1 & ~T0) | (~src1 & ~T1 & ~src1 & T0))<31>
Vqemu = ((src1 & T1 & ~T0) | (~src1 & ~T1 & T0))<31>
Vqemu = Vtheroy !

After theory, a bit of practice! I just wrote a small piece of code that enumerates the 2*2*2=8 combinations and proves experimentally that Vqemu = Vtheroy.

int main(int argc, char* argv[])
{
  int src1, T1, T0;
  for(src1=0;src1<=1;src1++)
  {
    for(T1=0;T1<=1;T1++)
    {
      for(T0=0;T0<=1;T0++)
      {
        int V1 = (src1 & T1 & ~T0) | (~src1 & ~T1 & T0);
        int V2 = (src1 ^ T1 ^ 1) & (src1 ^ T0);
        printf("src1=%d T1=%d T0=%d, V=%d=%d\n", src1, T1, T0,  V1, V2);
      }
    }
  }
}

The output is :
src1=0 T1=0 T0=0, V=0=0
src1=0 T1=0 T0=1, V=1=1
src1=0 T1=1 T0=0, V=0=0
src1=0 T1=1 T0=1, V=0=0
src1=1 T1=0 T0=0, V=0=0
src1=1 T1=0 T0=1, V=0=0
src1=1 T1=1 T0=0, V=1=1
src1=1 T1=1 T0=1, V=0=0

In other words, the V flag is set when :
the most significant bit of src1=src2=0 and dst=1 : the result of the addition of two signed positive words is not a signed positive word
the most significant bit of src1=src2=1 and dst=0 : the result of the addition of two signed negative words is not a signed negative word (or the result of the addition of two unsigned words is a lower unsigned word)
Conclusion : the computation of the V flag in qemu is correct, and their is no special case to consider if the C flag is set or not  :-)
For tomorrow, the formal proof of the correctness of the whole qemu code ;-)

Le Jeudi 13 Avril 2006 20:39, vous avez écrit :
> >As far as the V flag is concerned, mmm, I'm not really sure whether we
> >should
> >change something in the sparc code. If we compare to the arm code, we
> > don't take into account the fact that the carry flag is set before.
> >
> >We'd probably need some extensive tests and their associated expected
> >results.
>
> I made a small test program (attached) to test the addx instruction. The
> program calculates the sum of two 64-bit values, given on the command line
> as 32-bit lower and upper parts.  Native system produces following:
> $ ./addx -1 -1 0x80000000 -1
> ffffffffffffffff + ffffffff80000000 = ffffffff7fffffff, NZVC: 9
> while unpatched Qemu the following:
> $ qemu-sparc ./addx -1 -1 0x80000000 -1
> ffffffffffffffff + ffffffff80000000 = ffffffff7fffffff, NZVC: 8
>
> So the carry flag not set. When your patch is applied, the output is
> identical:
> ffffffffffffffff + ffffffff80000000 = ffffffff7fffffff, NZVC: 9
>
> I couldn't think of a combination of values that would set the V flag when
> there is also a carry from the 32-bit addition, any suggestions?
>
> _________________________________________________________________
> FREE pop-up blocking with the new MSN Toolbar - get it now!
> http://toolbar.msn.click-url.com/go/onm00200415ave/direct/01/

[-- Attachment #2: Type: text/html, Size: 5324 bytes --]

Re: [Qemu-devel] [PATCH] SPARC target : Fix carry flagupdate inaddxcc and subxc

[Blue Swirl]

[-- Attachment #1: Type: text/plain, Size: 1254 bytes --]

>Conclusion : the computation of the V flag in qemu is correct, and their is 
>no special case to consider if the C flag is set or not  :-)
>For tomorrow, the formal proof of the correctness of the whole qemu code 
>;-)

Thanks for the superb analysis!

Now it's time to check if real hardware works according to theory.

In this addition, there is a carry from lower 32 bits as well as sign 
change:
./addx -1 0x7fffffff 1 0
7fffffffffffffff + 1 = 8000000000000000, NZVC: 10
qemu-sparc ./addx  -1 0x7fffffff  1 0
7fffffffffffffff + 1 = 8000000000000000, NZVC: 10

Here, no carry:
./addx -1 0x7fffffff 0 1
7fffffffffffffff + 100000000 = 80000000ffffffff, NZVC: 10
qemu-sparc ./addx  -1 0x7fffffff 0 1
7fffffffffffffff + 100000000 = 80000000ffffffff, NZVC: 10

Another case:
./addx 0 0x80000000 0 0x80000000
8000000000000000 + 8000000000000000 = 0, NZVC: 7
qemu-sparc ./addx 0 0x80000000 0 0x80000000
8000000000000000 + 8000000000000000 = 0, NZVC: 7

Looks fine to me. There was a sign extension problem in the test program, 
fixed version attached.

_________________________________________________________________
Express yourself instantly with MSN Messenger! Download today it's FREE! 
http://messenger.msn.click-url.com/go/onm00200471ave/direct/01/

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: addx.c --]
[-- Type: text/x-csrc; name="addx.c", Size: 1258 bytes --]

#include <stdlib.h>
#include <stdio.h>

void addx_test(unsigned int a, unsigned int b, unsigned int c, unsigned int d)
{
    unsigned int flags, result1, result2;

    asm ("addcc %3,%5,%1\n\t"
         "addxcc %4,%6,%2\n\t"
         "mov 0, %0\n\t"
         "bcc 1f\n\t"
         " nop\n\t"
         "mov 1, %0\n\t"
         "1: \n\t"
         "bvc 1f\n\t"
         " nop\n\t"
         "or %0, 2, %0\n\t"
         "1: \n\t"
         "bnz 1f\n\t"
         " nop\n\t"
         "or %0, 4, %0\n\t"
         "1: \n\t"
         "bpos 1f\n\t"
         " nop\n\t"
         "or %0, 8, %0\n\t"
         "1: \n\t"
         : "=r" (flags), "=r" (result1), "=r" (result2) : "r" (a), "r" (b), "r" (c), "r" (d));
    printf("%llx + %llx = %llx, NZVC: %d\n", ((unsigned long long)b << 32) | (unsigned long long)a, 
           ((unsigned long long)d << 32) | (unsigned long long)c,
           ((unsigned long long)result2 << 32) | (unsigned long long)result1,
           flags);
}

int main(int argc, const char **argv)
{
    unsigned int a, b, c, d;

    if (argc != 5)
        return 1;
    a = strtoul(argv[1], NULL, 0);
    b = strtoul(argv[2], NULL, 0);
    c = strtoul(argv[3], NULL, 0);
    d = strtoul(argv[4], NULL, 0);
    addx_test(a, b, c, d);

    return 0;
}