http://codereview.chromium.org/1706013/diff/65002/41014 File src/arm/codegen-arm.cc (right):
http://codereview.chromium.org/1706013/diff/65002/41014#newcode8119 src/arm/codegen-arm.cc:8119: // r3: Length of subject string as a smi On 2010/04/28 13:25:27, Erik Corry wrote:
Missing full stop after comment on these lines.
There are a lot of such comments around. May be to fix them separately? http://codereview.chromium.org/1706013/diff/65002/41014#newcode8122 src/arm/codegen-arm.cc:8122: // Check that the third argument is a positive smi less than the subject On 2010/04/28 13:25:27, Erik Corry wrote:
we don't actually check that the third argument is a Smi. We just
check whether
it is in range or not, then shift out the tag bit. Not sure whether
that is OK,
but the comment should reflect it.
Runtime_RegExpExec checks for smi and throws an exception if it is not. So I added a check here. http://codereview.chromium.org/1706013/diff/65002/41014#newcode9029 src/arm/codegen-arm.cc:9029: __ tst(min_length, Operand(min_length)); On 2010/04/28 13:33:41, Vyacheslav Egorov wrote:
Testing smi for zero.
ASSERT kSmiTag == 0?
Done. http://codereview.chromium.org/1706013/diff/65002/41014#newcode9144 src/arm/codegen-arm.cc:9144: __ cmp(r2, Operand(0)); // Test if first string is empty. On 2010/04/28 13:33:41, Vyacheslav Egorov wrote:
Comparing smi with non-smi.
Assert kSmiTag == 0 or use Smi::FromInt() ?
Another interesting question [yes I am aware that you are not the
author of the
code] what is faster on ARM tst(x,x) or cmp(x, 0)?
Done. http://codereview.chromium.org/1706013/diff/65002/41014#newcode9146 src/arm/codegen-arm.cc:9146: __ cmp(r3, Operand(0), ne); // Else test if second string is empty. On 2010/04/28 13:33:41, Vyacheslav Egorov wrote:
Ditto.
Done. http://codereview.chromium.org/1706013/diff/65002/41015 File src/arm/macro-assembler-arm.cc (right): http://codereview.chromium.org/1706013/diff/65002/41015#newcode1060 src/arm/macro-assembler-arm.cc:1060: str(scratch2, FieldMemOperand(result, String::kLengthOffset)); On 2010/04/28 13:33:41, Vyacheslav Egorov wrote:
I talked to Erik and he says that it is reasonable to interleave load
with other
operation to utilize pipeline better.
Sorry for the confusion.
Done. http://codereview.chromium.org/1706013/diff/65002/41006 File src/ia32/codegen-ia32.cc (right): http://codereview.chromium.org/1706013/diff/65002/41006#newcode8824 src/ia32/codegen-ia32.cc:8824: __ test(edx, Operand(edx)); On 2010/04/28 13:33:41, Vyacheslav Egorov wrote:
Well as far as I can see edx is not used after this point, only
compared with 0.
So remove SmiUntag and assert kSmiTag == 0.
Done. http://codereview.chromium.org/1706013/diff/65002/41006#newcode10917 src/ia32/codegen-ia32.cc:10917: ASSERT(kSmiTag == 0 && kSmiTagSize); // edi is smi (powered by 2). On 2010/04/28 13:33:41, Vyacheslav Egorov wrote:
lost: == 1 in assertion.
Done. http://codereview.chromium.org/1706013/diff/65002/41011 File src/x64/codegen-x64.cc (right): http://codereview.chromium.org/1706013/diff/65002/41011#newcode9747 src/x64/codegen-x64.cc:9747: On 2010/04/28 13:33:41, Vyacheslav Egorov wrote:
empty line?
Done. http://codereview.chromium.org/1706013/diff/65002/41011#newcode10318 src/x64/codegen-x64.cc:10318: __ testq(min_length, min_length); On 2010/04/28 13:33:41, Vyacheslav Egorov wrote:
ASSERT kSmiTag == 0 ?
Done. http://codereview.chromium.org/1706013/diff/65002/41011#newcode10353 src/x64/codegen-x64.cc:10353: __ testq(length_difference, length_difference); On 2010/04/28 13:33:41, Vyacheslav Egorov wrote:
ASSERT kSmiTag == 0 ?
Done. http://codereview.chromium.org/1706013/show -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
