-
Notifications
You must be signed in to change notification settings - Fork 16
Align the bufferization pipeline according to the upstream doc #295
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
Conversation
| pm.addPass(bufferization::createOneShotBufferizePass(options)); | ||
| pm.addPass(createCanonicalizerPass()); | ||
| pm.addPass(createCSEPass()); | ||
| pm.addPass(createCanonicalizerPass()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
L101 is already CanonicalizerPass, do we really need to do it again after CSE?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the first CanonicalizerPass is mainly used to canonicalize the scf.for and scf.forall to the canonical form in memref, exposing more CSE optimization opportunities. The CSE pass is used to eliminate the common memref.subview so there will be some useless memref.copy memref.copy %a, %a. The last CanonicalizerPass is used to eliminate the useless memref.copy which copies from A to A like memref.copy %a, %a
lib/gc/Transforms/Pipeline.cpp
Outdated
| pm.addPass(bufferization::createDropEquivalentBufferResultsPass()); | ||
| pm.addNestedPass<func::FuncOp>( | ||
| bufferization::createPromoteBuffersToStackPass()); | ||
| mlir::bufferization::BufferDeallocationPipelineOptions deallocOption; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: no need to add mlir namespace here.
BTW, could you also put a note/link to the upstream bufferization pipeline?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the advice! the mlir namespace has been removed and the note has been added
Fix part of issue #288
empty_tensor_eliminationcould eliminate the copy introduced bybufferization.materialize_in_destination. And adjust the pipeline according to the doc(https://mlir.llvm.org/docs/Bufferization/#overview).