Skip to content

Conversation

@sjw36
Copy link
Contributor

@sjw36 sjw36 commented Apr 9, 2025

  • Converted ad-hoc attribute handling to AttrHelpers to ensure consistency
  • Changed tt_latency to tt.latency in AssignLatencies?

sjw36 added 2 commits April 9, 2025 10:19
* Converted ad-hoc attribute handling to AttrHelpers to ensure consistency
* Changed `tt_latency` to `tt.latency` in AssignLatencies
@sjw36 sjw36 requested a review from ptillet as a code owner April 9, 2025 15:56
@sjw36
Copy link
Contributor Author

sjw36 commented Apr 9, 2025

@pawelszczerbuk is this change correct? I couldn't see where tt_latency was set on any ops.

Copy link
Collaborator

@Mogball Mogball left a comment

Choose a reason for hiding this comment

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

This seems like a good thing. Can we delete kLatencyAttrName now?

}

DenseMap<Operation *, int> mlir::triton::deserializeLatencies(Operation *op) {
auto helper = op->getContext()
Copy link
Collaborator

Choose a reason for hiding this comment

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

suggestion: can you add a shorthand for this in Triton/Dialect.h?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes! added.

Copy link
Contributor Author

@sjw36 sjw36 Apr 15, 2025

Choose a reason for hiding this comment

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

posted these upstream here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

well.. I guess Medhi and River are not big fans..

Copy link
Collaborator

Choose a reason for hiding this comment

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

They tend to be picky :P

@sjw36
Copy link
Contributor Author

sjw36 commented Apr 15, 2025

This seems like a good thing. Can we delete kLatencyAttrName now?

Not sure as there is still some ambiguity with tt.latency and tt_latency.. ping @pawelszczerbuk

@Jokeren Jokeren merged commit 3a91f81 into triton-lang:main Apr 16, 2025
7 of 8 checks passed
njriasan pushed a commit to njriasan/triton that referenced this pull request Apr 18, 2025
* Converted ad-hoc attribute handling to AttrHelpers to ensure
consistency
* Changed `tt_latency` to `tt.latency` in AssignLatencies?
FindHao pushed a commit to FindHao/triton that referenced this pull request Apr 30, 2025
* Converted ad-hoc attribute handling to AttrHelpers to ensure
consistency
* Changed `tt_latency` to `tt.latency` in AssignLatencies?
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.

3 participants