LGTM On Mon, Dec 8, 2008 at 2:28 PM, <[EMAIL PROTECTED]> wrote: > > Reviewers: Erik Corry, > > Message: > Re. Your comments > > Description: > Minor presentation changes > > Please review this at http://codereview.chromium.org/13620 > > Affected files: > M src/jsregexp.cc > M src/regexp-macro-assembler-ia32.cc > > > Index: src/jsregexp.cc > diff --git a/src/jsregexp.cc b/src/jsregexp.cc > index > 3d1930ea9ea2d6b59550099b5dda935deeb2ea22..13c717220deac22f0b8453b5546768859c1ae6e8 > 100644 > --- a/src/jsregexp.cc > +++ b/src/jsregexp.cc > @@ -312,8 +312,7 @@ Handle<Object> RegExpImpl::Exec(Handle<JSRegExp> regexp, > return result; > } > // We couldn't handle the regexp using Irregexp, so fall back > - // on JSCRE. We rejoice at the though of the day when this is > - // no longer needed. > + // on JSCRE. > // Reset the JSRegExp to use JSCRE. > JscrePrepare(regexp, > Handle<String>(regexp->Pattern()), > @@ -343,8 +342,7 @@ Handle<Object> RegExpImpl::ExecGlobal(Handle<JSRegExp> > regexp, > return result; > } > // We couldn't handle the regexp using Irregexp, so fall back > - // on JSCRE. We rejoice at the though of the day when this is > - // no longer needed. > + // on JSCRE. > // Reset the JSRegExp to use JSCRE. > JscrePrepare(regexp, > Handle<String>(regexp->Pattern()), > @@ -911,7 +909,7 @@ Handle<Object> > RegExpImpl::IrregexpExecOnce(Handle<FixedArray> irregexp, > // String is now either Sequential or External > StringShape flatshape(*subject); > bool is_ascii = flatshape.IsAsciiRepresentation(); > - int char_size = is_ascii ? sizeof(char) : sizeof(uc16); // NOLINT > + int char_size_shift = is_ascii ? 0 : 1; > > if (flatshape.IsExternal()) { > const byte* address; > @@ -925,19 +923,20 @@ Handle<Object> > RegExpImpl::IrregexpExecOnce(Handle<FixedArray> irregexp, > rc = RegExpMacroAssemblerIA32::Execute( > *code, > &address, > - start_offset * char_size, > - end_offset * char_size, > + start_offset << char_size_shift, > + end_offset << char_size_shift, > offsets_vector, > previous_index == 0); > } else { // Sequential string > - int byte_offset = > - is_ascii ? SeqAsciiString::kHeaderSize - kHeapObjectTag: > - SeqTwoByteString::kHeaderSize - kHeapObjectTag; > + Address char_address = > + is_ascii ? SeqAsciiString::cast(*subject)->GetCharsAddress() > + : SeqTwoByteString::cast(*subject)->GetCharsAddress(); > + int byte_offset = char_address - > reinterpret_cast<Address>(*subject); > rc = RegExpMacroAssemblerIA32::Execute( > *code, > subject.location(), > - byte_offset + start_offset * char_size, > - byte_offset + end_offset * char_size, > + byte_offset + (start_offset << char_size_shift), > + byte_offset + (end_offset << char_size_shift), > offsets_vector, > previous_index == 0); > } > Index: src/regexp-macro-assembler-ia32.cc > diff --git a/src/regexp-macro-assembler-ia32.cc > b/src/regexp-macro-assembler-ia32.cc > index > 21993b7e7627c5e8cde3c3c68c4e9c1eb41ae0aa..be2990ea961f53f8ea948565167e948f18d12431 > 100644 > --- a/src/regexp-macro-assembler-ia32.cc > +++ b/src/regexp-macro-assembler-ia32.cc > @@ -684,8 +684,9 @@ int > RegExpMacroAssemblerIA32::CaseInsensitiveCompareUC16(uc16** buffer, > int byte_offset1, > int byte_offset2, > size_t > byte_length) { > - // This function MUST NOT cause a garbage collection. A GC might move > - // the calling generated code and invalidate the stacked return address. > + // This function is not allowed to cause a garbage collection. > + // A GC might move the calling generated code and invalidate the > + // return address on the stack. > ASSERT(byte_length % 2 == 0); > Address buffer_address = reinterpret_cast<Address>(*buffer); > uc16* substring1 = reinterpret_cast<uc16*>(buffer_address + > byte_offset1); > > > > > >
-- Erik Corry, Software Engineer Google Denmark ApS. CVR nr. 28 86 69 84 c/o Philip & Partners, 7 Vognmagergade, P.O. Box 2227, DK-1018 Copenhagen K, Denmark. --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
