LGTM with comments.

https://chromiumcodereview.appspot.com/11037023/diff/39023/src/arm/assembler-arm.cc
File src/arm/assembler-arm.cc (right):

https://chromiumcodereview.appspot.com/11037023/diff/39023/src/arm/assembler-arm.cc#newcode843
src/arm/assembler-arm.cc:843: return true;
Why don't we check for !CpuFeatures::IsSupported(ARMv7) in this case?

https://chromiumcodereview.appspot.com/11037023/diff/39023/src/arm/assembler-arm.h
File src/arm/assembler-arm.h (right):

https://chromiumcodereview.appspot.com/11037023/diff/39023/src/arm/assembler-arm.h#newcode757
src/arm/assembler-arm.h:757: static const int
kPatchDebugBreakSlotReturnOffset = 2 * kInstrSize;
Is the offset correct when we don't use BLX?

https://chromiumcodereview.appspot.com/11037023/diff/39023/src/v8globals.h
File src/v8globals.h (right):

https://chromiumcodereview.appspot.com/11037023/diff/39023/src/v8globals.h#newcode429
src/v8globals.h:429: UnknownImplementer,
Nit: names of constants are usually all upper case or prefixed with 'k'.

https://chromiumcodereview.appspot.com/11037023/

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

Reply via email to