Bug 200460

Summary:

[WHLSL] Simplify and eliminate redundant work in WHLSLFunctionWriter.cpp

Product:

WebKit

Reporter:

Robin Morisset <rmorisset>

Component:

WebGPU

Assignee:

Robin Morisset <rmorisset>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, ews-watchlist, mmaxfield, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	rmorisset: review-, rmorisset: commit-queue-
Patch	mmaxfield: review+, mmaxfield: commit-queue-
Patch	none

Robin Morisset

Reported 2019-08-05 17:25:07 PDT

WHLSLFunctionWriter.cpp has a few blatantly unnecessary/inneficient parts: - FunctionDeclarationWriter is a subclass of Visitor for no reason, it could just be a stand-alone function - We convert a few times between StringBuilder and String. We could just keep a single StringBuilder throughout. - We give a name to every function in functionMapping, even those that we know at that point not to be reachable. - When computing reachable functions we can visit each function many times (exponential complexity, same problem that we already fixed in the recursion checker)

Attachments
Patch (13.25 KB, patch) 2019-08-05 17:31 PDT, Robin Morisset	rmorisset: review- rmorisset: commit-queue-	Details Formatted Diff Diff
Patch (11.77 KB, patch) 2019-08-05 18:32 PDT, Robin Morisset	mmaxfield: review+ mmaxfield: commit-queue-	Details Formatted Diff Diff
Patch (5.19 KB, patch) 2019-08-07 11:28 PDT, Robin Morisset	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Robin Morisset

Comment 1 2019-08-05 17:31:29 PDT

Created attachment 375586 [details] Patch

Robin Morisset

Comment 2 2019-08-05 17:44:38 PDT

Comment on attachment 375586 [details] Patch It conflicts with the patch that inlines native functions.

Robin Morisset

Comment 3 2019-08-05 18:32:19 PDT

Created attachment 375591 [details] Patch Properly rebased. One of the simplifications is gone because it no longer makes sense (we no longer want to emit native function declarations, even if they're reachable).

EWS Watchlist

Comment 4 2019-08-05 18:35:05 PDT

Attachment 375591 [details] did not pass style-queue: ERROR: Source/WebCore/Modules/webgpu/WHLSL/Metal/WHLSLFunctionWriter.cpp:740: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WebCore/Modules/webgpu/WHLSL/Metal/WHLSLFunctionWriter.cpp:767: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] Total errors found: 2 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.

Robin Morisset

Comment 5 2019-08-07 11:28:20 PDT

Created attachment 375721 [details] Patch Rebased, most of the part disappeared because it was duplicated in https://bugs.webkit.org/attachment.cgi?id=375632&action=prettypatch

WebKit Commit Bot

Comment 6 2019-08-07 12:58:21 PDT

Comment on attachment 375721 [details] Patch Clearing flags on attachment: 375721 Committed r248384: <https://trac.webkit.org/changeset/248384>

WebKit Commit Bot

Comment 7 2019-08-07 12:58:23 PDT

All reviewed patches have been landed. Closing bug.

Radar WebKit Bug Importer

Comment 8 2019-08-07 12:59:20 PDT

<rdar://problem/54046511>

Note You need to log in before you can comment on or make changes to this bug.