[xegpu] Generalize fused attention schedule for KernelBench - #266
Conversation
|
FYI @charithaintc |
1a63c1b to
dc69604
Compare
There was a problem hiding this comment.
Pull request overview
Generalizes the XeGPU fused-attention lowering pipeline to support KernelBench’s level1-97 ScaledDotProductAttention-style kernels, including support for both true 4D attention shapes and payloads that collapse the leading batch dimensions.
Changes:
- Refactors common lowering utilities (
get_payload_func, vectorization/bufferization/GPU launch conversion helpers) and updates existing schedules to use them. - Generalizes fused attention scheduling and the
replace_with_fused_attentiontransform to handle additional shapes and mixed/bf16-related type flows. - Extends KernelBench example driver to recognize attention-like kernels and route them through the fused-attention schedule.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| lighthouse/utils/mlir.py | Extends layer metadata extraction for batch_matmul to include element-type info. |
| lighthouse/schedule/xegpu/reduction_schedule.py | Updates schedule to use get_payload_func and updated helper signatures. |
| lighthouse/schedule/xegpu/mlp_schedule.py | Updates schedule to use get_payload_func. |
| lighthouse/schedule/xegpu/lowering_common.py | Introduces get_payload_func and refactors helper APIs; adds vector cleanup patterns and LICM pass in vector→xegpu. |
| lighthouse/schedule/xegpu/fused_attention_schedule.py | Reworks fused attention schedule to support more shapes and reuse common lowering helpers. |
| lighthouse/schedule/xegpu/elemwise_schedule.py | Updates schedule to use get_payload_func. |
| lighthouse/ingress/mlir_gen/gpu_attention_payload.py | Unifies attention dimension naming and updates payload generation for collapsed-batch handling. |
| lighthouse/dialects/transform/transform_ext/ops/replace_with_fused_attention.py | Generalizes fused-attention replacement for batch dims and mixed/bf16-related typing behavior. |
| examples/xegpu/nanoGPT_schedule.py | Updates callsite to updated convert_to_gpu_launch signature. |
| examples/xegpu/kernel_bench.py | Adds attention schedule path, adds CPU reference option, and adjusts verification tolerances. |
| examples/xegpu/fused_attention.py | Unifies naming, extends CLI params, and updates schedule parameter plumbing. |
Suppressed comments (1)
lighthouse/dialects/transform/transform_ext/ops/replace_with_fused_attention.py:516
- Call site uses the misspelled helper name
normalize_ouput_by_sum; rename it to match the corrected helper name.
output_normalized = normalize_ouput_by_sum(
pv_out, l_i_out, batch_shape, wg_rows, d_head, compute_type
)
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
dc69604 to
7a92481
Compare
charithaintc
left a comment
There was a problem hiding this comment.
Looks great. some nit comments.
I will continue the review tomorrow.
| compute_type, | ||
| ) | ||
| # Truncate Q@K^T (f32 accumulator) to the softmax type before scaling. | ||
| if reduction_type != compute_type: |
There was a problem hiding this comment.
line 364 sets the reduction_type = compute_type. this this branch is never taken?
There was a problem hiding this comment.
You are right, it's actually dead code. I'm inclined to keep the code as it is, however, because one can change the reduction data type to a narrower one just by changing the assignment reduction_type = compute_type. The generated IR is still valid. Added a comment.
| # Rescale running P@V accumulator by alpha; the accumulator is | ||
| # kept in f32, so widen the softmax-type alpha to match. | ||
| alpha_wide = alpha | ||
| if reduction_type != compute_type: |
| with ir.InsertionPoint.after(loop): | ||
| # The sum accumulator is in the softmax type; widen it to the f32 | ||
| # accumulator type before dividing the f32 P@V result. | ||
| if reduction_type != compute_type: |
| - Q @ K^T: 2 * n_ctx^2 * n_head FLOPs | ||
| - Attention @ V: 2 * n_ctx^2 * n_head FLOPs | ||
| The softmax is left out: it is O(n_ctx^2) (~2% of the above at n_head = 64) and | ||
| - Q @ K^T: 2 * n_ctx^2 * d_head FLOPs |
There was a problem hiding this comment.
nit: n_head -> d_head change also need to be reflected on nanoGPT case
There was a problem hiding this comment.
Updated all nanoGPT files to use the same variable naming convention
|
|
||
| # Check values match reference | ||
| values_ok = np.allclose(output, output_ref, rtol=1e-3, atol=1e-3) | ||
| values_ok = np.allclose(output, output_ref, rtol=1e-3, atol=5e-3) |
There was a problem hiding this comment.
what is the reason for atol change?
There was a problem hiding this comment.
After updating llvm version the tests no longer passed, even with LH main. After scratching my head for a while I realized it's just a tolerance issue (test failed even with older llvm version if inputs were filled differently). So I think the tolerance was too tight. This kernel has two matmuls and a reduction so errors accumulate.
| # loads, but are distributed over the subgroups. | ||
| prefetch_sg_data = layer_params["prefetch_tile"] | ||
| prefetch_sg_layout = [ | ||
| reduction_tile // prefetch_sg_data[0], |
There was a problem hiding this comment.
init: maybe validate against num_sgs?
There was a problem hiding this comment.
hmm, in principle you can use different number of sgs for the computation and prefetches? Might not be optimal config but who knows. The hardware limits should not be exceeded but those are not validated in the schedule.
| op_attrs=op_attrs, | ||
| ) | ||
| # Return the first function | ||
| func = transform_ext.extract_handle(func_ops, 0) |
There was a problem hiding this comment.
this would fail silently if none matched. what is the reason for removing match_and_split?
There was a problem hiding this comment.
extract_handle raises an IndexError if the index is out of bounds so it's a definitive failure at lowering time.
also: - C -> n_embd - hidden -> n_ffn - vocab -> n_vocab - clean up unused variables
| result_ref = torch_model(*torch_inputs).to("cpu") | ||
| else: | ||
| # execute torch model on the device | ||
| torch_inputs = [inp.to("xpu") for inp in torch_inputs] |
There was a problem hiding this comment.
is the recent results discrepancy only for GPU?
There was a problem hiding this comment.
yes, this script only targets xegpu
There was a problem hiding this comment.
hmm, I mean I usually cross check xegpu output with CPU output. it seems here you use pytoch with xpu support to cross validate?
There was a problem hiding this comment.
Ah yes, in this script we indeed run the reference solution on the GPU with torch xpu backend. The new attention kernel is an exception though because torch xpu doesn't currently support torch's scaled_dot_product_attention op. That's why I added the --compute-reference-on-cpu flag to enable testing against torch CPU backend.
Generalizes
fused_attention_scheduleso that it works for KernelBench level1-97 ScaledDotProductAttention kernel.The schedule can consume 4D attention kernels
[batch_size, n_head, n_ctx, d_head]as well as versions where the 2 leading dimensions have been collapsed to a single batch dimensionbatch_size, n_headwithin the payload function (e.g., in theexamples/xegpu/fused_attention.py).At the moment, the KernelBench problem shape must be simplified from the original
[32, 32, 512, 1024]to[32, 32, 512, 64]because we do not yet have a good strategy for distributing the last dimension (head dimension).fused_attention_scheduleuses common xegpu helper utils where possible.lowering_commonandinspect_kernelmethods.replace_with_fused_attentiontransform ext op is generalized to handle bf16/mixed data types and 2 leading dimensions.[batch_size, n_head, n_ctx, d_head]consistently everywhere.examples/xegpu/kernel_bench.pysupports level1-97 attention kernel. Currently the kernel file must be manually patched to setembedding_dimension = 64.Assisted-by: Claude