515545 - Add EXECUTE_TREE_TIMER code for Windows x64

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Makoto Kato [:m_kato]]

Attachment: patch v1

VC++ dones't support inline assembler, so we must use build-in function for rdtsc

Comment 1

[David Anderson [:dvander] - inactive, e-mail if emergency]

Could we just use the compiler intrinsic for both architectures?

Comment 2

[Makoto Kato [:m_kato]]

(In reply to comment #1)
> Could we just use the compiler intrinsic for both architectures?

__rdtsc is provided on both x86 and x64.  But old compiler (VC++7.1?) doesn't support this intrinsic.  I believe that Neil uses VC++7.1, so we should not replace x86 inline assembler with inctinsic version.

Comment 3

[[e-mail]]

Looks like I'm in luck: my WinNT.h contains #pragma intrinsic(__rdtsc)

Comment 4

[David Anderson [:dvander] - inactive, e-mail if emergency]

Makoto: With Neil's comment is it okay? I'm in favor of reducing platform #ifdefs and inline assembly.

Comment 5

[Makoto Kato [:m_kato]]

(In reply to comment #4)
> Makoto: With Neil's comment is it okay? I'm in favor of reducing platform
> #ifdefs and inline assembly.

OK, I write new patch, then I will send review to you.

Comment 6

[Makoto Kato [:m_kato]]

Attachment: patch v2

Comment 7

[Makoto Kato [:m_kato]]

Attachment: patch v3

Previous patch breaks on x86.  Now testing more.

Comment 8

[Makoto Kato [:m_kato]]

Attachment: patch v3.1 [Checkin: Comment 9 & 10]

#include <intrin.h> cause build error on VC++ 2005 x86.  So remove it (because it is already defined in WinNT.h)

Comment 9

[Makoto Kato [:m_kato]]

http://hg.mozilla.org/tracemonkey/rev/3cd616d615cd

Comment 10

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/3cd616d615cd

Comment 11

[[e-mail]]

(In reply to comment #3)
> Looks like I'm in luck: my WinNT.h contains #pragma intrinsic(__rdtsc)
So, I think this was because it was in my Vista SDK; it's not in the VC7 headers. Also, another machine I have with VC8 but only the Windows Server 2003 SDK doesn't compile this because it too is missing the intrinsic.

Comment 12

[Makoto Kato [:m_kato]]

(In reply to comment #11)
> (In reply to comment #3)
> > Looks like I'm in luck: my WinNT.h contains #pragma intrinsic(__rdtsc)
> So, I think this was because it was in my Vista SDK; it's not in the VC7
> headers. Also, another machine I have with VC8 but only the Windows Server 2003
> SDK doesn't compile this because it too is missing the intrinsic.

Humm, when I add #include <intrin.h> like patch v3 with VC8 + Vista SDK or Win7 SDK, VC8 throws unexpected build error (comment #8).  This issue seems to be fixed by VC9.

David, I think patch v1 is better (use inline asm for x86 and intrin function for x64).  How do you think?

Comment 13

[[e-mail]]

FYI, I added these lines so that I could compile with VC8Express + 2003SDK:
>extern "C" unsigned __int64 __rdtsc(void);
>#pragma intrinsic(__rdtsc)
As for VC7, I've given up; I can't work around vprof.h's varadic macros.

Comment 14

[Serge Gautherie (:sgautherie)]

(In reply to comment #13)
> VC8Express + 2003SDK:

That's what I'm using too (FF 3.7 32bit build) and I "still" get
{
jsapi.cpp
...\js\src\nanojit\avmplus.h(106) : error C3861: '__rdtsc': identifier not found
}

Is this bug actually (fully) fixed?

Comment 15

[David Anderson [:dvander] - inactive, e-mail if emergency]

(In reply to comment #12)
> David, I think patch v1 is better (use inline asm for x86 and intrin function
> for x64).  How do you think?

Sorry for late reply. Sure, v1 is okay if this is going to cause problems.

Comment 16

[Serge Gautherie (:sgautherie)]

Please, fix comment 14 failure.

Comment 17

[Makoto Kato [:m_kato]]

Attachment: patch v4 [Checkin: Comment 18]

Comment 18

[Makoto Kato [:m_kato]]

re-landed.
http://hg.mozilla.org/tracemonkey/rev/1549b37afb8f

Comment 19

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/1549b37afb8f

Comment 20

[Serge Gautherie (:sgautherie)]

V.Fixed, I can build (SM) trunk locally again.