X64: Changed TickSample to hold pointer-sized values for registers.

X64: Changed TickSample to hold pointer-sized values for registers.

[Lasse Reichstein, 2009-05-07 08:12:39]

QUick review of TickSample fix.

[Dean McNamee, 2009-05-07 08:34:08]

I don't know much about how our tick stuff works, but I imagine all of the other
tools need changes to (tick processor, etc).

http://codereview.chromium.org/113094/diff/1/2
File src/log.cc (right):

http://codereview.chromium.org/113094/diff/1/2#newcode1085
Line 1085: msg.Append("tick,0x%p,0x%p,%d", sample->pc, sample->sp,
I realize this is our own StringStream, but just FYI that %p is different
between Windows and Linux.

http://codereview.chromium.org/113094/diff/1/2#newcode1091
Line 1091: msg.Append(",0x%p", reinterpret_cast<uint32_t>(sample->stack[i]));
why are you usin %p for a non-pointer?

http://codereview.chromium.org/113094/diff/1/4
File src/platform-win32.cc (right):

http://codereview.chromium.org/113094/diff/1/4#newcode1778
Line 1778: #if V8_HOST_ARCH_64_BIT
shouldn't we check for X64 here.

[Søren Thygesen Gjesse, 2009-05-07 08:42:25]

LGTM with the %p issues addressed (which also affects
http://codereview.chromium.org/114010).

[Lasse Reichstein, 2009-05-07 09:06:00]

I added macro for our the pointer format om the other changelist. I'll use it
here instead of %p as well.

http://codereview.chromium.org/113094/diff/1/2
File src/log.cc (right):

http://codereview.chromium.org/113094/diff/1/2#newcode1085
Line 1085: msg.Append("tick,0x%p,0x%p,%d", sample->pc, sample->sp,
Yes, it needs to be changed. Will do that in another changelist and use the
change here as well.

http://codereview.chromium.org/113094/diff/1/2#newcode1091
Line 1091: msg.Append(",0x%p", reinterpret_cast<uint32_t>(sample->stack[i]));
Because I forgot to remove the cast. Good catch.

http://codereview.chromium.org/113094/diff/1/4
File src/platform-win32.cc (right):

http://codereview.chromium.org/113094/diff/1/4#newcode1778
Line 1778: #if V8_HOST_ARCH_64_BIT
No, this is really the host system's registers, not the target system's
(potentially simulated) ones.

[Dean McNamee, 2009-05-07 09:19:06]

http://codereview.chromium.org/113094/diff/1/4
File src/platform-win32.cc (right):

http://codereview.chromium.org/113094/diff/1/4#newcode1778
Line 1778: #if V8_HOST_ARCH_64_BIT
On 2009/05/07 09:06:01, Lasse Reichstein wrote:
> No, this is really the host system's registers, not the target system's
> (potentially simulated) ones.

Yes, we have both V8_HOST_ARCH_X64 and V8_TARGET_ARCH_X64.  I was suggesting we
use V8_HOST_ARCH_X64 instead of V8_HOST_ARCH_64_BIT.  After all, they are x86-64
registers

[Lasse Reichstein, 2009-05-07 09:21:40]

And yes, the tick processor will need change in order to work in 64-bit mode. In
particular, a JavaScript number can't hold a 64-bit pointer.

[William Hesse, 2009-05-07 09:27:50]

http://codereview.chromium.org/113094/diff/1/4
File src/platform-win32.cc (right):

http://codereview.chromium.org/113094/diff/1/4#newcode1779
Line 1779: UNIMPLEMENTED();
Shouldn't UNIMPLEMENTED() be removed, if this code is supposed to be correct?

http://codereview.chromium.org/113094/diff/1/4#newcode1779
Line 1779: UNIMPLEMENTED();
Shouldn't UNIMPLEMENTED() be removed, if this code is supposed to be correct?

[Lasse Reichstein, 2009-05-12 08:17:20]

http://codereview.chromium.org/113094/diff/1/4
File src/platform-win32.cc (right):

http://codereview.chromium.org/113094/diff/1/4#newcode1778
Line 1778: #if V8_HOST_ARCH_64_BIT
Done.