Menu
Search

Forums

Forum Index

Thread overview
[dmd-internals] 64 bit bug: rndtol fails.
Jan 12, 2011
Don Clugston
Jan 18, 2011
Brad Roberts
Jan 18, 2011
Brad Roberts
Jan 19, 2011
Walter Bright
Jan 19, 2011
Brad Roberts
January 12, 2011
Gravatar of Don ClugstonPosted by Don ClugstonReply
Don Clugston
Gravatar of Don Clugston


Reply
---
import std.math;

void main()
{
      assert( rndtol(-3.0) == -3 );
}
---

This is the only failure in std.mathspecial.
January 18, 2011
Gravatar of Brad RobertsPosted by Brad Roberts
in reply to Don Clugston		Reply
Brad Roberts
Gravatar of Brad Roberts
Posted in reply to Don Clugston


Reply
On 1/12/2011 1:45 PM, Don Clugston wrote:
> ---
> import std.math;
> 
> void main()
> {
>       assert( rndtol(-3.0) == -3 );
> }
> ---
> 
> This is the only failure in std.mathspecial.

Confirmed buggy in d1 as well.

The generated code:
_Dmain:
                push    RBP
                mov     RBP,RSP
                sub     RSP,010h
                fld     tbyte ptr FLAT:.rodata[00h][RIP]
                fistp   long64 ptr -8[RBP]
                mov     EAX,-8[RBP]
                cmp     RAX,0FFFFFFFDh
                je      L25
                mov     EDI,7
                call      _D3bug8__assertFiZv at PC32
L25:            xor     EAX,EAX
                leave
                ret

Comparing it with the 32 bit version, the tbyte ptr part is fine, but the mov EAX is almost certainly wrong.

Try this change.. tested only with this repro case so far:

Index: backend/cg87.c ===================================================================
--- backend/cg87.c      (revision 880)
+++ backend/cg87.c      (working copy)
@@ -3063,7 +3063,11 @@
             genfltreg(c2,0x8B,findreglsw(retregs),0);
         }
         else
+        {
             c2 = genfltreg(c2,0x8B,reg,0);      // MOV reg,floatreg
+            if (tysize(tym) == 8 && I64)
+                code_orrex(c2, REX_W);
+        }
         c2 = cat(c2,fixresult(e,retregs,pretregs));

         return cat(c1,c2);
January 18, 2011
Gravatar of Brad RobertsPosted by Brad Roberts
in reply to Brad Roberts		Reply
Brad Roberts
Gravatar of Brad Roberts
Posted in reply to Brad Roberts


Reply
On 1/18/2011 1:20 AM, Brad Roberts wrote:
> On 1/12/2011 1:45 PM, Don Clugston wrote:
>> ---
>> import std.math;
>>
>> void main()
>> {
>>       assert( rndtol(-3.0) == -3 );
>> }
>> ---
>>
>> This is the only failure in std.mathspecial.
> 
> Confirmed buggy in d1 as well.
> 
> Try this change.. tested only with this repro case so far:
> 
> Index: backend/cg87.c ===================================================================
> --- backend/cg87.c      (revision 880)
> +++ backend/cg87.c      (working copy)
> @@ -3063,7 +3063,11 @@
>              genfltreg(c2,0x8B,findreglsw(retregs),0);
>          }
>          else
> +        {
>              c2 = genfltreg(c2,0x8B,reg,0);      // MOV reg,floatreg
> +            if (tysize(tym) == 8 && I64)
> +                code_orrex(c2, REX_W);
> +        }
>          c2 = cat(c2,fixresult(e,retregs,pretregs));
> 
>          return cat(c1,c2);

The unit tests all pass still -- not that there's likely terribly much that exercises that particular builtin function.  I've also confirmed that std.mathspecial's unittests pass with this fix too.  Once DMD's change is checked in, it can be removed from the disabled list in phobos' posix.mak.

Thanks for the reduced test case.  Have you done a similar analysis for std.math's unit test failure?

Later,
Brad
January 18, 2011
Gravatar of Walter BrightPosted by Walter Bright
in reply to Brad Roberts		Reply
Walter Bright
Gravatar of Walter Bright
Posted in reply to Brad Roberts


Reply
Thanks, Brad & Don. Commit 882.

Brad Roberts wrote:
> On 1/12/2011 1:45 PM, Don Clugston wrote:
> 
>> ---
>> import std.math;
>>
>> void main()
>> {
>>       assert( rndtol(-3.0) == -3 );
>> }
>> ---
>>
>> This is the only failure in std.mathspecial.
>> 
>
> Confirmed buggy in d1 as well.
>
> The generated code:
> _Dmain:
>                 push    RBP
>                 mov     RBP,RSP
>                 sub     RSP,010h
>                 fld     tbyte ptr FLAT:.rodata[00h][RIP]
>                 fistp   long64 ptr -8[RBP]
>                 mov     EAX,-8[RBP]
>                 cmp     RAX,0FFFFFFFDh
>                 je      L25
>                 mov     EDI,7
>                 call      _D3bug8__assertFiZv at PC32
> L25:            xor     EAX,EAX
>                 leave
>                 ret
>
> Comparing it with the 32 bit version, the tbyte ptr part is fine, but the mov EAX is almost certainly wrong.
>
> Try this change.. tested only with this repro case so far:
>
> Index: backend/cg87.c ===================================================================
> --- backend/cg87.c      (revision 880)
> +++ backend/cg87.c      (working copy)
> @@ -3063,7 +3063,11 @@
>              genfltreg(c2,0x8B,findreglsw(retregs),0);
>          }
>          else
> +        {
>              c2 = genfltreg(c2,0x8B,reg,0);      // MOV reg,floatreg
> +            if (tysize(tym) == 8 && I64)
> +                code_orrex(c2, REX_W);
> +        }
>          c2 = cat(c2,fixresult(e,retregs,pretregs));
>
>          return cat(c1,c2);
>
> _______________________________________________
> dmd-internals mailing list
> dmd-internals at puremagic.com
> http://lists.puremagic.com/mailman/listinfo/dmd-internals
>
>
>
January 18, 2011
Gravatar of Brad RobertsPosted by Brad Roberts
in reply to Walter Bright		Reply
Brad Roberts
Gravatar of Brad Roberts
Posted in reply to Walter Bright


Reply
Another one down.. 18 phobos tests left.  22 dmd tests left (was 29 and 33 at
last report on Jan 6).

On 1/18/2011 8:39 PM, Walter Bright wrote:
> Thanks, Brad & Don. Commit 882.
> 
> Brad Roberts wrote:
>> On 1/12/2011 1:45 PM, Don Clugston wrote:
>> 
>>> ---
>>> import std.math;
>>>
>>> void main()
>>> {
>>>       assert( rndtol(-3.0) == -3 );
>>> }
>>> ---
>>>
>>> This is the only failure in std.mathspecial.
>>> 
>>
>> Confirmed buggy in d1 as well.
>>
>> The generated code:
>> _Dmain:
>>                 push    RBP
>>                 mov     RBP,RSP
>>                 sub     RSP,010h
>>                 fld     tbyte ptr FLAT:.rodata[00h][RIP]
>>                 fistp   long64 ptr -8[RBP]
>>                 mov     EAX,-8[RBP]
>>                 cmp     RAX,0FFFFFFFDh
>>                 je      L25
>>                 mov     EDI,7
>>                 call      _D3bug8__assertFiZv at PC32
>> L25:            xor     EAX,EAX
>>                 leave
>>                 ret
>>
>> Comparing it with the 32 bit version, the tbyte ptr part is fine, but the mov EAX is almost certainly wrong.
>>
>> Try this change.. tested only with this repro case so far:
>>
>> Index: backend/cg87.c ===================================================================
>> --- backend/cg87.c      (revision 880)
>> +++ backend/cg87.c      (working copy)
>> @@ -3063,7 +3063,11 @@
>>              genfltreg(c2,0x8B,findreglsw(retregs),0);
>>          }
>>          else
>> +        {
>>              c2 = genfltreg(c2,0x8B,reg,0);      // MOV reg,floatreg
>> +            if (tysize(tym) == 8 && I64)
>> +                code_orrex(c2, REX_W);
>> +        }
>>          c2 = cat(c2,fixresult(e,retregs,pretregs));
>>
>>          return cat(c1,c2);
>>
>> _______________________________________________
>> dmd-internals mailing list
>> dmd-internals at puremagic.com
>> http://lists.puremagic.com/mailman/listinfo/dmd-internals
>>
>>
>> 
> _______________________________________________
> dmd-internals mailing list
> dmd-internals at puremagic.com
> http://lists.puremagic.com/mailman/listinfo/dmd-internals