http://codereview.chromium.org/155047/diff/1/8
File src/arm/codegen-arm.cc (right):

http://codereview.chromium.org/155047/diff/1/8#newcode779
Line 779: PrintF("GenericBinaryOpStub (%s)\n", Token::String(op_));
On 2009/07/03 11:27:01, William Hesse wrote:
> Why not make this message more informative, by including known_rhs_?

Oops, that is what I was trying to do, but I got the sense of the 'if'
wrong.

http://codereview.chromium.org/155047/diff/1/8#newcode792
Line 792: int known_rhs) {
On 2009/07/03 11:27:01, William Hesse wrote:
> Would "constant_rhs" be a better name than "known_rhs"?

Probably

http://codereview.chromium.org/155047/diff/1/8#newcode925
Line 925: if (popcnt > 2) return false;
On 2009/07/03 11:27:01, William Hesse wrote:
> The whole function can be replaced by:
> x &= x - 1;
> return (x & (x-1)) != 0;

Cool

http://codereview.chromium.org/155047/diff/1/8#newcode1045
Line 1045: case Token::SHL: {
On 2009/07/03 11:27:01, William Hesse wrote:
> Why no check of shift_value with 0 for SHL?  If it is ok, comment it.

Shift left by zero is the way reg-reg mov is encoded.  Section A5.1.4 in
the reference manual.  But since that would make this instruction a NOP
I will not emit it.

http://codereview.chromium.org/155047/diff/1/8#newcode5477
Line 5477: __ mov(destination, Operand(destination, LSL, first_bit));
On 2009/07/03 11:27:01, William Hesse wrote:
> Are you going to find out if the value overflowed from the flags?
Will they be
> set right from this combination of add and move?

No, the flags are not set by this routine.  Comment added.  You have to
check that the result will not overflow beforehand.

http://codereview.chromium.org/155047

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

Reply via email to