Bug 192826

Summary: [WHLSL] Add a Visitor class
Product: WebKit Reporter: Myles C. Maxfield <mmaxfield>
Component: WebGPUAssignee: Myles C. Maxfield <mmaxfield>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, dino, fpizlo, graouts, jonlee, justin_fan, keith_miller, rmorisset, rniwa, saam, simon.fraser, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 192991    
Bug Blocks: 193007, 193360    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch dino: review+

Myles C. Maxfield
Reported 2018-12-18 13:20:03 PST
Add a Visitor class
Attachments
Patch (79.52 KB, patch)
2018-12-18 13:21 PST, Myles C. Maxfield 		no flags
DetailsFormatted DiffDiff
Patch (79.43 KB, patch)
2018-12-20 23:13 PST, Myles C. Maxfield 		no flags
DetailsFormatted DiffDiff
Patch (92.13 KB, patch)
2018-12-21 18:56 PST, Myles C. Maxfield 		no flags
DetailsFormatted DiffDiff
Patch (48.06 KB, patch)
2019-01-11 00:12 PST, Myles C. Maxfield 		no flags
DetailsFormatted DiffDiff
Patch (46.94 KB, patch)
2019-01-11 10:50 PST, Myles C. Maxfield 		dino: review+
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Myles C. Maxfield
Comment 1 2018-12-18 13:21:46 PST
Created attachment 357603 [details] Patch
Myles C. Maxfield
Comment 2 2018-12-20 23:13:16 PST
Created attachment 357930 [details] Patch
Myles C. Maxfield
Comment 3 2018-12-21 18:56:41 PST
Created attachment 358011 [details] Patch
Myles C. Maxfield
Comment 4 2019-01-11 00:12:00 PST
Created attachment 358882 [details] Patch
Myles C. Maxfield
Comment 5 2019-01-11 10:50:20 PST
Created attachment 358911 [details] Patch
Robin Morisset
Comment 6 2019-01-11 15:16:10 PST
Comment on attachment 358911 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=358911&action=review > Source/WebCore/Modules/webgpu/WHLSL/WHLSLVisitor.cpp:621 > +void Visitor::checkErrorAndVisit(Program& program) These look very repetitive. Would the following fail for some reason? ``` template<typename T> void Visitor::checkErrorAndVisist(T& x) { if (!error) visit(x); } ```
Dean Jackson
Comment 7 2019-01-11 16:34:36 PST
Comment on attachment 358911 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=358911&action=review > Source/WebCore/Modules/webgpu/WHLSL/WHLSLVisitor.cpp:2 > + * Copyright (C) 2018 Apple Inc. All rights reserved. 2019 > Source/WebCore/Modules/webgpu/WHLSL/WHLSLVisitor.cpp:105 > + // These visiting functions might add new global statements, so don't use foreach syntax. :( > Source/WebCore/Modules/webgpu/WHLSL/WHLSLVisitor.cpp:357 > + WTF::visit(WTF::makeVisitor([&](AST::NumThreadsFunctionAttribute& numThreadsFunctionAttribute) { > + checkErrorAndVisit(numThreadsFunctionAttribute); > + }), functionAttribute); Why not call it directly since there is only one object to visit? > Source/WebCore/Modules/webgpu/WHLSL/WHLSLVisitor.h:2 > + * Copyright (C) 2018 Apple Inc. All rights reserved. 2019 > Source/WebCore/Modules/webgpu/WHLSL/WHLSLVisitor.h:179 > + void checkErrorAndVisit(Program&); Could these be private/protected?
Myles C. Maxfield
Comment 8 2019-01-11 17:35:21 PST
Comment on attachment 358911 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=358911&action=review >> Source/WebCore/Modules/webgpu/WHLSL/WHLSLVisitor.cpp:621 >> +void Visitor::checkErrorAndVisit(Program& program) > > These look very repetitive. Would the following fail for some reason? > ``` > template<typename T> void Visitor::checkErrorAndVisist(T& x) > { > if (!error) > visit(x); > } > ``` Yes! Good idea.
Myles C. Maxfield
Comment 9 2019-01-11 18:46:23 PST
Committed r239893: <https://trac.webkit.org/changeset/239893>
Radar WebKit Bug Importer
Comment 10 2019-01-11 18:47:38 PST
<rdar://problem/47227226>
Note You need to log in before you can comment on or make changes to this bug.