Skip to content

fix(matmul-lowering): use extractMemRefDataPtr for offset-correct pointers - #626

Draft
kirankumarmatam-amd wants to merge 2 commits into
mainfrom
fix/qwen-matmul-subview-offset
Draft

fix(matmul-lowering): use extractMemRefDataPtr for offset-correct pointers#626
kirankumarmatam-amd wants to merge 2 commits into
mainfrom
fix/qwen-matmul-subview-offset

Conversation

@kirankumarmatam-amd

@kirankumarmatam-amd kirankumarmatam-amd commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

Problem

MatmulOpLowering used extractContiguousMemRefPtr which returns only
alignedPtr from the memref descriptor, ignoring the offset field.

When onnx.Split produces Q/K/V slices via memref.subview, each slice
carries a non-zero offset into the parent buffer. extractContiguousMemRefPtr
returned the parent buffer's base address for all three, so Q and V MatMuls
silently read K's data instead of their own — reducing end-to-end model
accuracy to ~0.36 cosine on Qwen 7B VLM (gfx1151).

Fix

Switch A, B, and output to extractMemRefDataPtr, which computes
alignedPtr + GEP(offset) — the GEP advances by offset elements,
correctly accounting for element size:

// Before:
Value APtr = extractContiguousMemRefPtr(adaptor.getA(), rewriter, loc);
// After:
auto AType = cast<MemRefType>(op.getA().getType());
Value APtr = extractMemRefDataPtr(adaptor.getA(), AType,
                                  getTypeConverter(), rewriter, loc);

Testing

Verified on gfx1151 (Strix Halo, Windows D3D12 HMM): cosine similarity
for Qwen 7B VLM restored to ~1.0 after this fix.

…nters

Co-Authored-By: Claude <noreply@anthropic.com>
@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown

Thanks for opening a PR!

This project follows LLVM's incremental-development and AI-tool-use
guidance. See CONTRIBUTING.md
for the project workflow.

Before requesting review, please check that:

  1. The change is focused. Substantial work links the relevant issue
    or design discussion.
  2. The PR documents relevant test results and updates affected
    documentation.
  3. If AI tools provided substantial assistance, the description
    explains what was assisted and how it was validated, and commit
    trailers identify the tool. The contributor has reviewed and
    understands the result.

Reviewers are assigned through
CODEOWNERS where ownership
is configured.

@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown

L2 Accuracy Results (EP vs CPU)

Model Combined L2 Total Elems Skipped NaN/Inf
conv_test_hybrid 4.8668E-07 64 0
GroupQueryAttention_seq256 25.2366 2621440 0
MatMulNBits_o_seq128 259.906 368640 0
QMoE_seq128 34.957 368640 0

Threshold: 0.01 | Run: 3531 - Commit: 918c161

@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown

MorphiZen EP Performance Results

Model QPS Session (s) 1st Infer (ms) CPU% Mem (MB)
full_model_seq128 7.51 6.29 425 3 1242
GroupQueryAttention_seq128 4284.47 1.70747 11 6 311
matmul_down_seq128 523.83 2.45 75 3 352

EPContext Export Performance

Model QPS Session (s) 1st Infer (ms) CPU% Mem (MB)
full_model_seq128 7.55 44.50 364 3 15589

EPContext Import Performance

Model QPS Session (s) 1st Infer (ms) CPU% Mem (MB)
full_model_seq128 7.53 9.14 360 3 15761

OGA Benchmark Results

Model Warmup Reps Prompt Len Gen Tokens TTFT (ms) TPS Peak Mem (GB) GPU Mem (GB)
gpt-oss-20b-webgpu-int4-rtn-block-32 1 5 128 128 172.0 79.0 1.33 13.54
Llama-3.1-8B-awq-g128-int4-asym-fp16-onnx-dml 1 5 128 128 325.3 41.1 1.22 6.43

OGA Wheel Smoke (Python benchmark_e2e.py)

Model TTFT (ms) TPS
Llama-3.1-8B-awq-g128-int4-asym-fp16-onnx-dml 205 40.4

Run: 3531 - Commit: 918c161

Replace the verbose comment with a concise invariant statement, drop
the "(Bug 4 in gnpu-onnx-flow)" reference (project-internal, opaque
to hip-ep reviewers), and remove the noise annotation on the shape
variable declarations.

Co-Authored-By: Claude <noreply@anthropic.com>
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.

1 participant