Bug 200350 - [WHLSL] Inline all native function calls
Summary: [WHLSL] Inline all native function calls
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGPU (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Saam Barati
URL:
Keywords: InRadar
Depends on:
Blocks: 200332
  Show dependency tree / graph
 
Reported: 2019-08-01 10:18 PDT by Saam Barati
Modified: 2019-08-05 17:04 PDT (History)
11 users (show)

See Also:

Attachments
WIP (34.83 KB, patch)
2019-08-01 15:14 PDT, Saam Barati 		no flags Details | Formatted Diff | Diff
WIP (35.17 KB, patch)
2019-08-01 15:58 PDT, Saam Barati 		no flags Details | Formatted Diff | Diff
WIP (68.07 KB, patch)
2019-08-02 19:30 PDT, Saam Barati 		no flags Details | Formatted Diff | Diff
patch (48.39 KB, patch)
2019-08-05 14:32 PDT, Saam Barati 		no flags Details | Formatted Diff | Diff
patch (48.97 KB, patch)
2019-08-05 14:43 PDT, Saam Barati 		rmorisset: review+
Details | Formatted Diff | Diff
patch for landing (48.94 KB, patch)
2019-08-05 15:50 PDT, Saam Barati 		no flags Details | Formatted Diff | Diff
Show Obsolete (5) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Saam Barati 2019-08-01 10:18:33 PDT 
...
Comment 1 Saam Barati 2019-08-01 15:14:03 PDT 
Created attachment 375353 [details]
WIP
Comment 2 Saam Barati 2019-08-01 15:58:03 PDT 
Created attachment 375360 [details]
WIP

just need to finish up a few more of the native functions.
Comment 3 Saam Barati 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.
Comment 4 Saam Barati 2019-08-05 14:32:47 PDT 
Created attachment 375559 [details]
patch
Comment 5 Saam Barati 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
Comment 6 Saam Barati 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
Comment 7 EWS Watchlist 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.
Comment 8 Saam Barati 2019-08-05 14:43:34 PDT 
Created attachment 375560 [details]
patch
Comment 9 Robin Morisset 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.
Comment 10 Saam Barati 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
Comment 11 Saam Barati 2019-08-05 15:50:59 PDT 
Created attachment 375573 [details]
patch for landing
Comment 12 WebKit Commit Bot 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>
Comment 13 WebKit Commit Bot 2019-08-05 17:03:42 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 14 Radar WebKit Bug Importer 2019-08-05 17:04:19 PDT 
<rdar://problem/53965153>