On Tue, Feb 23, 2010 at 3:10 PM, <[email protected]> wrote:

>
> http://codereview.chromium.org/651029/diff/1006/48
> File src/arm/simulator-arm.cc (right):
>

fixed

>
> http://codereview.chromium.org/651029/diff/1006/48#newcode563
> src/arm/simulator-arm.cc:563: return v + ((v & 1) ? +
> Instr::kPCReadOffsetThumb : Instr::kPCReadOffset);
> There's a stray '+' here and the (v & 1) thing needs to be in its own
> method called IsInThumbMode.  When you do that method remember not to
> use implicit bool conversion.
>

added InThumbMode()


>
> http://codereview.chromium.org/651029/diff/1006/48#newcode2159
> src/arm/simulator-arm.cc:2159: result >>= (shift_amount - 1);
> Can shift_amount be zero?  If so how does this work.  If not, then
> please assert that it is not zero.
>
> ASSERT added


> http://codereview.chromium.org/651029/diff/1006/48#newcode2204
> src/arm/simulator-arm.cc:2204: //  enter_debug_ = true;
> Please no commented out code.
>

removed


>
> http://codereview.chromium.org/651029/diff/1006/48#newcode2320
> src/arm/simulator-arm.cc:2320: } else if (program_counter & 1) {
> Why only if program_counter & 1?
> (And implicit conversion)


Removed. No idea how this got there. Probably a merge gone bad.


>
>
> http://codereview.chromium.org/651029
>



-- 
Stefan Haustein
Google UK Limited

Registered Office: Belgrave House, 76 Buckingham Palace Road, London SW1W
9TQ; Registered in England Number: 3977902

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

Reply via email to