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.

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

[BYOC] Refine DNNL Codegen #5288

Merged
merged 3 commits into from
Apr 10, 2020
Merged

[BYOC] Refine DNNL Codegen #5288

merged 3 commits into from
Apr 10, 2020

Conversation

comaniac
Copy link
Contributor

@comaniac comaniac commented Apr 9, 2020

With multiple output and merge compiler region support, we can now refine the DNNL codegen to support ops with multiple outputs such as batch_norm. This PR:

  • Support operators with multiple outputs.
  • Fix the issue that the second fuse run in VM will go into external functions and decompose batch_norm.
  • Turn on DNNL batch_norm.
  • Fix TupleGeItem in DNNL codegen.
  • Skip MobileNet test because of the constant node issue for now.

cc @zhiics, @masahi.

@masahi masahi self-assigned this Apr 9, 2020
CHECK(out_.size() > static_cast<size_t>(op->index));

// Only keep the item we want for the child node.
// FIXME(@comaniac): The other items should still be requried for the primary outputs.
Copy link
Member

Choose a reason for hiding this comment

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

This looks ok to me, why "FIXME"? TupleGetItem only cares about one output.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In terms of the semantic, if we generate a partition function like conv2d->BN->relu, we should have a tuple output with 3 elements (relu_out, BN_out_1, BN_out_2) even the rest 2 are useless. In general we should achieve this semantic, but since no one cares the rest 2 outputs of batch_norm it's fine for now.

@masahi masahi merged commit f0f0364 into apache:master Apr 10, 2020
@masahi
Copy link
Member

masahi commented Apr 10, 2020

Thanks @comaniac @zhiics

@comaniac comaniac deleted the fix_dnnl branch April 10, 2020 16:39
trevor-m pushed a commit to trevor-m/tvm that referenced this pull request Apr 16, 2020
* Improve DNNL

* Add bind params

* trigger ci
zhiics pushed a commit to neo-ai/tvm that referenced this pull request Apr 17, 2020
* Improve DNNL

* Add bind params

* trigger ci
dpankratz pushed a commit to dpankratz/incubator-tvm that referenced this pull request Apr 24, 2020
* Improve DNNL

* Add bind params

* trigger ci
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