Skip to content
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

[BACKEND] Extend hoisting of convert op above ext ops #2206

Merged
merged 1 commit into from
Aug 30, 2023

Conversation

ThomasRaoux
Copy link
Collaborator

Handle more cases of hoisting convert above ext op. If there are multiple ext op in the slice but only one requires inserting a convert we can still apply the optimization.

Handle more cases of hoisting convert above ext op.
If there are multiple ext op in the slice but only one
requires inserting a convert we can still apply the optimization.
// If we can rematerialize the rest of the ext slice we can ignore this
// ext as it won't need a convert.
if (result.succeeded()) {
slice.insert(tempSlice.begin(), tempSlice.end());
Copy link
Contributor

Choose a reason for hiding this comment

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

Once the number of slice is changed, do you need to increase sliceSize as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

no, we only want to go through the original slice since here we get a backward slice without stopping at ext anymore. This new slice will be rematerialized without the need for convert so we don't have to consider it in looking for an ext that requires a convert.

Copy link
Contributor

Choose a reason for hiding this comment

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

Got you. That makes sense

// If we can rematerialize the rest of the ext slice we can ignore this
// ext as it won't need a convert.
if (result.succeeded()) {
slice.insert(tempSlice.begin(), tempSlice.end());
Copy link
Contributor

Choose a reason for hiding this comment

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

Got you. That makes sense

@ThomasRaoux ThomasRaoux merged commit 2ff88c1 into main Aug 30, 2023
5 checks passed
@ThomasRaoux ThomasRaoux deleted the hoisting_convert branch August 30, 2023 00:36
pingzhuu pushed a commit to siliconflow/triton that referenced this pull request Apr 2, 2024
Handle more cases of hoisting convert above ext op. If there are
multiple ext op in the slice but only one requires inserting a convert
we can still apply the optimization.
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.

2 participants