Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(68)

Issue 159397: X64: Add mov rax,(mem64) to disassembler. (Closed)

Created:
11 years, 5 months ago by William Hesse
Modified:
9 years, 7 months ago
Reviewers:
Lasse Reichstein
CC:
v8-dev
Visibility:
Public.

Description

X64: Add mov rax,(mem64) to disassembler. Committed: http://code.google.com/p/v8/source/detail?r=2540

Patch Set 1 #

Total comments: 10

Patch Set 2 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+33 lines, -0 lines) Patch
M src/x64/disasm-x64.cc View 1 1 chunk +33 lines, -0 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
William Hesse
11 years, 5 months ago (2009-07-27 08:28:04 UTC) #1
Lasse Reichstein
LGTM. http://codereview.chromium.org/159397/diff/1/2 File src/x64/disasm-x64.cc (right): http://codereview.chromium.org/159397/diff/1/2#newcode1350 Line 1350: reinterpret_cast<byte*>( Use "Address" instead of "byte*". http://codereview.chromium.org/159397/diff/1/2#newcode1357 ...
11 years, 5 months ago (2009-07-27 09:16:21 UTC) #2
William Hesse
11 years, 5 months ago (2009-07-27 10:22:46 UTC) #3
http://codereview.chromium.org/159397/diff/1/2
File src/x64/disasm-x64.cc (right):

http://codereview.chromium.org/159397/diff/1/2#newcode1350
Line 1350: reinterpret_cast<byte*>(
On 2009/07/27 09:16:21, Lasse Reichstein wrote:
> Use "Address" instead of "byte*".

Address is not defined in the disassembler.  The V8 header defining it is not
used.

http://codereview.chromium.org/159397/diff/1/2#newcode1357
Line 1357: NameOfAddress(*reinterpret_cast<byte**>(data+1)));
On 2009/07/27 09:16:21, Lasse Reichstein wrote:
> Use "Address*" instead of "byte**" (Much more readable!)

no

http://codereview.chromium.org/159397/diff/1/2#newcode1367
Line 1367: switch (operand_size()) {
On 2009/07/27 09:16:21, Lasse Reichstein wrote:
> Would it be possible to combine the two cases that only differ on the format
> string?

Yes, and I have no done so, so done.

http://codereview.chromium.org/159397/diff/1/2#newcode1371
Line 1371: *reinterpret_cast<int32_t*>(data + 1))));
On 2009/07/27 09:16:21, Lasse Reichstein wrote:
> ditto.

no

http://codereview.chromium.org/159397/diff/1/2#newcode1377
Line 1377: NameOfAddress(*reinterpret_cast<byte**>(data+1)));
On 2009/07/27 09:16:21, Lasse Reichstein wrote:
> And ditto.

no

Powered by Google App Engine
This is Rietveld 408576698