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

[BYOC-DNNL] enable conv3d->bn folding #10837

Merged
merged 5 commits into from
Apr 11, 2022
Merged

[BYOC-DNNL] enable conv3d->bn folding #10837

merged 5 commits into from
Apr 11, 2022

Conversation

crazydemo
Copy link
Contributor

This PR contains two modifications:

  1. support conv3d-bn folding.
  2. add some layout in dnnl_json_runtime, and fix some typos.

@masahi
Copy link
Member

masahi commented Mar 31, 2022

Is it possible to unify 2D and 3D impls?

Also please add test cases.

@crazydemo
Copy link
Contributor Author

Is it possible to identify conv2d or conv3d from the given callnode?

@masahi
Copy link
Member

masahi commented Apr 1, 2022

yeah, for example

tvm/src/relay/backend/utils.h

Lines 427 to 432 in 4322ebc

inline bool IsOp(const CallNode* call, const std::string& op_name) {
const auto* op_node = call->op.as<OpNode>();
ICHECK(op_node) << "Expects a single op.";
Op op = GetRef<Op>(op_node);
return op == Op::Get(op_name);
}

@crazydemo
Copy link
Contributor Author

yeah, for example

tvm/src/relay/backend/utils.h

Lines 427 to 432 in 4322ebc

inline bool IsOp(const CallNode* call, const std::string& op_name) {
const auto* op_node = call->op.as<OpNode>();
ICHECK(op_node) << "Expects a single op.";
Op op = GetRef<Op>(op_node);
return op == Op::Get(op_name);
}

Thank you so much! I will try to unify 2D and 3D impls.

@crazydemo
Copy link
Contributor Author

Hi @masahi, would you please review this PR and give me some suggestions?

masahi
masahi approved these changes Apr 11, 2022
View reviewed changes
Copy link
Member

@masahi masahi left a comment

Choose a reason for hiding this comment

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

Sorry forgot.

I have one minor comment, please address it in the next PR.

src/relay/transforms/fold_scale_axis.cc
const auto* param = ref_call->attrs.as<Conv2DAttrs>();
return ConvForwardRewrite(ref_call, param, new_args, message);
}
const auto* param = ref_call->attrs.as<Conv3DAttrs>();
Copy link
Member

Choose a reason for hiding this comment

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

ICHECK(param)

Add that to other places as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it.

@masahi masahi merged commit 45f3d4a into apache:main Apr 11, 2022
mehrdadh pushed a commit to mehrdadh/tvm that referenced this pull request Apr 11, 2022
@crazydemo @mehrdadh
[BYOC-DNNL] enable conv3d->bn folding (apache#10837)
b574fe3 
* support conv3d bn folding

* add test case for fold_scale_axis

* modify lint

* remove test cases

* unify conv2d 3d impls, and add test cases.
@crazydemo crazydemo deleted the dev-enhance_fold_scale_axis branch April 14, 2022 03:21
Lucien0 pushed a commit to Lucien0/tvm that referenced this pull request Apr 19, 2022
@crazydemo @Lucien0
[BYOC-DNNL] enable conv3d->bn folding (apache#10837)
f6c39b2 
* support conv3d bn folding

* add test case for fold_scale_axis

* modify lint

* remove test cases

* unify conv2d 3d impls, and add test cases.
altanh pushed a commit to altanh/tvm that referenced this pull request Apr 28, 2022
@crazydemo @altanh
[BYOC-DNNL] enable conv3d->bn folding (apache#10837)
6c404a0 
* support conv3d bn folding

* add test case for fold_scale_axis

* modify lint

* remove test cases

* unify conv2d 3d impls, and add test cases.
@driazati driazati mentioned this pull request Jul 14, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@masahi masahi masahi approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@crazydemo @masahi