Bin Miao (Gerrit)

unread,

Jun 24, 2024, 5:41:01 AM (9 days ago) Jun 24

to ningxin hu, Junwei Fu, Mingming1 Xu, Lisha Guo, Shiyi Zou, Chromium LUCI CQ, chromium...@chromium.org, Jiewei Qian, blundell+...@chromium.org

Attention needed from ningxin hu

Bin Miao added 1 comment

Patchset-level comments

File-level comment, Patchset 2 (Latest):

Bin Miao . resolved

This CL uses the decompose operation to emulate the gelu for DirectML
backend when the DML_FEATURE_LEVEL < 5_1. PTAL, thanks!

Open in Gerrit

Related details

Attention is currently required from:

ningxin hu

Submit Requirements:

Code-Coverage
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

ningxin hu (Gerrit)

unread,

Jun 25, 2024, 12:29:11 AM (8 days ago) Jun 25

to Bin Miao, Junwei Fu, Mingming1 Xu, Lisha Guo, Shiyi Zou, Chromium LUCI CQ, chromium...@chromium.org, Jiewei Qian, blundell+...@chromium.org

Attention needed from Bin Miao

ningxin hu added 8 comments

File services/webnn/dml/graph_impl_dml.cc

Line 2830, Patchset 3 (Latest):

  const NodeOutput* input =
      GetNodeOutputForOperand(id_to_node_output_map, gelu->input_operand_id);
  const TensorDesc& input_tensor_desc = input->GetTensorDesc();

ningxin hu . unresolved

Declare the variables close to where they are used.

Line 2833, Patchset 3 (Latest): DML_TENSOR_DATA_TYPE input_data_type = input_tensor_desc.GetDataType();

ningxin hu . unresolved

If we use `GetTensorDataType(data_type)`, this might be unnecessary?

Line 2854, Patchset 3 (Latest): const TensorDesc sqrt_output_tensor_desc = TensorDesc(input_data_type, {1});

ningxin hu . unresolved

`GetTensorDataType(data_type)`?

Line 2854, Patchset 3 (Latest): const TensorDesc sqrt_output_tensor_desc = TensorDesc(input_data_type, {1});

ningxin hu . unresolved

`/*dimensions*/ {1}`

Line 2956, Patchset 3 (Latest): "to create mutiply input operator."));

ningxin hu . unresolved

Remove "input", "multiply operator" is fine.

Line 2956, Patchset 3 (Latest): "to create mutiply input operator."));

ningxin hu . unresolved

typo: multiply

Line 2993, Patchset 3 (Latest): "For the emulation of gelu: failed to create mutiply "

ningxin hu . unresolved

Typo

Line 2994, Patchset 3 (Latest): "constant operator."));

ningxin hu . unresolved

ditto, remove "constant"

Open in Gerrit

Related details

Attention is currently required from:

Bin Miao

Submit Requirements:

Code-Coverage
Code-Review

No-Unresolved-Comments

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Bin Miao (Gerrit)

unread,

Jun 25, 2024, 2:08:20 AM (8 days ago) Jun 25

to AyeAye, ningxin hu, Junwei Fu, Mingming1 Xu, Lisha Guo, Shiyi Zou, Chromium LUCI CQ, chromium...@chromium.org, Jiewei Qian, blink-...@chromium.org, blundell+...@chromium.org

Attention needed from ningxin hu

Bin Miao added 9 comments

Patchset-level comments

File-level comment, Patchset 4 (Latest):

Bin Miao . resolved

Updated the code to reach the comments and updated the WPT baseline results. Please take another look, thanks a lot!

File services/webnn/dml/graph_impl_dml.cc

Line 2830, Patchset 3: const NodeOutput* input =


      GetNodeOutputForOperand(id_to_node_output_map, gelu->input_operand_id);
  const TensorDesc& input_tensor_desc = input->GetTensorDesc();

ningxin hu . resolved

Declare the variables close to where they are used.

Bin Miao

Done

Line 2833, Patchset 3: DML_TENSOR_DATA_TYPE input_data_type = input_tensor_desc.GetDataType();

ningxin hu . resolved

If we use `GetTensorDataType(data_type)`, this might be unnecessary?

Bin Miao

Done

Line 2854, Patchset 3: const TensorDesc sqrt_output_tensor_desc = TensorDesc(input_data_type, {1});

ningxin hu . resolved

`/*dimensions*/ {1}`

Bin Miao

Done

Line 2854, Patchset 3: const TensorDesc sqrt_output_tensor_desc = TensorDesc(input_data_type, {1});

ningxin hu . resolved

`GetTensorDataType(data_type)`?

Bin Miao

Done

Line 2956, Patchset 3: "to create mutiply input operator."));

ningxin hu . resolved

Remove "input", "multiply operator" is fine.

Bin Miao

Done

Line 2956, Patchset 3: "to create mutiply input operator."));

ningxin hu . resolved

typo: multiply

Bin Miao

Done

Line 2993, Patchset 3: "For the emulation of gelu: failed to create mutiply "

ningxin hu . resolved

Typo

Bin Miao

Done

Line 2994, Patchset 3: "constant operator."));

ningxin hu . resolved

ditto, remove "constant"

Bin Miao

Done

Open in Gerrit

Related details

Attention is currently required from:

ningxin hu

Submit Requirements:

Code-Coverage
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

ningxin hu (Gerrit)

unread,

Jun 25, 2024, 10:53:09 PM (7 days ago) Jun 25

to Bin Miao, Rafael Cintron, Dwayne Robinson, AyeAye, Junwei Fu, Mingming1 Xu, Lisha Guo, Shiyi Zou, Chromium LUCI CQ, chromium...@chromium.org, Jiewei Qian, blink-...@chromium.org, blundell+...@chromium.org

Attention needed from Bin Miao and Rafael Cintron

ningxin hu voted and added 1 comment

Votes added by ningxin hu

Code-Review

+1

1 comment

Patchset-level comments

File-level comment, Patchset 4 (Latest):

ningxin hu . resolved

lgtm % other reviewer's comments

Adding @rafael....@microsoft.com and @dwa...@microsoft.com for review. Thanks!

Open in Gerrit

Related details

Attention is currently required from:

Bin Miao
Rafael Cintron

Submit Requirements:

Code-Coverage
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Dwayne Robinson (Gerrit)

unread,

Jun 26, 2024, 3:10:35 AM (7 days ago) Jun 26

to Bin Miao, Rafael Cintron, ningxin hu, AyeAye, Junwei Fu, Mingming1 Xu, Lisha Guo, Shiyi Zou, Chromium LUCI CQ, chromium...@chromium.org, Jiewei Qian, blink-...@chromium.org, blundell+...@chromium.org

Attention needed from Bin Miao and Rafael Cintron

Dwayne Robinson added 2 comments

Patchset-level comments

File-level comment, Patchset 4 (Latest):

Dwayne Robinson . resolved

Small comment, else LGTM Bin. Thanks for adding the emulation path to avoid failures on Win11 21H2 & 22H2.

File services/webnn/dml/graph_impl_dml.cc

Line 2944, Patchset 4 (Latest):

  const TensorDesc second_mul_output_tensor_desc =
      TensorDesc(GetTensorDataType(data_type), /*dimensions*/ input_dimensions);

Dwayne Robinson . unresolved

Does just `const TensorDesc second_mul_output_tensor_desc = input_tensor_desc;` work here? (or maybe even just a reference rename like `const TensorDesc& second_mul_output_tensor_desc = input_tensor_desc;` to avoid the copy entirely) Except for the broadcasted constants, every one of these elementwise tensor descs has the same data type and dimensions, and since there is a lot of tediousness here and potential for little typos, I'm looking for ways to reduce the length for maintainers.

Open in Gerrit

Related details

Attention is currently required from:

Bin Miao
Rafael Cintron

Submit Requirements:

Code-Coverage
Code-Review

No-Unresolved-Comments

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Bin Miao (Gerrit)

unread,

Jun 26, 2024, 5:56:22 AM (7 days ago) Jun 26

to Rafael Cintron, ningxin hu, Dwayne Robinson, AyeAye, Junwei Fu, Mingming1 Xu, Lisha Guo, Shiyi Zou, Chromium LUCI CQ, chromium...@chromium.org, Jiewei Qian, blink-revie...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org

Attention needed from Dwayne Robinson, Rafael Cintron and ningxin hu

Bin Miao added 2 comments

Patchset-level comments

File-level comment, Patchset 5 (Latest):

Bin Miao . resolved

Optimized the code and add a test case for gelu WPT conformance test to check result when input is a constant. Since the WPT conformance test already covers the gelu operator test in services unittests, the gelu operator part is deleted. Please take another look, thanks a lot!

File services/webnn/dml/graph_impl_dml.cc

Line 2944, Patchset 4:

  const TensorDesc second_mul_output_tensor_desc =
      TensorDesc(GetTensorDataType(data_type), /*dimensions*/ input_dimensions);

Dwayne Robinson . resolved

Does just `const TensorDesc second_mul_output_tensor_desc = input_tensor_desc;` work here? (or maybe even just a reference rename like `const TensorDesc& second_mul_output_tensor_desc = input_tensor_desc;` to avoid the copy entirely) Except for the broadcasted constants, every one of these elementwise tensor descs has the same data type and dimensions, and since there is a lot of tediousness here and potential for little typos, I'm looking for ways to reduce the length for maintainers.

Bin Miao

Thank you for your comments and suggestion and this can indeed simplify the code and reduce copy. BTW, we need to use `const TensorDesc& xxx_output_tensor_desc = output_tensor_desc;` to avoid the case where the input is a constant.
Also add a test case to WPT to check if the input is a constant and please take another look.

Open in Gerrit

Related details

Attention is currently required from:

Dwayne Robinson
Rafael Cintron
ningxin hu

Submit Requirements:

Code-Coverage
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Dwayne Robinson (Gerrit)

unread,

Jun 26, 2024, 8:26:13 PM (6 days ago) Jun 26

to Bin Miao, Rafael Cintron, ningxin hu, AyeAye, Junwei Fu, Mingming1 Xu, Lisha Guo, Shiyi Zou, Chromium LUCI CQ, chromium...@chromium.org, Jiewei Qian, blink-revie...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org

Attention needed from Bin Miao, Rafael Cintron and ningxin hu

Dwayne Robinson added 2 comments

Patchset-level comments

File-level comment, Patchset 5 (Latest):

Dwayne Robinson . resolved

👍

File services/webnn/dml/graph_impl_dml.cc

Line 2944, Patchset 4:

  const TensorDesc second_mul_output_tensor_desc =
      TensorDesc(GetTensorDataType(data_type), /*dimensions*/ input_dimensions);

Dwayne Robinson . resolved

Does just `const TensorDesc second_mul_output_tensor_desc = input_tensor_desc;` work here? (or maybe even just a reference rename like `const TensorDesc& second_mul_output_tensor_desc = input_tensor_desc;` to avoid the copy entirely) Except for the broadcasted constants, every one of these elementwise tensor descs has the same data type and dimensions, and since there is a lot of tediousness here and potential for little typos, I'm looking for ways to reduce the length for maintainers.

Bin Miao

Thank you for your comments and suggestion and this can indeed simplify the code and reduce copy. BTW, we need to use `const TensorDesc& xxx_output_tensor_desc = output_tensor_desc;` to avoid the case where the input is a constant.
Also add a test case to WPT to check if the input is a constant and please take another look.

Dwayne Robinson

Nice.

Open in Gerrit

Related details

Attention is currently required from:

Bin Miao
Rafael Cintron
ningxin hu

satisfied_requirement

unsatisfied_requirement

open

diffy

Bin Miao (Gerrit)

unread,

Jun 27, 2024, 5:12:09 AM (6 days ago) Jun 27

to Rafael Cintron, ningxin hu, Dwayne Robinson, AyeAye, Junwei Fu, Mingming1 Xu, Lisha Guo, Shiyi Zou, Chromium LUCI CQ, chromium...@chromium.org, Jiewei Qian, mac-r...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org

Attention needed from Rafael Cintron and ningxin hu

Bin Miao added 1 comment

Patchset-level comments

File-level comment, Patchset 7 (Latest):

Bin Miao . resolved

Updated the WPT baseline results and the CL passed all the tests. Please take another look @rafael....@microsoft.com @ningx...@intel.com thanks a lot!

Open in Gerrit

Related details

Attention is currently required from:

Rafael Cintron
ningxin hu

Submit Requirements:

Code-Coverage
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

ningxin hu (Gerrit)

unread,

Jun 30, 2024, 10:25:23 PM (2 days ago) Jun 30

to Bin Miao, Rafael Cintron, Dwayne Robinson, AyeAye, Junwei Fu, Mingming1 Xu, Lisha Guo, Shiyi Zou, Chromium LUCI CQ, chromium...@chromium.org, Jiewei Qian, mac-r...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org

Attention needed from Bin Miao and Rafael Cintron

ningxin hu voted and added 1 comment

Votes added by ningxin hu

Code-Review

+1

1 comment

Patchset-level comments

File-level comment, Patchset 7 (Latest):

ningxin hu . resolved

still lgtm

Open in Gerrit

Related details

Attention is currently required from:

Bin Miao
Rafael Cintron

Submit Requirements:

Code-Coverage
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Rafael Cintron (Gerrit)

unread,

6:19 PM (5 hours ago) 6:19 PM

to Bin Miao, ningxin hu, Dwayne Robinson, AyeAye, Junwei Fu, Mingming1 Xu, Lisha Guo, Shiyi Zou, Chromium LUCI CQ, chromium...@chromium.org, Jiewei Qian, mac-r...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org

Attention needed from Bin Miao

Rafael Cintron voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

Bin Miao

Submit Requirements:

Code-Coverage
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Blink W3C Test Autoroller (Gerrit)

unread,

6:36 PM (5 hours ago) 6:36 PM

to Bin Miao, Rafael Cintron, ningxin hu, Dwayne Robinson, AyeAye, Junwei Fu, Mingming1 Xu, Lisha Guo, Shiyi Zou, Chromium LUCI CQ, chromium...@chromium.org, Jiewei Qian, mac-r...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org

Attention needed from Bin Miao

Message from Blink W3C Test Autoroller

Exportable changes to web-platform-tests were detected in this CL and a pull request in the upstream repo has been made: https://github.com/web-platform-tests/wpt/pull/46971.

When this CL lands, the bot will automatically merge the PR on GitHub if the required GitHub checks pass; otherwise, ecosystem-infra@ team will triage the failures and may contact you.

WPT Export docs:
https://chromium.googlesource.com/chromium/src/+/main/docs/testing/web_platform_tests.md#Automatic-export-process

Open in Gerrit

Related details

Attention is currently required from:

Bin Miao

Submit Requirements:

Code-Coverage
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Chromium LUCI CQ (Gerrit)

unread,

9:27 PM (2 hours ago) 9:27 PM

to Bin Miao, Blink W3C Test Autoroller, Rafael Cintron, ningxin hu, Dwayne Robinson, AyeAye, Junwei Fu, Mingming1 Xu, Lisha Guo, Shiyi Zou, chromium...@chromium.org, Jiewei Qian, mac-r...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org

Chromium LUCI CQ submitted the change

Change information

Commit message:

WebNN: Emulate gelu with decomposed operations



This CL uses the decompose operation to emulate the gelu for DirectML

backend when the DirectML feature level is lower than 5.1.

Bug: 40206287

Change-Id: Iaf472b53b4c6bdcad80ff02dd6b208b4e2df9a3c

Cq-Include-Trybots: luci.chromium.try:win11-blink-rel,mac14.arm64-blink-rel,mac14-blink-rel

Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5649158

Commit-Queue: Bin Miao <bin....@intel.com>

Reviewed-by: ningxin hu <ningx...@intel.com>

Reviewed-by: Rafael Cintron <rafael....@microsoft.com>

Cr-Commit-Position: refs/heads/main@{#1322514}

Files:

M services/webnn/dml/graph_impl_dml.cc
M services/webnn/webnn_graph_impl_backend_test.cc
M third_party/blink/web_tests/external/wpt/webnn/resources/test_data/gelu.json
M third_party/blink/web_tests/platform/mac/virtual/webnn-service-on-cpu/external/wpt/webnn/conformance_tests/gelu.https.any.worker_cpu-expected.txt
M third_party/blink/web_tests/platform/mac/virtual/webnn-service-on-cpu/external/wpt/webnn/conformance_tests/gelu.https.any_cpu-expected.txt
M third_party/blink/web_tests/platform/mac/virtual/webnn-service-with-gpu/external/wpt/webnn/conformance_tests/gelu.https.any.worker_gpu-expected.txt
M third_party/blink/web_tests/platform/mac/virtual/webnn-service-with-gpu/external/wpt/webnn/conformance_tests/gelu.https.any_gpu-expected.txt
D third_party/blink/web_tests/platform/win/virtual/webnn-service-with-gpu/external/wpt/webnn/conformance_tests/gelu.https.any.worker_gpu-expected.txt
D third_party/blink/web_tests/platform/win/virtual/webnn-service-with-gpu/external/wpt/webnn/conformance_tests/gelu.https.any_gpu-expected.txt
M third_party/blink/web_tests/virtual/webnn-service-on-npu/external/wpt/webnn/conformance_tests/gelu.https.any.worker_npu-expected.txt
M third_party/blink/web_tests/virtual/webnn-service-on-npu/external/wpt/webnn/conformance_tests/gelu.https.any_npu-expected.txt

Change size: L

Delta: 11 files changed, 272 insertions(+), 109 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Rafael Cintron, +1 by ningxin hu

Open in Gerrit

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

open

diffy

satisfied_requirement

Blink W3C Test Autoroller (Gerrit)

unread,

10:58 PM (1 hour ago) 10:58 PM

to Bin Miao, Chromium LUCI CQ, Rafael Cintron, ningxin hu, Dwayne Robinson, AyeAye, Junwei Fu, Mingming1 Xu, Lisha Guo, Shiyi Zou, chromium...@chromium.org, Jiewei Qian, mac-r...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org

Message from Blink W3C Test Autoroller

The WPT PR for this CL has been merged upstream! https://github.com/web-platform-tests/wpt/pull/46971

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

Code-Coverage
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Reply all

Reply to author

Forward

WebNN: Emulate gelu with decomposed operations [chromium/src : main]

Bin Miao (Gerrit)

Bin Miao added 1 comment

Related details

ningxin hu (Gerrit)

ningxin hu added 8 comments

Related details

Bin Miao (Gerrit)

Bin Miao added 9 comments

Related details

ningxin hu (Gerrit)

ningxin hu voted and added 1 comment

Votes added by ningxin hu

1 comment

Related details

Dwayne Robinson (Gerrit)

Dwayne Robinson added 2 comments

Related details

Bin Miao (Gerrit)

Bin Miao added 2 comments

Related details

Dwayne Robinson (Gerrit)

Dwayne Robinson added 2 comments

Related details

Bin Miao (Gerrit)

Bin Miao added 1 comment

Related details

ningxin hu (Gerrit)

ningxin hu voted and added 1 comment

Votes added by ningxin hu

1 comment

Related details

Rafael Cintron (Gerrit)

Rafael Cintron voted Code-Review+1

Related details

Blink W3C Test Autoroller (Gerrit)

Message from Blink W3C Test Autoroller

Related details

Chromium LUCI CQ (Gerrit)

Chromium LUCI CQ submitted the change

Change information

Blink W3C Test Autoroller (Gerrit)

Message from Blink W3C Test Autoroller

Related details