-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
[Refactor] Move VarUseDefAnalysis
to header file
#14185
Conversation
Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from Reviewers by @-ing them in a comment.
Generated by tvm-bot |
@Hzfengsy @Lunderberg @tqchen I have decoupled the previous
I know this class has been used for long time and we don't want any changes on its behavior, so we need to be careful in this refactor, and please correct me if I made any mistakes. One thing I'm not sure is if we need to couple the three functionalities together in a single pass, or we can decouple them like what I did in the refactor. If my refactor doesn't work, I'm totally okay if we keep the original class and create a simpler |
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.
LGTM! Just a couple of small nitpicks.
cc @Hzfengsy please take another look and manage the PR |
Motivation
UndefinedVars
is a frequently used function in our codebase and its implementation relies onVarUseDefAnalysis
class which is more general, currently we exposeUndefinedVars
inanalysis.h
, but both the definitions ofUndefinedVars
andVarUseDefAnalysis
resides insplit_host_device.cc
.This PR moves
VarUseDefAnalysis
class toanalysis.h
so that developers can use it in other files that requires use/def analysis thansplit_host_devices.cc
. We create avar_use_def_analysis.cc
undersrc/src/analysis
for the implementations of bothUndefinedVars
andVarUseDefAnalysis
.Notes
We rename
VarUseDefAnalysis
toVarUseDefAnalyzer
.cc @Hzfengsy @Lunderberg