-
Notifications
You must be signed in to change notification settings - Fork 17
Add all-in-one pass pipeline #75
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
WIP: Depending on #70 and support of openmp in gcc. |
populateVectorPasses(pm); | ||
// back-end, arith/math/vector/memref dialects | ||
populateBufferizationPasses(pm); | ||
// REMOVE this pass after the TensorPasses are added. Currently we add this |
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.
Can we add "TODO" to highlight the remaining items to better distinguish with other common comments?
int main(int argc, char **argv) { | ||
// keeps GCCPURuntime linked | ||
gc_runtime_keep_alive = 0; |
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.
Do we still need this?
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 we do, to make sure gc_runtime is linked.
@@ -0,0 +1,23 @@ | |||
// RUN: gc-opt %s --gc-cpu-pipeline | gc-cpu-runner -e main -entry-point-result=void | FileCheck %s |
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.
We might need to add some complex example to cover the default 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.
@LongshengDu Please help to add some test cases and check if the pipeline works for the current linalgx and onednngraph dialect. Maybe in another PR, or in this one?
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.
Looks good, could you please clean up all the commented out code before merge?
@AndreyPavlenko, please take a look |
OK, changed the comments in pipeline to "todo"s.
I have changed most of them to "todo"s. They mark the position where not-yet-implemented MLIR passes should be. The developers of that pass can know where to add to the pipeline in the future. BTW, this PR depends on #79. It contains code of that PR. |
No description provided.