Bug 200749 - [WHLSL] Make operator== native and add bool matrices
Summary: [WHLSL] Make operator== native and add bool matrices
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: 200699
  Show dependency treegraph
 
Reported: 2019-08-14 18:49 PDT by Saam Barati
Modified: 2019-08-16 16:02 PDT (History)
10 users (show)

See Also:


Attachments
patch (16.53 KB, patch)
2019-08-15 18:11 PDT, Saam Barati
no flags Details | Formatted Diff | Diff
patch (16.39 KB, patch)
2019-08-15 18:21 PDT, Saam Barati
mmaxfield: review-
Details | Formatted Diff | Diff
patch (37.11 KB, patch)
2019-08-16 12:59 PDT, Saam Barati
no flags Details | Formatted Diff | Diff
patch (37.86 KB, patch)
2019-08-16 13:23 PDT, Saam Barati
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Saam Barati 2019-08-14 18:49:40 PDT
...
Comment 1 Saam Barati 2019-08-15 18:11:42 PDT
Created attachment 376458 [details]
patch
Comment 2 Myles C. Maxfield 2019-08-15 18:17:26 PDT
Comment on attachment 376458 [details]
patch

We should make sure we modify the spec to include the new functionality.
Comment 3 Saam Barati 2019-08-15 18:18:42 PDT
Comment on attachment 376458 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=376458&action=review

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLStandardLibrary.txt:16299
> +native bool2 operator==(bool2 a, bool2 b) ;

oops, I'll fix styling on these.
Comment 4 Saam Barati 2019-08-15 18:21:00 PDT
Created attachment 376459 [details]
patch
Comment 5 Myles C. Maxfield 2019-08-15 18:43:40 PDT
Comment on attachment 376458 [details]
patch

This seems like not enough tests for adding a whole new type. By my count, this patch adds 32 functions and modifies 30 functions, but only tests 3 of them?
Comment 6 Myles C. Maxfield 2019-08-15 19:29:19 PDT
Comment on attachment 376459 [details]
patch

Inequalities need to be matrices too
Comment 7 Myles C. Maxfield 2019-08-15 19:36:41 PDT
checkOperatorOverload and writeNativeType and Intrinsics::addMatrix need to be modified
Comment 9 Saam Barati 2019-08-16 12:58:07 PDT
(In reply to Myles C. Maxfield from comment #6)
> Comment on attachment 376459 [details]
> patch
> 
> Inequalities need to be matrices too

doing this in:
https://bugs.webkit.org/show_bug.cgi?id=200823
Comment 10 Saam Barati 2019-08-16 12:59:40 PDT
Created attachment 376520 [details]
patch
Comment 11 Saam Barati 2019-08-16 13:23:55 PDT
Created attachment 376528 [details]
patch
Comment 13 WebKit Commit Bot 2019-08-16 13:59:35 PDT
Comment on attachment 376528 [details]
patch

Clearing flags on attachment: 376528

Committed r248795: <https://trac.webkit.org/changeset/248795>
Comment 14 WebKit Commit Bot 2019-08-16 13:59:37 PDT
All reviewed patches have been landed.  Closing bug.
Comment 15 Radar WebKit Bug Importer 2019-08-16 14:00:22 PDT
<rdar://problem/54406208>
Comment 16 Truitt Savell 2019-08-16 15:26:18 PDT
The new test webgpu/whlsl/bool-matrix.html

added in https://trac.webkit.org/changeset/248795/webkit

is failing on Mojave.

History:
http://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=webgpu%2Fwhlsl%2Fbool-matrix.html

Diff:
--- /Volumes/Data/slave/mojave-release-tests-wk2/build/layout-test-results/webgpu/whlsl/bool-matrix-expected.txt
+++ /Volumes/Data/slave/mojave-release-tests-wk2/build/layout-test-results/webgpu/whlsl/bool-matrix-actual.txt
@@ -1,11 +1,11 @@
 
 PASS boolMatrix 
-PASS boolMatrix2 
-PASS boolMatrix3 
-PASS boolMatrix4 
+FAIL boolMatrix2 promise_test: Unhandled rejection with value: object "Error: GPUDevice.createComputePipeline(): WHLSL compile error: 3:25: Cannot resolve function call to a concrete callee. Make sure you are using compatible types."
+FAIL boolMatrix3 promise_test: Unhandled rejection with value: object "Error: GPUDevice.createComputePipeline(): WHLSL compile error: 3:25: Cannot resolve function call to a concrete callee. Make sure you are using compatible types."
+FAIL boolMatrix4 promise_test: Unhandled rejection with value: object "Error: GPUDevice.createComputePipeline(): WHLSL compile error: 3:25: Cannot resolve function call to a concrete callee. Make sure you are using compatible types."
 PASS boolMatrix5 
-PASS boolMatrixAny 
-PASS boolMatrixAny2 
-PASS boolMatrixAll 
-PASS boolMatrixAll2 
+FAIL boolMatrixAny promise_test: Unhandled rejection with value: object "Error: GPUDevice.createComputePipeline(): WHLSL compile error: 3:25: Cannot resolve function call to a concrete callee. Make sure you are using compatible types."
+FAIL boolMatrixAny2 promise_test: Unhandled rejection with value: object "Error: GPUDevice.createComputePipeline(): WHLSL compile error: 3:25: Cannot resolve function call to a concrete callee. Make sure you are using compatible types."
+FAIL boolMatrixAll promise_test: Unhandled rejection with value: object "Error: GPUDevice.createComputePipeline(): WHLSL compile error: 3:25: Cannot resolve function call to a concrete callee. Make sure you are using compatible types."
+FAIL boolMatrixAll2 promise_test: Unhandled rejection with value: object "Error: GPUDevice.createComputePipeline(): WHLSL compile error: 3:25: Cannot resolve function call to a concrete callee. Make sure you are using compatible types."
Comment 17 Saam Barati 2019-08-16 15:48:16 PDT
(In reply to Truitt Savell from comment #16)
> The new test webgpu/whlsl/bool-matrix.html
> 
> added in https://trac.webkit.org/changeset/248795/webkit
> 
> is failing on Mojave.
> 
> History:
> http://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.
> html#showAllRuns=true&tests=webgpu%2Fwhlsl%2Fbool-matrix.html
> 
> Diff:
> ---
> /Volumes/Data/slave/mojave-release-tests-wk2/build/layout-test-results/
> webgpu/whlsl/bool-matrix-expected.txt
> +++
> /Volumes/Data/slave/mojave-release-tests-wk2/build/layout-test-results/
> webgpu/whlsl/bool-matrix-actual.txt
> @@ -1,11 +1,11 @@
>  
>  PASS boolMatrix 
> -PASS boolMatrix2 
> -PASS boolMatrix3 
> -PASS boolMatrix4 
> +FAIL boolMatrix2 promise_test: Unhandled rejection with value: object
> "Error: GPUDevice.createComputePipeline(): WHLSL compile error: 3:25: Cannot
> resolve function call to a concrete callee. Make sure you are using
> compatible types."
> +FAIL boolMatrix3 promise_test: Unhandled rejection with value: object
> "Error: GPUDevice.createComputePipeline(): WHLSL compile error: 3:25: Cannot
> resolve function call to a concrete callee. Make sure you are using
> compatible types."
> +FAIL boolMatrix4 promise_test: Unhandled rejection with value: object
> "Error: GPUDevice.createComputePipeline(): WHLSL compile error: 3:25: Cannot
> resolve function call to a concrete callee. Make sure you are using
> compatible types."
>  PASS boolMatrix5 
> -PASS boolMatrixAny 
> -PASS boolMatrixAny2 
> -PASS boolMatrixAll 
> -PASS boolMatrixAll2 
> +FAIL boolMatrixAny promise_test: Unhandled rejection with value: object
> "Error: GPUDevice.createComputePipeline(): WHLSL compile error: 3:25: Cannot
> resolve function call to a concrete callee. Make sure you are using
> compatible types."
> +FAIL boolMatrixAny2 promise_test: Unhandled rejection with value: object
> "Error: GPUDevice.createComputePipeline(): WHLSL compile error: 3:25: Cannot
> resolve function call to a concrete callee. Make sure you are using
> compatible types."
> +FAIL boolMatrixAll promise_test: Unhandled rejection with value: object
> "Error: GPUDevice.createComputePipeline(): WHLSL compile error: 3:25: Cannot
> resolve function call to a concrete callee. Make sure you are using
> compatible types."
> +FAIL boolMatrixAll2 promise_test: Unhandled rejection with value: object
> "Error: GPUDevice.createComputePipeline(): WHLSL compile error: 3:25: Cannot
> resolve function call to a concrete callee. Make sure you are using
> compatible types."

Looking into this now. It doesn't seem to be failing for me. So I'll update my checkout
Comment 18 Saam Barati 2019-08-16 15:57:03 PDT
Looks like a bad merge when I rebased and things got moved around in the standard library :/
Comment 19 Saam Barati 2019-08-16 15:58:21 PDT
(In reply to Saam Barati from comment #18)
> Looks like a bad merge when I rebased and things got moved around in the
> standard library :/

This breaks things because we have a particular organization in the standard library we depend on when parsing functions with particular names. Fix forthcoming
Comment 20 Saam Barati 2019-08-16 16:02:35 PDT
fixed in:
https://trac.webkit.org/changeset/248801/webkit