Hi, I answered your comments. I also fixed a little issue with the Disassembler concerning the message address of stops inlined in the code.
The new patch will be uploaded when somebody who has commit rights comes back.
Thanks, Alexandre 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 On 2010/10/10 12:37:27, Mads Ager wrote:
with a code -> with a non-negative code
Done. 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. On 2010/10/10 12:37:27, Mads Ager wrote:
kDefaultStopCode?
Done. 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); On 2010/10/10 12:37:27, Mads Ager wrote:
Replace -1 with kDefaultStopCode?
Done. http://codereview.chromium.org/3606009/diff/21001/6007 File src/arm/disasm-arm.cc (right): http://codereview.chromium.org/3606009/diff/21001/6007#newcode129 src/arm/disasm-arm.cc:129: void DecodeType7(Instr* instr); Changed DecodeType7 prototype to: int DecodeType7(Instr* instr); DecodeType7 decodes svc, and thus Debugger specific instructions. The return value is used to update the pc (for decoding). For example stop will return 8, so that the Disassembler does not try to interpret the message address as an instruction (which could crash). http://codereview.chromium.org/3606009/diff/21001/6007#newcode1019 src/arm/disasm-arm.cc:1019: void Decoder::DecodeUnconditional(Instr* instr) { Removed. Added UNIMPLEMENTED at call site instead. v8 implements but never uses blx immediate, which uses the special_condition code. Same changes in simulator-arm.cc . On 2010/10/10 12:37:27, Mads Ager wrote:
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 (left): http://codereview.chromium.org/3606009/diff/21001/6008#oldcode2219 src/arm/simulator-arm.cc:2219: void Simulator::DecodeUnconditional(Instr* instr) { Removed and added UNIMPLEMENTED() at the call site. (Same as for the disassembler.) 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); Changed. On 2010/10/10 12:37:27, Mads Ager wrote:
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
