Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[BUG] ToBasicBlockNormalForm immutability #8778

Merged
merged 3 commits into from
Aug 25, 2021
Merged

[BUG] ToBasicBlockNormalForm immutability #8778

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
058760b
ToBasicBlockNormalForm immutability
ganler Aug 18, 2021
cdaf602
better comment on ToBasicBlock
ganler Aug 19, 2021
68a7fe1
refine comment of ToBasicBlockForm
ganler Aug 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions src/relay/transforms/to_basic_block_normal_form.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,11 @@ Expr ToBasicBlockNormalFormAux(const Expr& e) {
IRModule ToBasicBlockNormalForm(const IRModule& mod) {
DLOG(INFO) << "ToBBlock:" << std::endl << mod;

// Efficient copy.
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 make a more direct comment here? Maybe the "Efficient copy." won't help other developers more because the code is just a copying. Can we change it to something like "Efficient copy for avoiding change the original IRModule."?

Copy link
Member

Choose a reason for hiding this comment

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

Alternatively we could say "shallow copy"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what about now?

Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need such explanation here actually lol. Let's just keep it brief and say it is a shallow copy :-)

auto mod_ = IRModule(mod->functions, mod->type_definitions, mod->Imports(), mod->source_map);

tvm::Map<GlobalVar, Function> updates;
auto funcs = mod->functions;
auto funcs = mod_->functions;
for (const auto& it : funcs) {
ICHECK_EQ(FreeVars(it.second).size(), 0) << "Expected no free variables";
if (const auto* n = it.second.as<FunctionNode>()) {
Expand All @@ -63,12 +66,12 @@ IRModule ToBasicBlockNormalForm(const IRModule& mod) {
}

for (auto pair : updates) {
mod->Add(pair.first, pair.second, true);
mod_->Add(pair.first, pair.second, true);
}

DLOG(INFO) << "ToBBlock: transformed" << std::endl << mod;
DLOG(INFO) << "ToBBlock: transformed" << std::endl << mod_;

return mod;
return mod_;
}

bool BasicBlockNormalFormCheck(const Expr& e) {
Expand Down
24 changes: 23 additions & 1 deletion tests/python/relay/test_pass_to_basic_block_normal_form.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
from tvm.relay.analysis import detect_feature
from tvm.relay import op, create_executor, transform
from tvm.relay.prelude import Prelude
from tvm.relay.testing import count
from tvm.relay.testing import count, create_workload
from tvm.relay.analysis import Feature
from tvm.relay.analysis import check_basic_block_normal_form

Expand Down Expand Up @@ -489,5 +489,27 @@ def test_higher_order_nested():
check_basic_block_normal_form(bblock)


def test_immutability():
simple_net = relay.nn.conv2d(
data=relay.var("data", relay.TensorType((1, 3, 224, 224), "float32")),
weight=relay.var("weight"),
kernel_size=(5, 5),
channels=3,
padding=(1, 1),
)
simple_net = relay.Function(relay.analysis.free_vars(simple_net), simple_net)
mod, _ = create_workload(simple_net)

old_mod = mod

with tvm.transform.PassContext(opt_level=4):
with tvm.target.Target("llvm"):
seq = tvm.transform.Sequential(passes=[transform.ToBasicBlockNormalForm()], opt_level=4)
new_mod = seq(mod)

assert old_mod.astext() == mod.astext()
assert old_mod.astext() != new_mod.astext()


if __name__ == "__main__":
pytest.main([__file__])