Summary: | [YARR] Precompute BMP / non-BMP status when constructing character classes | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Michael Saboff <msaboff> | ||||||||||
Component: | JavaScriptCore | Assignee: | Michael Saboff <msaboff> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | ews-watchlist, guijemont, guijemont+jsc-armv7-ews, keith_miller, mark.lam, saam, webkit-bug-importer | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | WebKit Nightly Build | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Michael Saboff
2019-03-27 08:15:56 PDT
Created attachment 366074 [details]
Patch
Attachment 366074 [details] did not pass style-queue:
ERROR: Source/JavaScriptCore/yarr/YarrPattern.h:62: Missing spaces around | [whitespace/operators] [3]
Total errors found: 1 in 9 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 366074 [details] 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; } Comment on attachment 366074 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=366074&action=review > Source/JavaScriptCore/yarr/YarrJIT.cpp:1900 > + I'll eliminate this extra whitespace. > Source/JavaScriptCore/yarr/YarrJIT.cpp:1916 > + I'll eliminate this extra whitespace. Comment on attachment 366074 [details] Patch Attachment 366074 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/11685313 New failing tests: js/regexp-unicode.html Created attachment 366085 [details]
Archive of layout-test-results from ews206 for win-future
The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews206 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment on attachment 366074 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=366074&action=review >> Source/JavaScriptCore/yarr/YarrJIT.cpp:1813 >> + add32(TrustedImm32(1), countRegister); > > Seems like this could be a helper method. Done. (In reply to Build Bot from comment #7) > Created attachment 366085 [details] > Archive of layout-test-results from ews206 for win-future > > The attached test failures were seen while running run-webkit-tests on the > win-ews. > Bot: ews206 Port: win-future Platform: > CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit Fixed this issue locally in the YarrInterpreter. (In reply to Michael Saboff from comment #9) > (In reply to Build Bot from comment #7) > > Created attachment 366085 [details] > > Archive of layout-test-results from ews206 for win-future > > > > The attached test failures were seen while running run-webkit-tests on the > > win-ews. > > Bot: ews206 Port: win-future Platform: > > CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit > > Fixed this issue locally in the YarrInterpreter. The jsc-armv7 test failure is the same fixed issue as the win test failure. Created attachment 366183 [details]
Patch fo landing
Attachment 366183 [details] did not pass style-queue:
ERROR: Source/JavaScriptCore/yarr/YarrPattern.h:62: Missing spaces around | [whitespace/operators] [3]
Total errors found: 1 in 9 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 366185 [details]
Updated Patch for landing - fixed MIPS build
Attachment 366185 [details] did not pass style-queue:
ERROR: Source/JavaScriptCore/yarr/YarrPattern.h:62: Missing spaces around | [whitespace/operators] [3]
Total errors found: 1 in 9 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Committed r243642: <https://trac.webkit.org/changeset/243642> |