Skip to content

Clean up Intel specific code in the common TritonGPU dialect source file. #4469

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

chengjunlu
Copy link
Contributor

Add a new interface in the DialectVerifyTensorLayoutInterface. Allow the third party dialect to verify the dot op layout as extension.

Run the CI test to make sure there is no functional issue.

The changes in common TritonGPU dialect need to be pushed to upstream first.

@chengjunlu chengjunlu requested a review from Copilot June 10, 2025 02:50
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR cleans up Intel-specific dot-op layout logic in the common TritonGPU dialect by introducing a new dialect interface method and dispatching layout verification to each dialect’s implementation.

  • Define verifyDotOpLayout in the common DialectVerifyTensorLayoutInterface and implement it in both the Intel and core TritonGPU dialects.
  • Remove hard-coded Intel DPAS checks from DotOperandEncodingAttr::verify and replace with trait-based dispatch.
  • Register the new TritonIntelGPUVerifyTensorLayoutInterface in the Intel-specific dialect.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
third_party/intel/lib/Dialect/TritonIntelGPU/IR/Dialect.cpp Added TritonIntelGPUVerifyTensorLayoutInterface with verifyTensorLayout and verifyDotOpLayout, and registered it
lib/Dialect/TritonGPU/IR/Dialect.cpp Replaced manual Intel DPAS layout checks in DotOperandEncodingAttr::verify with trait-based dispatch and added verifyDotOpLayout
include/triton/Dialect/Triton/IR/Dialect.h Extended DialectVerifyTensorLayoutInterface by adding the pure-virtual verifyDotOpLayout
Comments suppressed due to low confidence (4)

third_party/intel/lib/Dialect/TritonIntelGPU/IR/Dialect.cpp:1167

  • The error message uses "un-known" which is nonstandard; please change it to "unknown" for consistency.
return emitError() << "ttg.dot_op un-known parent layout of TritonIntelGPU dialect: " << parent;

lib/Dialect/TritonGPU/IR/Dialect.cpp:2761

  • Similarly, this error message should read "unknown" instead of "un-known".
return emitError() << "ttg.dot_op un-known parent layout of TritonGPU dialect: " << parent;

include/triton/Dialect/Triton/IR/Dialect.h:105

  • [nitpick] Consider providing a default implementation or documenting the expected behavior of verifyDotOpLayout to avoid breaking existing dialects that implement this interface.
virtual LogicalResult
verifyDotOpLayout(Attribute parent, unsigned opIdx, unsigned kWidth,
function_ref<InFlightDiagnostic()> emitError) const = 0;

lib/Dialect/TritonGPU/IR/Dialect.cpp:687

  • Add unit tests covering the new dispatch path through isa<MmaEncodingTrait> to ensure both core and third-party dialects invoke the correct verifyDotOpLayout implementations.
if (isa<MmaEncodingTrait>(parent)) {

…w the third party dialect to verify the dot op layout as extension.

Signed-off-by: Lu,Chengjun <[email protected]>
Comment on lines +1182 to +1183
// addInterfaces<TritonGPUOpAsmInterface>();
// addInterfaces<TritonGPUInferLayoutInterface>();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// addInterfaces<TritonGPUOpAsmInterface>();
// addInterfaces<TritonGPUInferLayoutInterface>();

@etiotto
Copy link
Contributor

etiotto commented Jun 20, 2025

@chengjunlu the upstream part, is it done ? Can you rebase this PR if it is done please?

@chengjunlu
Copy link
Contributor Author

chengjunlu commented Jun 22, 2025

@chengjunlu the upstream part, is it done ? Can you rebase this PR if it is done please?

Here is the upstream PR: triton-lang/triton#7147
I think it is not get noticed by upstream Triton maintainer.
I would like to ping the upstream reviewer to review it out.

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

Successfully merging this pull request may close these issues.

3 participants