http://codereview.chromium.org/569015/diff/5006/4006
File src/arm/codegen-arm.cc (right):

http://codereview.chromium.org/569015/diff/5006/4006#newcode5827
src/arm/codegen-arm.cc:5827: void
GenericBinaryOpStub::GetShiftCountLeastBits(MacroAssembler* masm,
Please add register parameters source and destination instead of
hardcoding r0 and r2.

I think the function should be called something like
GetLeastBitsFromSmi, as what it really does is getting the
num_least_bits from a smi. If something like this is needed in other
places you could consider moving it to the macro assembler instead.

http://codereview.chromium.org/569015/diff/5006/4006#newcode5831
src/arm/codegen-arm.cc:5831: // UBFX r2, r0, #kSmiTagSize, #(5-1)
Comment assumes num_least_bits == 5.

http://codereview.chromium.org/569015/diff/5006/4006#newcode5833
src/arm/codegen-arm.cc:5833: } else {
This branch does not take num_least_bits into account. Something like
0x1f -> (1 << num_least_bits) - 1 below.

http://codereview.chromium.org/569015/diff/5006/4006#newcode5834
src/arm/codegen-arm.cc:5834: __  mov(r2, Operand(r0, ASR, kSmiTagSize));
 // y
Please remove comment // y

http://codereview.chromium.org/569015/diff/5006/4006#newcode6052
src/arm/codegen-arm.cc:6052: __ mov(r3, Operand(r1, ASR, kSmiTagSize));
// x
Use GetShiftCountLeastBits(masm, r0, r2, 5) here as well?

http://codereview.chromium.org/569015

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

Reply via email to