http://codereview.chromium.org/10984057/diff/1/src/arm/simulator-arm.cc File src/arm/simulator-arm.cc (right):
http://codereview.chromium.org/10984057/diff/1/src/arm/simulator-arm.cc#newcode1504 src/arm/simulator-arm.cc:1504: int32_t left = result >> shift_amount; How does this work with a negative input? http://codereview.chromium.org/10984057/diff/1/src/hydrogen-instructions.cc File src/hydrogen-instructions.cc (right): http://codereview.chromium.org/10984057/diff/1/src/hydrogen-instructions.cc#newcode1624 src/hydrogen-instructions.cc:1624: : new(zone) Range(); Don't you have to Ror the result here? http://codereview.chromium.org/10984057/diff/1/src/hydrogen-instructions.h File src/hydrogen-instructions.h (right): http://codereview.chromium.org/10984057/diff/1/src/hydrogen-instructions.h#newcode3638 src/hydrogen-instructions.h:3638: ChangeRepresentation(Representation::Integer32()); Why is HRor different from all other bit ops here? http://codereview.chromium.org/10984057/diff/1/src/hydrogen.cc File src/hydrogen.cc (right): http://codereview.chromium.org/10984057/diff/1/src/hydrogen.cc#newcode1043 src/hydrogen.cc:1043: !shl->representation().IsInteger32()) Missing {} http://codereview.chromium.org/10984057/diff/1/src/hydrogen.cc#newcode1052 src/hydrogen.cc:1052: if (!shr->right()->IsSub()) return false; This function needs a comment explaining what it is looking for, and the line above here needs an explanation of why only a sub function is OK here. http://codereview.chromium.org/10984057/diff/1/src/hydrogen.cc#newcode1062 src/hydrogen.cc:1062: if (sub->right()->OperandAt(0) != shl->right()->OperandAt(0)) Missing {} http://codereview.chromium.org/10984057/diff/1/src/hydrogen.cc#newcode1083 src/hydrogen.cc:1083: bool HGraph::TryRotateRight(HInstruction* instr) { There is a lot of copied code here - can they be combined into one function with a flag? http://codereview.chromium.org/10984057/diff/1/src/token.h File src/token.h (right): http://codereview.chromium.org/10984057/diff/1/src/token.h#newcode102 src/token.h:102: T(ROR, "rotate right", 11) \ This needs a comment to explain that it is only used in Crankshaft. http://codereview.chromium.org/10984057/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
