Sync with Microsoft ONNX Runtime - 11072026#1196
Open
ai-fw-intg wants to merge 3 commits into
Open
Conversation
…on (microsoft#29631) ### Summary Fixes wrong `AveragePool` output on the **CUDA** EP whenever cuDNN's pooling descriptor cannot represent the requested pooling — i.e. **asymmetric padding** or **dilation > 1**. This is the CUDA-EP counterpart of the CPU fix in microsoft#29629 and, together with it, the JSEP shape fix in microsoft#29627, closes out the CUDA leg of pytorch/pytorch#183528. ### Root cause `CudnnPoolingDescriptor::Set` copies only the **begin** pads (`pads[0..rank)`) into the cuDNN pooling descriptor, which stores a *single symmetric pad value per axis* and applies it to both sides. The ONNX **end** pads (`pads[rank..2*rank)`) are silently dropped. As a result, **all** asymmetric-pad AveragePool on CUDA is wrong: - explicit asymmetric `pads`, - `auto_pad = SAME_UPPER` / `SAME_LOWER` (which produce naturally asymmetric pads), - `ceil_mode = 1` boundary windows. Separately, the cuDNN pooling descriptor has **no dilation parameter at all**, so any `dilations > 1` is also silently ignored — even with symmetric pads. Example divergence from the CPU reference (QA probe): 1D `pad(0,3)`, k7, s3, `ceil_mode=1`, `count_include_pad=1` gave CUDA `[4, 6.5, 8]` vs. correct `[4, 5.571, 4]`; 2D `pad(0,0,3,3)` gave `71.0` vs. correct `17.75`. ### Fix Add a custom CUDA average-pool kernel (`avg_pool_impl.cu` / `.h`, modeled on `max_pool_with_index.cu`) that honors **per-side pads** and **dilation**, and computes the `count_include_pad` divisor exactly like the CPU v19 reference functor (`AveragePool{1,2,3}DTask`): - include-pad divisor clamps the window end to `input + pad_tail` (drops the ceil-mode phantom cells), dividing by `∏ (1 + (end - start - 1)/dilation)`; - exclude-pad divisor counts only in-bounds cells. In `Pool<T, AveragePool, Layout>::ComputeInternal`, a cheap dispatch guard routes to the custom kernel **only** when the pooling is non-global **and** (`asymmetric pads` **or** `!default_dilations`). Every symmetric, non-dilated case — the overwhelmingly common path, including **all** `GlobalAveragePool` — stays on the existing cuDNN fast path, so there is **zero perf regression** on the common path. `GlobalAveragePool` is excluded explicitly because `PoolAttributes` leaves its `kernel_shape`/`strides`/`dilations` unpopulated. `MaxPool` is unaffected (it ignores pad cells; `MaxPool<8>` already routes dilation to its own custom kernel via the same `!default_dilations` check we mirror here). Covers fp32, fp64, fp16, and bf16 (fp16/bf16 accumulate in float). ### Tests Added CUDA-**un-excluded** parity tests in `pool_op_test.cc` — the CUDA leg actually runs and must match the CPU reference oracle: - asymmetric tail-pad 1D/2D, include- and exclude-pad; - `auto_pad=SAME_UPPER` and `SAME_LOWER` (naturally asymmetric); - **symmetric-pad + dilation>1** (wrong on cuDNN, correct via the kernel — locks in the dilation guard); - a symmetric-pad regression case (must stay on cuDNN and remain correct); - fp16; - a `MaxPool` asymmetric-pad case confirming MaxPool is unaffected. The `ceil_mode + count_include_pad` cases use opset 19 so the CPU leg runs the already-correct v19 reference functor and validates the CUDA kernel independently of the separate opset-7..18 CPU MLAS fix (microsoft#29629); CUDA routing is opset-independent. Full `PoolTest` suite passes on an A100 (53 passed / 2 DML-skipped / 0 failures). ### Related - CPU (a): microsoft#29629 — opset 7-18 MLAS `ceil_mode + count_include_pad` divisor fix. - JSEP (c): microsoft#29627 — JSEP pooling output shape honoring `ceil_mode`. - pytorch/pytorch#183528. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…oft#29157) ### Summary OrtApi::GetValue on a sequence of tensors copied elements using element_type->Size() * element_count bytes. For packed sub-byte types (INT4/UINT4, two elements per byte) this is ~2× the real storage size, causing a heap over-read of the source and overflow of the destination. ### Fix In PopulateTensorWithData (onnxruntime/core/session/onnxruntime_c_api.cc), copy the tensor's actual packed size via Tensor::SizeInBytes() instead of element_type->Size() * num_elems, and drop the now-unused elem_size parameter. SizeInBytes() is packing-aware: identical for ≥1-byte types (no behavior change) and correct for sub-byte types. ### Testing - Added CApiTest.CreateGetSeqSubByteTensors: sequences of INT4/UINT4 tensors with an odd length (7 elements → 4 packed bytes), verifying GetValue() round-trips correctly. - Full onnxruntime_shared_lib_test suite passes (185/185); other paths unchanged. - Confirmed under AddressSanitizer: the old formula triggers a heap-buffer-overflow, the new one is clean.
ai-fw-intg
requested review from
Jaswanth51,
ankitm3k,
jatinwadhwa921 and
vthaniel
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automated daily backmerge from ORT main to ovep-develop. No conflicts detected. Do NOT squash or rebase - use merge commit only.