Skip to content

[GPU] Fix various Xe3p/CRI performance edge cases - #5692

Open
Simonsays095 wants to merge 4 commits into
uxlfoundation:mainfrom
Simonsays095:CRI_kchain
Open

[GPU] Fix various Xe3p/CRI performance edge cases#5692
Simonsays095 wants to merge 4 commits into
uxlfoundation:mainfrom
Simonsays095:CRI_kchain

Conversation

@Simonsays095

@Simonsays095 Simonsays095 commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Fixes several small bugs/edge cases found while working on CRI:

  • bdpas has GRF conflicts unless A and A_scale are in the same bank. Update register allocation to enforce this.
  • systolic fwd usage was brittle and implemented strangely, changed to match spec.
  • Accumulator counting is incorrect for Xe3p+, since grf512 was not accounted for.
  • Enable k-chaining by default. Very important for performance on CRI. Backported from the gemmstone repo.

@Simonsays095
Simonsays095 requested a review from a team as a code owner July 27, 2026 18:20
@github-actions github-actions Bot added platform:gpu-intel Codeowner: @oneapi-src/onednn-gpu-intel third_party labels Jul 27, 2026
kealan-barbieri
kealan-barbieri reviewed Jul 27, 2026
View reviewed changes
Comment thread src/gpu/intel/gemm/jit/generator/pieces/matrix_multiply.cxx
}

if (hhbase + ksys < opCount && rc == 8) mod |= Fwd;
bool canFwd = (hw == ngen::HW::Xe3p)

@kealan-barbieri kealan-barbieri Jul 27, 2026

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.

do we need to be checking a maximum sequence length for fwd to comply with spec or is it already handled/not applicable?

@Simonsays095 Simonsays095 Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't think there is a max sequence length for forwarding according to spec. Am I missing something?

@kealan-barbieri

kealan-barbieri commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

This probably deserves a backport as well since its enabling critical CRI features.

rjoursler
rjoursler reviewed Jul 27, 2026
View reviewed changes
Comment thread src/gpu/intel/gemm/jit/generator/strategy.cpp Outdated
Comment thread src/gpu/intel/gemm/jit/generator/strategy.cpp
// Priority: k chaining > extra C registers > r0 header storage.
// 64-bit emulation > r0 header storage.
if (AccumulatorRegister::count(hw, GRFs, problem.Tc.real().ngen()) == 0)
if (kChain == 0) {

@rjoursler rjoursler Jul 27, 2026

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.

FYI: this definitely steps on #5681 a little bit. I will make sure to merge onto your changes.

@Simonsays095

Simonsays095 commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

make test
disable test_device_cpu
disable build_cpu_runtime_omp
disable build_cpu_runtime_sycl
disable build_cpu_runtime_tbb
disable benchdnn_all
enable benchdnn_matmul
enable benchdnn_ip
enable benchdnn_graph
enable arch_gpu_xe3p-cri
enable arch_gpu_xe3p-lpg

@Simonsays095

Simonsays095 commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

make test perf-gpu
set primitive=gpu:gemm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rjoursler rjoursler rjoursler approved these changes
@echeresh echeresh echeresh approved these changes
@kealan-barbieri kealan-barbieri kealan-barbieri approved these changes

Assignees

No one assigned

Labels

platform:gpu-intel Codeowner: @oneapi-src/onednn-gpu-intel third_party

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Simonsays095 @kealan-barbieri @echeresh @rjoursler