Skip to content

[DML EP] Revert DML's cpu fallback logic - #13605

Merged
Patrice Vignola (PatriceVignola) merged 1 commit into
mainfrom
user/pavignol/fix-dml-cpu-fallback-logic
Nov 10, 2022
Merged

[DML EP] Revert DML's cpu fallback logic#13605
Patrice Vignola (PatriceVignola) merged 1 commit into
mainfrom
user/pavignol/fix-dml-cpu-fallback-logic

Conversation

@PatriceVignola

@PatriceVignola Patrice Vignola (PatriceVignola) commented Nov 9, 2022

Copy link
Copy Markdown
Contributor

Description

Revert DML's CPU fallback logic from #13442.

Motivation and Context

Although the logic works great in many models that have good DML coverage, it makes perf worse in some models where many operators are missing DML coverage (e.g. int64). Overall, the right fix seems to instead implement the operator on DML even though it almost always falls back to the CPU, just for the sake of having a registration.

@fdwr Dwayne Robinson (fdwr) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deja vu.

@Lafi7e

Copy link
Copy Markdown
Contributor

I found that the implementation of GetCapability for DML EP is different from other EPs such as CUDA. So a possible fix for your case is to follow the way CUDA EP does, instead of put all nodes as candidates passed to GetCpuPreferredNodes, just pass those nodes supported by target EP (DML here) to GetCpuPreferredNodes. Then inside GetCpuPreferredNodes, you will not hit the assertion of "ORT_ENFORCE(kernel_info != nullptr);".

MS (simon-moo) pushed a commit to simon-moo/onnxruntime that referenced this pull request Dec 21, 2022
### Description
Revert DML's CPU fallback logic from
microsoft#13442.

### Motivation and Context
Although the logic works great in many models that have good DML
coverage, it makes perf worse in some models where many operators are
missing DML coverage (e.g. int64). Overall, the right fix seems to
instead implement the operator on DML even though it almost always falls
back to the CPU, just for the sake of having a registration.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants