Bug 192294

Summary: [WHLSL] Add a handwritten lexer
Product: WebKit Reporter: Myles C. Maxfield <mmaxfield>
Component: New BugsAssignee: Myles C. Maxfield <mmaxfield>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, dino, ews-watchlist, fpizlo, jonlee, justin_fan, rmorisset, saam, simon.fraser, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 192355, 192991    
Attachments:
Description Flags
WIP
none
Patch none

Myles C. Maxfield
Reported 2018-12-02 20:33:11 PST
[WHLSL] Add a handwritten lexer
Attachments
WIP (32.09 KB, patch)
2018-12-02 20:38 PST, Myles C. Maxfield
no flags
Patch (42.73 KB, patch)
2018-12-18 14:55 PST, Myles C. Maxfield
no flags
Myles C. Maxfield
Comment 1 2018-12-02 20:38:41 PST
EWS Watchlist
Comment 2 2018-12-02 20:39:52 PST
Attachment 356353 [details] did not pass style-queue: ERROR: Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] Total errors found: 1 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
Myles C. Maxfield
Comment 3 2018-12-18 14:55:38 PST
Jon Lee
Comment 4 2018-12-19 01:36:58 PST
Comment on attachment 357617 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=357617&action=review > Source/WebCore/Modules/webgpu/WHLSL/WHLSLLexer.cpp:556 > + for ( ; m_offset < m_stringView.length() && !isNewline(m_stringView[m_offset]); ++m_offset) { } Nit: consistent style of empty block.
Myles C. Maxfield
Comment 5 2018-12-19 15:01:55 PST
Comment on attachment 357617 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=357617&action=review >> Source/WebCore/Modules/webgpu/WHLSL/WHLSLLexer.cpp:556 >> + for ( ; m_offset < m_stringView.length() && !isNewline(m_stringView[m_offset]); ++m_offset) { } > > Nit: consistent style of empty block. ERROR: Source/WebCore/Modules/webgpu/WHLSL/WHLSLLexer.cpp:558: One line control clauses should not use braces. [whitespace/braces] [4] and if i use a semicolon ERROR: Source/WebCore/Modules/webgpu/WHLSL/WHLSLLexer.cpp:556: Semicolon defining empty statement for this loop. Use { } instead. [whitespace/semicolon] [5]
WebKit Commit Bot
Comment 6 2018-12-19 15:27:54 PST
Comment on attachment 357617 [details] Patch Clearing flags on attachment: 357617 Committed r239398: <https://trac.webkit.org/changeset/239398>
WebKit Commit Bot
Comment 7 2018-12-19 15:27:56 PST
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 8 2018-12-19 15:28:23 PST
Saam Barati
Comment 9 2019-01-10 01:17:51 PST
Comment on attachment 357617 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=357617&action=review > Source/WebCore/Modules/webgpu/WHLSL/WHLSLLexer.cpp:255 > +auto Lexer::recognizeKeyword(unsigned end) -> std::optional<Token::Type> Is this slow? Do you know what code LLVM emits for this? Is it actually a bunch of string compares? It might be worth doing what JSC’s lexer does. IIRC, we have a compile step that essentially compiles a function that does this.
Myles C. Maxfield
Comment 10 2019-01-10 08:45:39 PST
Yeah I thought about making a trie at compiler time but I didn’t want to add the complexity in the first patch. I’ll measure how bad the current code is to see if it’s worth it.
Note You need to log in before you can comment on or make changes to this bug.