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

[Region] Bufferize with one-shot-bufferize #973

Merged
merged 5 commits into from
Dec 18, 2024

Conversation

tkarna
Copy link
Contributor

@tkarna tkarna commented Dec 3, 2024

  • Remove region-bufferize pass and Region dialect conversion patterns
  • Add BufferizableOpInterface extensions to Region dialect to enable bufferization via one-shot-bufferize
  • Update region bufferize test that uses one-shot-bufferize

Please review these guidelines to help with the review process:

  • Have you provided a meaningful PR description?
  • Have you added a test, a reproducer, or a reference to an issue with a reproducer?
  • Have you tested your changes locally for CPU and GPU devices?
  • Have you made sure that new changes do not introduce compiler warnings?
  • If this PR is a work in progress, are you filing the PR as a draft?
  • Have you organized your commits logically and ensured each can be built by itself?

Comment on lines 84 to 87
if (noTensorsIn(envOp.getArgs()) && noTensorsIn(envOp.getResults())) {
// Nothing to do.
return ::mlir::success();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this needed? the docs say

         This method will never be called on ops that do not have at least one
         tensor operand/result.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, this is not needed.

bool bufferizesToMemoryRead(
::mlir::Operation *op, ::mlir::OpOperand &opOperand,
const ::mlir::bufferization::AnalysisState &state) const {
return true;
Copy link
Contributor

Choose a reason for hiding this comment

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

what is it reading?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This implementation follows scf.yield op bufferization: read is true, write is false etc.

@tkarna tkarna force-pushed the region-dialect-one-shot-upstream branch from 48fd9b8 to e4bdcbb Compare December 18, 2024 09:30
@fschlimb fschlimb merged commit 43a7d7c into intel:main Dec 18, 2024
2 checks passed
@tkarna tkarna deleted the region-dialect-one-shot-upstream branch December 18, 2024 11:38
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.

4 participants