mersenneforum.org

mersenneforum.org (https://www.mersenneforum.org/index.php)
-   Cunningham Tables (https://www.mersenneforum.org/forumdisplay.php?f=51)
-   -   Help! Compiler bug (https://www.mersenneforum.org/showthread.php?t=14004)

R.D. Silverman 2010-10-02 00:09

[QUOTE=R.D. Silverman;232204]Instead, I am just going to kick out the assembler code and look at it.[/QUOTE]

I looked at the assembler output. Nothing obvious stands out except
that the get_time() call is in-lined.

[code]
; 5650 : { /* start of prepare_bounds */

push ebp
mov ebp, esp
sub esp, 20 ; 00000014H

; 5651 : int do_nothing(char *x);
; 5652 : double m_over_a1, n_over_b1, b0_over_b1, a0_over_a1;
; 5653 : double a0, a1, b0, b1, one_over_a1, one_over_b1;
; 5654 : double l1and4, l2and3, l1and3, determ, inv_determ;
; 5655 : double stime;
; 5656 : double temp;
; 5657 :
; 5658 :
; 5659 : if (TIME_STATS) stime = get_time();

DB 15 ; 0000000fH
DB 49 ; 00000031H
mov DWORD PTR _a$89577[ebp], eax
mov DWORD PTR _b$89578[ebp], edx

; 5660 :
; 5661 : a0 = (double)v1[0];
; 5662 : a1 = (double)v2[0];
; 5663 : b0 = (double)v1[1];
; 5664 : b1 = (double)v2[1];
; 5665 :
; 5666 : one_over_a1 = 1.0/a1;
; 5667 : a0_over_a1 = a0 * one_over_a1;
; 5668 : one_over_b1 = 1.0/b1;
; 5669 : b0_over_b1 = b0 * one_over_b1;
; 5670 :
; 5671 : /* We have a parallelogram. One point is always (0,0). f is the */
; 5672 : /* vertical axis, e the horizontal. Compute emin & emax */
; 5673 : /* Also, compute intersections and boundary slopes */
; 5674 : /* Note that determ = p (up to sign) so could precompute: but it */
; 5675 : /* would require xtra storage to hold the sign bit */
; 5676 :
; 5677 : determ = (a0 * b1 - a1 * b0);
; 5678 : inv_determ = 1.0/determ;
; 5679 :
; 5680 : if (SHOW_PREP)
; 5681 : {
; 5682 : (void) printf("Prep: a0,a1,b0,b1 = %g %g %g %g\n",a0,a1,b0,b1);
; 5683 : (void) printf("m_over_a1, n_over_b1 = %g %g\n",m_over_a1, n_over_b1);
; 5684 : (void) printf("a0_over_a1 , b0_over_b1 = %g %g\n",a0_over_a1, b0_over_b1);
; 5685 : (void) printf("determ, inv = %g %g\n",determ,inv_determ);
; 5686 : }
; 5687 :
; 5688 : if (sign == 1) {

cmp DWORD PTR _sign$[ebp], 1
mov ecx, DWORD PTR _a$89577[ebp]
movsd xmm2, QWORD PTR __real@3ff0000000000000
movd xmm5, DWORD PTR [edx+4]
movd xmm7, DWORD PTR [edx]
mov DWORD PTR _t$89576[ebp], ecx
mov ecx, DWORD PTR _b$89578[ebp]
mov DWORD PTR _t$89576[ebp+4], ecx
fild QWORD PTR _t$89576[ebp]
mov ecx, DWORD PTR [eax]
mov eax, DWORD PTR [eax+4]
xorps xmm4, xmm4
cvtsi2sd xmm4, eax
movapd xmm1, xmm2
divsd xmm1, xmm4
cvtdq2pd xmm5, xmm5
cvtdq2pd xmm7, xmm7
xorps xmm6, xmm6
cvtsi2sd xmm6, ecx
movapd xmm0, xmm2
mulsd xmm1, xmm5
mulsd xmm4, xmm7
mulsd xmm5, xmm6
subsd xmm4, xmm5
divsd xmm0, xmm6
movapd xmm3, xmm0
divsd xmm2, xmm4

; 5689 :
; 5690 : if (a0 > 0 && a1 > 0)

[/code]

retina 2010-10-02 00:24

[QUOTE=R.D. Silverman;232273][code] DB 15 ; 0000000fH
DB 49 ; 00000031H
mov DWORD PTR _a$89577[ebp], eax
mov DWORD PTR _b$89578[ebp], edx

cmp DWORD PTR _sign$[ebp], 1
mov ecx, DWORD PTR _a$89577[ebp]
movsd xmm2, QWORD PTR __real@3ff0000000000000
movd xmm5, DWORD PTR [edx+4]
movd xmm7, DWORD PTR [edx]
[/code][/QUOTE]RDTSC (0x0f,0x31) returns the counter in edx:eax
And then "movd xmm5, DWORD PTR [edx+4]" will randomly crash.

Why is edx never initialised to point to anything proper after reading the TSC? Did you really show all the compiled code for that section? If so then get a new compiler.

R.D. Silverman 2010-10-02 00:38

[QUOTE=retina;232275]RDTSC (0x0f,0x31) returns the counter in edx:eax
And then "movd xmm5, DWORD PTR [edx+4]" will randomly crash.

Why is edx never initialised to point to anything proper after reading the TSC? Did you really show all the compiled code for that section? If so then get a new compiler.[/QUOTE]

Yes. This is all the code. It grabs the clock counter, then converts it to a double.

The compiler is Microsoft Visual Studio 2010 (and VS 2008)

R.D. Silverman 2010-10-02 00:45

[QUOTE=retina;232275]RDTSC (0x0f,0x31) returns the counter in edx:eax
And then "movd xmm5, DWORD PTR [edx+4]" will randomly crash.

Why is edx never initialised to point to anything proper after reading the TSC? Did you really show all the compiled code for that section? If so then get a new compiler.[/QUOTE]

Nice catch. I failed to see it.

Here is the debug assembler. Note that it does not in-line the get_time()
call and subsequently does not use the edx register:

[code]
; 5650 : { /* start of prepare_bounds */

push ebp
mov ebp, esp
sub esp, 228 ; 000000e4H
push ebx
push esi
push edi

; 5651 : int do_nothing(char *x);
; 5652 : double m_over_a1, n_over_b1, b0_over_b1, a0_over_a1;
; 5653 : double a0, a1, b0, b1, one_over_a1, one_over_b1;
; 5654 : double l1and4, l2and3, l1and3, determ, inv_determ;
; 5655 : double stime;
; 5656 : double temp;
; 5657 :
; 5658 :
; 5659 : if (TIME_STATS) stime = get_time();

mov eax, 1
test eax, eax
je SHORT $LN21@prepare_bo
call _get_time
fstp QWORD PTR _stime$[ebp]
$LN21@prepare_bo:

; 5660 :
; 5661 : a0 = (double)v1[0];

mov eax, DWORD PTR _v1$[ebp]
fild DWORD PTR [eax]
fstp QWORD PTR _a0$[ebp]

; 5662 : a1 = (double)v2[0];

mov eax, DWORD PTR _v2$[ebp]
fild DWORD PTR [eax]
fstp QWORD PTR _a1$[ebp]

; 5663 : b0 = (double)v1[1];

mov eax, DWORD PTR _v1$[ebp]
fild DWORD PTR [eax+4]
fstp QWORD PTR _b0$[ebp]

; 5664 : b1 = (double)v2[1];

mov eax, DWORD PTR _v2$[ebp]

etc.
[/code]

retina 2010-10-02 00:47

[QUOTE=R.D. Silverman;232278]Yes. This is all the code. It grabs the clock counter, then converts it to a double.[/QUOTE]The double conversion is done further down, after transferring through ecx to another location - "fild QWORD PTR _t$89576[ebp]"[QUOTE=R.D. Silverman;232278]The compiler is Microsoft Visual Studio 2010 (and VS 2008)[/QUOTE]Write a nice letter to MS and complain.

R.D. Silverman 2010-10-02 01:04

[QUOTE=retina;232280]The double conversion is done further down, after transferring through ecx to another location - "fild QWORD PTR _t$89576[ebp]"Write a nice letter to MS and complain.[/QUOTE]

get_time calls an assembler routine that samples the clock and returns
a 64 bit int. It converts the 64-bit int to a double and returns it.

I could reorganize the code inside get_time(), but I doubt it will help.

The problem is the misuse of the edx register AFTER get_time() returns.
I suspect it is a bug in the code optimizer when dealing with (the other) floating point code.

R.D. Silverman 2010-10-02 01:47

[QUOTE=R.D. Silverman;232283]get_time calls an assembler routine that samples the clock and returns
a 64 bit int. It converts the 64-bit int to a double and returns it.

I could reorganize the code inside get_time(), but I doubt it will help.

The problem is the misuse of the edx register AFTER get_time() returns.
I suspect it is a bug in the code optimizer when dealing with (the other) floating point code.[/QUOTE]

I changed the code so that the 64-bit routine that samples the clock
returns a double instead of an int64 and then called it directly.
The call is not inlined. Instead I just get

call _get_time1

However, the emitted code STILL mis-uses the edx register in the
middle of the floating computations that follow.

3 lines later it does:

mov ecx DWORD PTR [edx] but without initializing where edx is pointing.

In fact, it is still pointing to whatever was placed in it by the clock
sample code.

axn 2010-10-02 03:31

[QUOTE=R.D. Silverman;232288]I changed the code so that the 64-bit routine that samples the clock
returns a double instead of an int64 and then called it directly.
The call is not inlined. Instead I just get

call _get_time1

However, the emitted code STILL mis-uses the edx register in the
middle of the floating computations that follow.

3 lines later it does:

mov ecx DWORD PTR [edx] but without initializing where edx is pointing.

In fact, it is still pointing to whatever was placed in it by the clock
sample code.[/QUOTE]
I believe the first few parameters are passed via registers -- look at the place where it _calls_ your routine. I bet v1 and v2 are passed in eax and edx. These four lines use apparently uninitialized registers.

[CODE] movd xmm5, DWORD PTR [edx+4]
movd xmm7, DWORD PTR [edx]

mov ecx, DWORD PTR [eax]
mov eax, DWORD PTR [eax+4]
[/CODE]

retina 2010-10-02 04:08

[QUOTE=axn;232293]I believe the first few parameters are passed via registers -- look at the place where it _calls_ your routine. I bet v1 and v2 are passed in eax and edx.[/QUOTE]Unlikely because the debug code show the values being loaded from the stack. It would be extremely strange code that places pointers to the data on the stack AND loads eax/edx with pointers to the data and then calls the subroutine. There is no calling standard that defines that behaviour.

Random Poster 2010-10-02 09:56

[QUOTE=R.D. Silverman;232267]More weirdness. If I replace printf("1") with do_nothing("1") where do_nothing is just a dummy routine the code STILL fails.[/QUOTE]

Did you define do_nothing in the same source file? If so, then the optimizer probably replaced the call to it by the contents of the function. Try defining do_nothing in a different source file.

[QUOTE=R.D. Silverman;232267] How can the addition of a printf of a static string cure a core dump caused by a read access failure?[/QUOTE]

Values of general registers aren't expected to survive across function calls, so adding any call (that can't be inlined away) will force the compiler to reassign registers, and (as you noticed) this often works around optimization bugs where registers get garbled.

R.D. Silverman 2010-10-02 11:20

[QUOTE=Random Poster;232311]Did you define do_nothing in the same source file? If so, then the optimizer probably replaced the call to it by the contents of the function. Try defining do_nothing in a different source file.



Values of general registers aren't expected to survive across function calls, so adding any call (that can't be inlined away) will force the compiler to reassign registers, and (as you noticed) this often works around optimization bugs where registers get garbled.[/QUOTE]

I found the following in an Intel development manual:

"As discussed in section 2.3, some compilers do not implicitly recognize the RDTSC and CPUID function in inline
assembly code. Compilers like Microsoft® Visual C++® 5.0 normally "guarantee" that any register affected by an
inline assembly code section will not affect the C code around it. When overriding the compiler by using the emit
statements, however, the compiler does not know those instructions are overwriting registers (RDTSC overwrites
EAX and EDX, and CPUID overwrites EAX, EBX, ECX, and EDX). Thus, the compiler may not properly store away
the affected registers, so this must be done manually by the programmer by pushing them onto the stack.
There are a few cases where this will not matter. If the code being time measured is a stand-alone section of code,
completely surrounded by the calls to RDTSC, then the register overwriting cannot affect the code around it. If the
measured code section is written in assembly, and the variables are actually used inside of this section, the compiler
will handle the stack allocation itself. Finally, it will not matter if affecting the correctness of the code around the
measured section is not an issue while cycle testing."

Note however, that I did a workaround such that the compiler does NOT
in-line the clock sample code, but instead calls a subroutine. The emitted
code is still mis-using the edx register a few lines later. It seems certain
that the clock sample subroutine is not restoring the edx register when it
returns.


All times are UTC. The time now is 08:06.

Powered by vBulletin® Version 3.8.11
Copyright ©2000 - 2021, Jelsoft Enterprises Ltd.