LGTM. Could you address these nit picks and then upload again. I will then land
for you. Thanks for the patch!

http://codereview.chromium.org/3606009/diff/21001/6003
File src/arm/assembler-arm.cc (right):

http://codereview.chromium.org/3606009/diff/21001/6003#newcode1637
src/arm/assembler-arm.cc:1637: // Stops with a code less than
kNumOfWatchedStops support enabling/disabling
with a code -> with a non-negative code

http://codereview.chromium.org/3606009/diff/21001/6003#newcode1643
src/arm/assembler-arm.cc:1643: ASSERT(code >= -1);   // Code should be
positive, except for the default -1.
kDefaultStopCode?

http://codereview.chromium.org/3606009/diff/21001/6004
File src/arm/assembler-arm.h (right):

http://codereview.chromium.org/3606009/diff/21001/6004#newcode907
src/arm/assembler-arm.h:907: void stop(const char* msg, Condition cond =
al, int32_t code = -1);
Replace -1 with kDefaultStopCode?

http://codereview.chromium.org/3606009/diff/21001/6007
File src/arm/disasm-arm.cc (right):

http://codereview.chromium.org/3606009/diff/21001/6007#newcode1019
src/arm/disasm-arm.cc:1019: void Decoder::DecodeUnconditional(Instr*
instr) {
If this method is unreachable, shouldn't you just remove it?

http://codereview.chromium.org/3606009/diff/21001/6008
File src/arm/simulator-arm.cc (right):

http://codereview.chromium.org/3606009/diff/21001/6008#newcode447
src/arm/simulator-arm.cc:447: + Instr::kInstrSize);
Strange indentation. I would do two lines:

Instr* msg_address =
  reinterpre_cast<Instr*>(stop_pc + Instr::kInstrSize);

http://codereview.chromium.org/3606009/show

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

Reply via email to