Bug 200350

Summary: [WHLSL] Inline all native function calls
Product: WebKit Reporter: Saam Barati <saam>
Component: WebGPUAssignee: Saam Barati <saam>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, dino, ews-watchlist, fpizlo, graouts, jonlee, justin_fan, kondapallykalyan, mmaxfield, rmorisset, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 200332    
Attachments:
Description Flags
WIP
none
WIP
none
WIP
none
patch
none
patch
rmorisset: review+
patch for landing none

Saam Barati
Reported 2019-08-01 10:18:33 PDT
...
Attachments
WIP (34.83 KB, patch)
2019-08-01 15:14 PDT, Saam Barati 		no flags
DetailsFormatted DiffDiff
WIP (35.17 KB, patch)
2019-08-01 15:58 PDT, Saam Barati 		no flags
DetailsFormatted DiffDiff
WIP (68.07 KB, patch)
2019-08-02 19:30 PDT, Saam Barati 		no flags
DetailsFormatted DiffDiff
patch (48.39 KB, patch)
2019-08-05 14:32 PDT, Saam Barati 		no flags
DetailsFormatted DiffDiff
patch (48.97 KB, patch)
2019-08-05 14:43 PDT, Saam Barati 		rmorisset: review+
DetailsFormatted DiffDiff
patch for landing (48.94 KB, patch)
2019-08-05 15:50 PDT, Saam Barati 		no flags
DetailsFormatted DiffDiff
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.
Saam Barati
Comment 1 2019-08-01 15:14:03 PDT
Created attachment 375353 [details] WIP
Saam Barati
Comment 2 2019-08-01 15:58:03 PDT
Created attachment 375360 [details] WIP just need to finish up a few more of the native functions.
Saam Barati
Comment 3 2019-08-02 19:30:17 PDT
Created attachment 375477 [details] WIP I think it's done. It passes all our tests. Just need to clean up the code.
Saam Barati
Comment 4 2019-08-05 14:32:47 PDT
Created attachment 375559 [details] patch
Saam Barati
Comment 5 2019-08-05 14:33:34 PDT
Comment on attachment 375559 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=375559&action=review > Source/WebCore/platform/graphics/gpu/cocoa/GPUComputePipelineMetal.mm:38 > +#import <wtf/MonotonicTime.h> will revert changes in this file
Saam Barati
Comment 6 2019-08-05 14:33:53 PDT
Comment on attachment 375559 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=375559&action=review > Source/WebCore/Modules/webgpu/WHLSL/Metal/WHLSLNativeFunctionWriter.h:30 > +#include <wtf/text/WTFString.h> will revert
EWS Watchlist
Comment 7 2019-08-05 14:35:52 PDT
Attachment 375559 [details] did not pass style-queue: ERROR: Source/WebCore/platform/graphics/gpu/cocoa/GPUComputePipelineMetal.mm:38: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/platform/graphics/gpu/cocoa/GPUComputePipelineMetal.mm:38: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/platform/graphics/gpu/cocoa/GPUComputePipelineMetal.mm:94: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/WebCore/platform/graphics/gpu/cocoa/GPUComputePipelineMetal.mm:97: Should have a space between // and comment [whitespace/comments] [4] Total errors found: 4 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
Saam Barati
Comment 8 2019-08-05 14:43:34 PDT
Created attachment 375560 [details] patch
Robin Morisset
Comment 9 2019-08-05 15:35:29 PDT
Comment on attachment 375560 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=375560&action=review r=me > Source/WebCore/Modules/webgpu/WHLSL/Metal/WHLSLNativeFunctionWriter.cpp:247 > + "if (", args[1], " < ", args[0], ".length) ", returnName, " = &(", args[0], ".pointer[", args[1], "]);\n" nit: might be simpler/shorter with a ternary conditional.
Saam Barati
Comment 10 2019-08-05 15:45:31 PDT
Comment on attachment 375560 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=375560&action=review >> Source/WebCore/Modules/webgpu/WHLSL/Metal/WHLSLNativeFunctionWriter.cpp:247 >> + "if (", args[1], " < ", args[0], ".length) ", returnName, " = &(", args[0], ".pointer[", args[1], "]);\n" > > nit: might be simpler/shorter with a ternary conditional. sounds good
Saam Barati
Comment 11 2019-08-05 15:50:59 PDT
Created attachment 375573 [details] patch for landing
WebKit Commit Bot
Comment 12 2019-08-05 17:03:40 PDT
Comment on attachment 375573 [details] patch for landing Clearing flags on attachment: 375573 Committed r248280: <https://trac.webkit.org/changeset/248280>
WebKit Commit Bot
Comment 13 2019-08-05 17:03:42 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 14 2019-08-05 17:04:19 PDT
<rdar://problem/53965153>
Note You need to log in before you can comment on or make changes to this bug.