[webkit-reviews] review granted: [Bug 196296] [YARR] Precompute BMP / non-BMP status when constructing character classes : [Attachment 366074] Patch

[bugzilla-daemon at webkit.org]

Keith Miller <keith_miller at apple.com> has granted Michael Saboff
<msaboff at apple.com>'s request for review:
Bug 196296: [YARR] Precompute BMP / non-BMP status when constructing character
classes
https://bugs.webkit.org/show_bug.cgi?id=196296

Attachment 366074: Patch

https://bugs.webkit.org/attachment.cgi?id=366074&action=review




--- Comment #4 from Keith Miller <keith_miller at apple.com> ---
Comment on attachment 366074
  --> https://bugs.webkit.org/attachment.cgi?id=366074
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=366074&action=review

r=me.

> Source/JavaScriptCore/yarr/YarrJIT.cpp:1813
> -	   add32(TrustedImm32(1), countRegister);
>  #ifdef JIT_UNICODE_EXPRESSIONS
>	   if (m_decodeSurrogatePairs) {
> -	       Jump isBMPChar = branch32(LessThan, character,
supplementaryPlanesBase);
> -	       op.m_jumps.append(atEndOfInput());
> -	       add32(TrustedImm32(1), countRegister);
> -	       add32(TrustedImm32(1), index);
> -	       isBMPChar.link(this);
> -	   }
> +	       if (term->characterClass->hasOneCharacterSize() &&
!term->invert())
> +		  
add32(TrustedImm32(term->characterClass->hasNonBMPCharacters() ? 2 : 1),
countRegister);
> +	       else {
> +		   add32(TrustedImm32(1), countRegister);
> +		   Jump isBMPChar = branch32(LessThan, character,
supplementaryPlanesBase);
> +		   op.m_jumps.append(atEndOfInput());
> +		   add32(TrustedImm32(1), countRegister);
> +		   add32(TrustedImm32(1), index);
> +		   isBMPChar.link(this);
> +	       }
> +	   } else
>  #endif
> +	       add32(TrustedImm32(1), countRegister);

Seems like this could be a helper method.

> Source/JavaScriptCore/yarr/YarrJIT.cpp:1977
> +		  
add32(TrustedImm32(term->characterClass->hasNonBMPCharacters() ? 2 : 1),
index);

Nit: it might be nice to have a helper method to get the width for one class
like:

Optional<size_t> CharacterClass::knownCharacterClass()
{
    if (!hasOneCharacterSize())
	return nullopt;
   return hasNonBMPCharacters() ? 2 : 1;
}