Skip to content

Conversation

azazhu
Copy link
Collaborator

@azazhu azazhu commented Nov 15, 2022

add print debug info about expr transforms on GpuLower. So, we can review the expr strings and check if a pass's behavior meets our expectation.
use PYTORCH_NVFUSER_DUMP="pass_transform" to enable the debug print.

Copy link
Collaborator

@naoyam naoyam left a comment

Choose a reason for hiding this comment

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

Mostly looks good, just some minor cleanup suggestions

@@ -109,6 +110,8 @@ auto parseDebugDumpOptions() {
options_map[DebugDumpOption::BankConflictInfo] = true;
} else if (token == "sync_map") {
options_map[DebugDumpOption::SyncMap] = true;
} else if (token == "pass_transform") {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We may want to use some more specific keyword than "pass_transfrom". Perhaps, "lower_verbose"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done, renamed lower_verbose

Comment on lines 229 to 234
if (isDebugDumpEnabled(DebugDumpOption::PassTransform)) {
std::cout << "Before validateIr:" << std::endl;
for (auto exp : fusion_->exprs()) {
std::cout << exp->toString() << std::endl;
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's create a function to do this so that we don't need to have this code repeatedly.

void dumpExprs(const std::vector<Expr*>& exprs, std::string msg_pre) {
 if (isDebugDumpEnabled(DebugDumpOption::PassTransform)) {
    std::cout << msg_pre << ":" << std::endl;
    for (auto exp : exprs) {
      std::cout << exp->toString() << std::endl;
    }
  }
}

Ideally, we should have an actual pass manager to do this kind of pre- and post-processing automatically, which should also automatically enforce correct dependencies between passes, but for now, I think this should be fine.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done. move to dumpExprsIfEnabled(const std::vector<Expr*>& exprs, std::string msg_pre)

Copy link
Collaborator

@naoyam naoyam left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for adding this functionality!

@naoyam
Copy link
Collaborator

naoyam commented Nov 15, 2022

You may first need to merge devel to this branch to get the lint error fixed.

@azazhu
Copy link
Collaborator Author

azazhu commented Nov 16, 2022

done

@zasdfgbnm
Copy link
Collaborator

@azazhu Linter passes. You can merge it now.

@azazhu
Copy link
Collaborator Author

azazhu commented Nov 16, 2022

@azazhu Linter passes. You can merge it now.

@zasdfgbnm , I don't have write permission. Could you add write permission to me?

@zasdfgbnm
Copy link
Collaborator

@azazhu Linter passes. You can merge it now.

@zasdfgbnm , I don't have write permission. Could you add write permission to me?

Sorry, I wasn't aware of that. ping @csarofeen for permission. I will merge this PR for you.

@zasdfgbnm zasdfgbnm merged commit 11c459e into csarofeen:devel Nov 16, 2022
@azazhu azazhu deleted the fw/dump_debug branch November 16, 2022 03:07
@csarofeen
Copy link
Owner

@azazhu permission fixed.
@azazhu can you please change this to not print to std::cout, but to output to a file (or a file per pass output), similar to DebugDumpOption::CudaToFile?

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.

4 participants