Skip to content

Fix issue w/ DML batched readback size#28680

Merged
adrastogi merged 5 commits into
mainfrom
adrastogi/dml-overflow
Jun 6, 2026
Merged

Fix issue w/ DML batched readback size#28680
adrastogi merged 5 commits into
mainfrom
adrastogi/dml-overflow

Conversation

@adrastogi

Copy link
Copy Markdown
Contributor

Description

Fixes DML batched GPU readback sizing to avoid uint32_t aggregate wraparound, which could undersize the D3D12 readback. Adds checked size_t accumulation, safer readback validation, and regression coverage for overflow cases.

Motivation and Context

DML batched readback previously accumulated output tensor byte sizes in uint32_t, so multiple individually valid outputs could wrap the aggregate size used to allocate the D3D12 readback heap. This change tightens the sizing path and keeps the readback allocation consistent with the per-tensor copy sizes.

@adrastogi
adrastogi requested a review from Copilot May 26, 2026 21:17

Copilot AI 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.

Copilot encountered an error: Your billing is not configured or you have Copilot licenses from multiple standalone organizations or enterprises. To use premium requests, select a billing entity via the GitHub site, under Settings > Copilot > Features.

Copilot AI 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.

Copilot's findings

  • Files reviewed: 4/4 changed files
  • Comments generated: 5

Comment thread onnxruntime/core/providers/dml/DmlExecutionProvider/src/ReadbackHeap.cpp Outdated
Comment thread onnxruntime/test/providers/dml_readback_heap_test.cc Outdated
Comment thread onnxruntime/test/providers/dml_readback_heap_test.cc
Comment thread onnxruntime/test/providers/dml_readback_heap_test.cc

Copilot AI 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.

Copilot's findings

  • Files reviewed: 4/4 changed files
  • Comments generated: 1

@adrastogi
adrastogi marked this pull request as ready for review May 27, 2026 15:55
@adrastogi
adrastogi requested a review from fdwr June 1, 2026 05:33
fdwr
fdwr previously approved these changes Jun 4, 2026

@fdwr 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.

👍 LGTM sir.

Comment thread onnxruntime/core/providers/dml/DmlExecutionProvider/src/ExecutionProvider.cpp Outdated
Comment thread onnxruntime/core/providers/dml/DmlExecutionProvider/src/ExecutionProvider.cpp Outdated
Wrap HResultToStatus return per intrafile convention (one parameter per line) and indent the ReadbackFromGpu call inside the ORT_TRY block.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

@fdwr 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.

TY

@adrastogi
adrastogi merged commit 90c095d into main Jun 6, 2026
84 of 86 checks passed
@adrastogi
adrastogi deleted the adrastogi/dml-overflow branch June 6, 2026 02:39
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