-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WebNN] Improve data type check of slice op #22988
base: main
Are you sure you want to change the base?
Conversation
@Honry PTAL, thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @shiyi9801, data type check is not rigorous in WebNN EP, especially for those ONNX ops that need to be supported by decomposed WebNN ops. This is a good starting.👍
if (!ReadIntArrayFrom1DTensor(*initializers.at(input_defs[4]->Name()), steps, logger)) | ||
return false; | ||
if (std::any_of(steps.begin(), steps.end(), [](int64_t step) { return step < 0; })) { | ||
if (!IsDataTypeSupportedByWebNNOp(op_type, "reverse", input_type, wnn_limits, "input", "data", logger)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can simplify this by using a parameter webnn_op_type
, set it to reverse
or slice
according to the step
value.
Then only call IsDataTypeSupportedByWebNNOp
once.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
slice
always needs to be checked (except for a few cases like steps=1, start=0, end=input_dim) so we still have to call IsDataTypeSupportedByWebNNOp()
twice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, sorry, I missed this.
Co-authored-by: Wanming Lin <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
A follow-up of [WebNN] Support negative steps for slice. Slice op is emulated by reverse+slice when steps < 0 so
SliceOpBuilder::HasSupportedInputsImpl()
should also check the supported data types of reverse.