Partial comments only.

http://codereview.chromium.org/7122003/diff/1/src/x64/assembler-x64.h
File src/x64/assembler-x64.h (right):

http://codereview.chromium.org/7122003/diff/1/src/x64/assembler-x64.h#newcode1378
src/x64/assembler-x64.h:1378: inline byte get_opcode(int offset) {
Rename to get_byte/set_byte. Or read_byte/patch_byte.
There is nothing opcode-specific in this.

http://codereview.chromium.org/7122003/diff/1/src/x64/code-stubs-x64.cc
File src/x64/code-stubs-x64.cc (right):

http://codereview.chromium.org/7122003/diff/1/src/x64/code-stubs-x64.cc#newcode5189
src/x64/code-stubs-x64.cc:5189: __ jmp(&skip_non_incremental_part,
Label::kNear);
But why overwrite?
Why not just:
  if (!HEAP->incremental_marking()->IsMarking()) {
    __ nop(2);
  } else {
    __ jmp(&....);
  }
And is this even safe? The label is unbound, so won't we be overwriting
this later?

http://codereview.chromium.org/7122003/diff/1/src/x64/code-stubs-x64.cc#newcode5190
src/x64/code-stubs-x64.cc:5190: if
(!HEAP->incremental_marking()->IsMarking()) {
HEAP should be replaced by something faster. Maybe you can get the
isolate from the MacroAssembler.

http://codereview.chromium.org/7122003/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to