Skip to content

[PASS] Refactor a couple of TIR passes - BindTarget, AnnotateEntryFunc, Filter, LowerInitBlock#11628

Merged
junrushao merged 4 commits into
apache:mainfrom
sunggg:fix_pass_inconsistency
Jun 9, 2022
Merged

[PASS] Refactor a couple of TIR passes - BindTarget, AnnotateEntryFunc, Filter, LowerInitBlock#11628
junrushao merged 4 commits into
apache:mainfrom
sunggg:fix_pass_inconsistency

Conversation

@sunggg

@sunggg sunggg commented Jun 8, 2022

Copy link
Copy Markdown
Contributor

This PR fixes a few inconsistent pass registration and add testcases for them.

  • LowerInitBlock had mismatch between its pass name and ffi key.
  • BindTarget, AnnotateEntryFunc, Filter were not following the name convention of tir passes and they were not registered in FFI registry.

cc. @junrushao1994

Comment thread include/tvm/tir/transform.h Outdated
#ifndef TVM_TIR_TRANSFORM_H_
#define TVM_TIR_TRANSFORM_H_

#include <tvm/driver/driver_api.h>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is it possible that we don't depend on driver_api.h?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for pointing out! Changed to #include <tvm/target/target.h> which is more accurate.

Comment thread src/tir/transforms/helpers.cc Outdated
*/

/*!
* \file helpers.cc

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

we might want to find better names other than helpers.cc. For example, driver_api_pass.cc

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agreed. How about the primfunc_utils.cc?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

that sounds a reasonable choice!

@junrushao junrushao left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Overall the PR looks in a good state! Please address my nitpicks and let's get it merged sooooon!

@junrushao junrushao merged commit 87502dd into apache:main Jun 9, 2022
Kathryn-cat pushed a commit to Kathryn-cat/tvm that referenced this pull request Jun 10, 2022
…c, Filter, LowerInitBlock (apache#11628)

This PR fixes a few inconsistent pass registration and add testcases for them. 
- `LowerInitBlock` had mismatch between its pass name and ffi key.
- `BindTarget`, `AnnotateEntryFunc`, `Filter` were not following the name convention of tir passes and they were not registered in FFI registry.
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