[Qemu-devel] [PATCH] Fix overflow conditions for MIPS add / subtract

[Qemu-devel] [PATCH] Fix overflow conditions for MIPS add / subtract

[Stefan Weil]

Hi,

I had problems with MIPS system emulation (AR7 based DSL router)
which were caused by wrong overflow exceptions.

With the patch given below emulation works. See this link for
first results: http://forum.openwrt.org/viewtopic.php?id=4381

In user mode emulation, the MIPS emulation currently ignores
exceptions. So the bug might have an effect on emulation speed
but not on functionality for user mode emulation.

Regards
Stefan Weil

PS. Please include this and also my last MIPS patch in CVS HEAD.


Index: target-mips/op.c
===================================================================
RCS file: /sources/qemu/qemu/target-mips/op.c,v
retrieving revision 1.5
diff -u -b -B -r1.5 op.c
--- target-mips/op.c    5 Dec 2005 19:59:36 -0000       1.5
+++ target-mips/op.c    13 Apr 2006 18:38:19 -0000
@@ -206,7 +206,8 @@

     tmp = T0;
     T0 += T1;
-    if ((T0 >> 31) ^ (T1 >> 31) ^ (tmp >> 31)) {
+    if (((tmp ^ T1 ^ (-1)) & (T0 ^ T1)) >> 31) {
+       /* operands of same sign, result different sign */
         CALL_FROM_TB1(do_raise_exception_direct, EXCP_OVERFLOW);
     }
     RETURN();
@@ -224,7 +225,8 @@

     tmp = T0;
     T0 = (int32_t)T0 - (int32_t)T1;
-    if (!((T0 >> 31) ^ (T1 >> 31) ^ (tmp >> 31))) {
+    if (((tmp ^ T1) & (tmp ^ T0)) >> 31) {
+       /* operands of different sign, first operand and result 
different sign */
         CALL_FROM_TB1(do_raise_exception_direct, EXCP_OVERFLOW);
     }
     RETURN();

Re: [Qemu-devel] [PATCH] Fix overflow conditions for MIPS add / subtract

[Daniel Jacobowitz]

On Thu, Apr 13, 2006 at 08:49:19PM +0200, Stefan Weil wrote:
> -    if ((T0 >> 31) ^ (T1 >> 31) ^ (tmp >> 31)) {
> +    if (((tmp ^ T1 ^ (-1)) & (T0 ^ T1)) >> 31) {
> +       /* operands of same sign, result different sign */
>         CALL_FROM_TB1(do_raise_exception_direct, EXCP_OVERFLOW);
>     }

I see this went in, but - huh?  The math doesn't make sense.

T0 ^ T1 -> operands of different sign
tmp ^ T1 ^ (-1) -> result has same sign as T1

Which is a "who cares" case.  This is addition, it can't overflow if
the operands have the same sign.

> -    if (!((T0 >> 31) ^ (T1 >> 31) ^ (tmp >> 31))) {
> +    if (((tmp ^ T1) & (tmp ^ T0)) >> 31) {
> +       /* operands of different sign, first operand and result 
> different sign */
>         CALL_FROM_TB1(do_raise_exception_direct, EXCP_OVERFLOW);
>     }

tmp ^ T1 -> result and T1 of different sign
tmp ^ T0 -> result and T0 of different sign

Which implies that the operands have the same sign.  Again, this case
can't overflow.

I haven't tested the patched qemu, but I did test the expressions
themselves in standalone code, and they definitely do not detect
overflow.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [Qemu-devel] [PATCH] Fix overflow conditions for MIPS add / subtract

[Dirk Behme]

Daniel Jacobowitz wrote:
> I haven't tested the patched qemu, but I did test the expressions
> themselves in standalone code, and they definitely do not detect
> overflow.

Maybe you can test Ralf's alternative proposal

http://lists.gnu.org/archive/html/qemu-devel/2006-02/msg00154.html

as well?

Thanks

Dirk

Re: [Qemu-devel] [PATCH] Fix overflow conditions for MIPS add / subtract

[Daniel Jacobowitz]

On Fri, Apr 28, 2006 at 04:51:39PM +0200, Dirk Behme wrote:
> Daniel Jacobowitz wrote:
> >I haven't tested the patched qemu, but I did test the expressions
> >themselves in standalone code, and they definitely do not detect
> >overflow.
> 
> Maybe you can test Ralf's alternative proposal
> 
> http://lists.gnu.org/archive/html/qemu-devel/2006-02/msg00154.html
> 
> as well?

Using 64-bit math for this would be awful for performance.  My original
checks were wrong; we just need to use a correct fix...  Lightly
tested, but I think this is right for add:

-    if ((T0 >> 31) ^ (T1 >> 31) ^ (tmp >> 31)) {
+    if (~(T0 ^ T1) & (T0 ^ tmp) & 0x80000000) {

And this for sub:

-    if (!((T0 >> 31) ^ (T1 >> 31) ^ (tmp >> 31))) {
+    if ((T0 ^ T1) & (T0 ^ tmp) & 0x80000000) {


-- 
Daniel Jacobowitz
CodeSourcery

Re: [Qemu-devel] [PATCH] Fix overflow conditions for MIPS add / subtract

[Julian Seward]

> > -    if ((T0 >> 31) ^ (T1 >> 31) ^ (tmp >> 31)) {
> > +    if (((tmp ^ T1 ^ (-1)) & (T0 ^ T1)) >> 31) {
> > +       /* operands of same sign, result different sign */
> >         CALL_FROM_TB1(do_raise_exception_direct, EXCP_OVERFLOW);
> >     }
>
> I see this went in, but - huh?  The math doesn't make sense.
>
> T0 ^ T1 -> operands of different sign
> tmp ^ T1 ^ (-1) -> result has same sign as T1

The definitive reference for all this bit twiddling magic and
much more besides is an excellent book, "Hacker's Delight", by
Hank Warren.  It has loads of stuff about integer overflow and
whatnot.

J