Skip to content

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

Merged
merged 28 commits into from
May 28, 2024
Merged

Add all-in-one pass pipeline #75

merged 28 commits into from
May 28, 2024

Conversation

Menooker
Copy link

No description provided.

@Menooker Menooker added the WIP work in progress label May 15, 2024
@Menooker
Copy link
Author

WIP: Depending on #70 and support of openmp in gcc.

@Menooker Menooker removed the WIP work in progress label May 23, 2024
@Menooker Menooker changed the title [WIP] Add all-in-one pass pipeline Add all-in-one pass pipeline May 23, 2024
populateVectorPasses(pm);
// back-end, arith/math/vector/memref dialects
populateBufferizationPasses(pm);
// REMOVE this pass after the TensorPasses are added. Currently we add this
Copy link
Contributor

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;
Copy link
Contributor

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?

Copy link
Author

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
Copy link
Contributor

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.

Copy link
Author

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?

Copy link
Contributor

@kurapov-peter kurapov-peter left a 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?

@kurapov-peter
Copy link
Contributor

@AndreyPavlenko, please take a look

@Menooker
Copy link
Author

Menooker commented May 28, 2024

Can we add "TODO" to highlight the remaining items to better distinguish with other common comments?

OK, changed the comments in pipeline to "todo"s.

Looks good, could you please clean up all the commented out code before merge?

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.

@kurapov-peter kurapov-peter merged commit cb24090 into main May 28, 2024
4 checks passed
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.

3 participants