[webkit-reviews] review canceled: [Bug 90319] Make TextCodecLatin1 handle 8 bit data without converting to UChar's : [Attachment 151833] Patch for Review

[bugzilla-daemon at webkit.org]

Michael Saboff <msaboff at apple.com> has canceled Michael Saboff
<msaboff at apple.com>'s request for review:
Bug 90319: Make TextCodecLatin1 handle 8 bit data without converting to UChar's
https://bugs.webkit.org/show_bug.cgi?id=90319

Attachment 151833: Patch for Review
https://bugs.webkit.org/attachment.cgi?id=151833&action=review

------- Additional Comments from Michael Saboff <msaboff at apple.com>
(In reply to comment #2)
> (From update of attachment 151833 [details])
> View in context:
https://bugs.webkit.org/attachment.cgi?id=151833&action=review
> 
> > Source/WebCore/platform/text/TextCodecASCIIFastPath.h:43
> > +	 static void copy(LChar* destination, const uint8_t* source)
> > +	 {
> > +	     destination[0] = source[0];
> > +	     destination[1] = source[1];
> > +	     destination[2] = source[2];
> > +	     destination[3] = source[3];
> > +	 }
> > +	 
> 
> What does the assembly for this look like?
> 
> > Source/WebCore/platform/text/TextCodecASCIIFastPath.h:62
> > +	     destination[0] = source[0];
> > +	     destination[1] = source[1];
> > +	     destination[2] = source[2];
> > +	     destination[3] = source[3];
> > +	     destination[4] = source[4];
> > +	     destination[5] = source[5];
> > +	     destination[6] = source[6];
> > +	     destination[7] = source[7];
> 
> ditto

Changed both of these explicit copies to memcpy(destination, source, <4 or 8>)

For Mac 64 bit with these changes, the compiler generated:
    mov    (%r12),%rdx
    ...
    mov    %rdx,(%r15)

> 
> > Source/WebCore/platform/text/TextCodecLatin1.cpp:172
> > +	 while (characters < destination)
> 
> This condition is really confusing to me, what on earth is destination?

Changed this to:

    // Zero extend and copy already processed 8 bit data
    LChar* ptr8 = characters;
    LChar* endPtr8 = destination;

    while (ptr8 < endPtr8)
	*destination16++ = *ptr8++;