LGTM if comments below are addressed.

http://codereview.chromium.org/6647012/diff/5001/src/x64/lithium-codegen-x64.cc
File src/x64/lithium-codegen-x64.cc (right):

http://codereview.chromium.org/6647012/diff/5001/src/x64/lithium-codegen-x64.cc#newcode261
src/x64/lithium-codegen-x64.cc:261: JumpTableEntry* info =
&jump_table_[i];
This is OK, but I'm nervous to see it.  A pointer into the backing store
of a list can be dangerous because the backing store can change and the
old one become invalid.

It's OK here because the list isn't resized, but it could be a potential
problem with various refactorings.

It's easily avoided:

...i++) {
  __ bind(&jump_table_[i].label);
  __ Jump(jump_table_[i].address_, RelocInfo::RUNTIME_ENTRY);
}

http://codereview.chromium.org/6647012/diff/5001/src/x64/lithium-codegen-x64.cc#newcode543
src/x64/lithium-codegen-x64.cc:543: if (jump_table_.length() == 0 ||
if (jump_table_.is_empty() || jump_table_.last().address != entry) {
  ...
}

http://codereview.chromium.org/6647012/diff/5001/src/x64/lithium-codegen-x64.cc#newcode547
src/x64/lithium-codegen-x64.cc:547: __ j(cc,
&jump_table_[jump_table_.length() - 1].label_);
__ j(cc, &jump_table_.last().label);

http://codereview.chromium.org/6647012/

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

Reply via email to