One more thing. Can you please also add a testcase that triggered the
bad behaviour so that we do not regress in the future.

Thanks,
-Ivan


On 2008/12/09 16:13:17, iposva wrote:
> Please fix the comments, otherwise LGTM.
>
> Nice catch,
> -Ivan
>
> http://codereview.chromium.org/13290/diff/1/4
> File src/macro-assembler-arm.cc (right):
>
> http://codereview.chromium.org/13290/diff/1/4#newcode179
> Line 179: void MacroAssembler::SmiJumpTable(Register index,
Vector<Label*>
> targets) {
> The parameter index is not used within this function. Also it seems
that r0 is
> used directly.
>
> http://codereview.chromium.org/13290/diff/1/4#newcode182
> Line 182: add(pc, pc, Operand(r0, LSL, 2 - kSmiTagSize));
> Can you document the constant 2 here? Or even better use something
like
> kInstrSizeLog2 which you should define in constants-arm.h next to
kInstrSize.
>
> http://codereview.chromium.org/13290/diff/1/4#newcode184
> Line 184: stop("Unreachable: jump table alignment");
> Please use a nop() instruction here. The stop macro is not guaranteed
to be
> encoded in one instruction.



http://codereview.chromium.org/13290

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

Reply via email to