Skip to content

[midend/lib/Conversion]TOSA:ReduceSumOP Vectorize Optimization #490

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 5 commits into
base: main
Choose a base branch
from

Conversation

Hayden727
Copy link

A clean PR commit only have reduce_sum vectorize Pass

Copy link
Member

@zhanghb97 zhanghb97 left a comment

Choose a reason for hiding this comment

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

Giving some quick code sytle comments. Still reviewing the optimization strategy.

Copy link
Member

Choose a reason for hiding this comment

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

This analysis seems unrelated to the vectorization optimization. Please separate it into a different PR.

Copy link
Author

Choose a reason for hiding this comment

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

This is only useful for analysis that counts the number of operations in different dialects.
Will re-issue a different PR

Copy link
Member

Choose a reason for hiding this comment

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

Please use FileCheck to verify correctness and use rtclock to obtain performance results. See here as an example.

Copy link
Author

Choose a reason for hiding this comment

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

This script is used to verify the correctness of using different Pass Pipelines for the same file and calculate the speedup ratio to verify the efficiency.

Copy link
Author

Choose a reason for hiding this comment

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

Whether it is an mlir file, you need to use FileCheck to verify the correctness

Copy link
Member

Choose a reason for hiding this comment

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

Please use English for comments.

%a = memref.get_global @A : memref<12x40x40xf32>
call @kernel(%a) : (memref<12x40x40xf32>) -> ()
return
}
Copy link
Member

Choose a reason for hiding this comment

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

Please add an empty line here.

Copy link
Member

Choose a reason for hiding this comment

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

Name: next-reduce-sum-vec-manual.mlir

Copy link
Member

Choose a reason for hiding this comment

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

The name '1' is unclear and lacks meaning. Please use a more descriptive and meaningful suffix.

call @kernel(%c0) : (tensor<12x40x40xf32>) -> ()

return
}
Copy link
Member

Choose a reason for hiding this comment

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

Please add an empty line here.

call @kernel(%c0) : (tensor<1x40x1536xf32>) -> ()

return
}
Copy link
Member

Choose a reason for hiding this comment

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

Please add an empty line here.

Copy link
Member

Choose a reason for hiding this comment

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

The name '1' is unclear and lacks meaning. Please use a more descriptive and meaningful suffix.

PassRegistration<ReduceSumVectorizationPass>();
}
} // namespace buddy
} // namespace mlir
Copy link
Member

Choose a reason for hiding this comment

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

Please add an empty line here.

@Hayden727 Hayden727 changed the title TOSA:ReduceSumOP Vectorize Optimization [midend/lib/Conversion]TOSA:ReduceSumOP Vectorize Optimization May 25, 2025
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