LGTM with comments

http://codereview.chromium.org/11365084/diff/9001/src/arm/lithium-arm.cc
File src/arm/lithium-arm.cc (right):

http://codereview.chromium.org/11365084/diff/9001/src/arm/lithium-arm.cc#newcode1884
src/arm/lithium-arm.cc:1884: external_pointer =
UseRegisterAtStart(instr->elements());
I don't see the difference in these two case in the codegen file, and in
practice since there are no temp registers, it probably doesn't make a
difference (I am not 100% sure whether UseRegister overlaps with the
return register. If it really doesn't make a difference, UseRegister all
the time probably will simplify the code.

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

http://codereview.chromium.org/11365084/diff/9001/src/x64/lithium-codegen-x64.cc#newcode2616
src/x64/lithium-codegen-x64.cc:2616: inline void
LCodeGen::HandleKeyedAlterations(T* hydrogen_instr, LOperand* key) {
How about "PrepareKeyForKeyedOp" or "PrepareKeyForKeyedOperation"?

http://codereview.chromium.org/11365084/diff/9001/src/x64/lithium-x64.cc
File src/x64/lithium-x64.cc (right):

http://codereview.chromium.org/11365084/diff/9001/src/x64/lithium-x64.cc#newcode1847
src/x64/lithium-x64.cc:1847: if (instr->is_external()) {
Wrap this in #if DEBUG like you did elsewhere?

http://codereview.chromium.org/11365084/diff/9001/src/x64/lithium-x64.h
File src/x64/lithium-x64.h (right):

http://codereview.chromium.org/11365084/diff/9001/src/x64/lithium-x64.h#newcode1375
src/x64/lithium-x64.h:1375: inline static bool ArrayOpClobbersKey(T
*value) {
You can move this to the .cc file. It's only used internally.

http://codereview.chromium.org/11365084/

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

Reply via email to