Did a second pass over sources.
http://codereview.chromium.org/1706013/diff/50001/51004 File src/ia32/codegen-ia32.cc (right): http://codereview.chromium.org/1706013/diff/50001/51004#newcode10786 src/ia32/codegen-ia32.cc:10786: __ j(below, &runtime); On 2010/04/27 16:16:30, SeRya wrote:
On 2010/04/27 12:27:24, Vyacheslav Egorov wrote: > negation of above is below_equal, not below. > > But having below_equal here would contradict comment. Very
interesting. Was
> there a bug here? > >
Actually (a < b) is equivalent to (b > a) but not to (b >= a).
I suppose this value is checked again in the regexp code so difference
is not
observable. Anyway, going into the runtime when they are equal looks
more
logical.
Yeah, my fault again. For some reason I thought that you _negated_ condition. http://codereview.chromium.org/1706013/diff/50001/51009 File src/x64/codegen-x64.cc (right): http://codereview.chromium.org/1706013/diff/50001/51009#newcode7604 src/x64/codegen-x64.cc:7604: __ SmiToInteger32(rdi, rdi); On 2010/04/27 16:16:30, SeRya wrote:
On 2010/04/27 12:27:24, Vyacheslav Egorov wrote: > Move this up to avoid duplication on both codepaths? >
It couldn't be placed after 'movq' since the next conditional jump
relies on the
flags register state set by 'testb'.
Yeah, you are right. http://codereview.chromium.org/1706013/diff/65002/41014 File src/arm/codegen-arm.cc (right): http://codereview.chromium.org/1706013/diff/65002/41014#newcode9029 src/arm/codegen-arm.cc:9029: __ tst(min_length, Operand(min_length)); Testing smi for zero. ASSERT kSmiTag == 0? 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. 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)? 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. Ditto. 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)); 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. 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)); 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. 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). lost: == 1 in assertion. 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: empty line? http://codereview.chromium.org/1706013/diff/65002/41011#newcode10318 src/x64/codegen-x64.cc:10318: __ testq(min_length, min_length); ASSERT kSmiTag == 0 ? http://codereview.chromium.org/1706013/diff/65002/41011#newcode10353 src/x64/codegen-x64.cc:10353: __ testq(length_difference, length_difference); ASSERT kSmiTag == 0 ? http://codereview.chromium.org/1706013/show -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
