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

What should do to add MKLDNN kernel #8305

Closed
jacquesqiao opened this issue Feb 9, 2018 · 5 comments
Closed

What should do to add MKLDNN kernel #8305

jacquesqiao opened this issue Feb 9, 2018 · 5 comments
Labels

Comments

@jacquesqiao
Copy link
Member

jacquesqiao commented Feb 9, 2018

Base class that need to implement:

  • MKLDNNLoDTensor derived from LoDTensor. in formal PaddlePaddle.
    Like MKLDNNMatrix.h for v2

  • MKLDNNKernelBase derived from KernelBase. Like MKLDNNLayer.h in formal PaddlePaddle for v2

  • Tensor transform functions used to tranform between LodTensor and MKLDNNKernel

Steps to add new kernel

Take pool_cudnn_op as example:

  1. add pool_cudnn_op.cu.cc
  2. implement PoolCUDNNOpKernel
  3. register kernel with library type CUDNN
REGISTER_OP_KERNEL(pool2d, CUDNN, ::paddle::platform::CUDAPlace,
                   ops::PoolCUDNNOpKernel<float>,
                   ops::PoolCUDNNOpKernel<double>);

If we want to add pool_mkldnn kernel, the step should be:

  1. add new file pool_mkldnn_op.cc
  2. implement PoolMKLDNNOpKernel
  3. register PoolMKLDNNOpKernel with library MKLDNN
REGISTER_OP_KERNEL(pool2d, MKLDNN, ::paddle::platform::MKLDNNPlace,
                   ops::PoolMKLDNNOpKernel<float>,
                   ops::PoolMKLDNNOpKernel<double>);
@luotao1 luotao1 added the Intel label Feb 9, 2018
@mrysztow
Copy link

mrysztow commented Mar 6, 2018

Can we move the instruction to some wiki page, link it here and close the issue?

@luotao1
Copy link
Contributor

luotao1 commented Mar 7, 2018

Your suggestion sounds nice. The related documentation is new_op_kernel_en.md. Can you move the instruction to it?

@mrysztow
Copy link

mrysztow commented Mar 8, 2018

@pzelazko-intel can you validate the instruction above and if it is actual, insert it into the file pointed by @luotao1 ?

@pzelazko-intel
Copy link
Contributor

@jacquesqiao For MKLDNN kernels we actually derive from framework::OpKernel directly. Please take a look at #8879.

Does class representing MKLDNN matrix have to derive from LoDTensor? Couldn't it derive from Tensor?

@luotao1
Copy link
Contributor

luotao1 commented Mar 12, 2018

Does class representing MKLDNN matrix have to derive from LoDTensor? Couldn't it derive from Tensor?

It should derive from LoDTensor. The reason is that (sorry, we don't have a documentation about it now, we will add it):

All the input and output of ops are LoDTensor in fact. Ops should deliver the LoD information one by one, if one of them is not LoDTensor, the delivery is broken. For examples, A->B->C, if B doesn't use LoDTensor, C would not get the LoD information. (#3717)

The related code is

static const Tensor* GetTensorFromVar(Variable* var) {
if (var->IsType<LoDTensor>()) {
return var->GetMutable<LoDTensor>();
} else if (var->IsType<SelectedRows>()) {
return var->GetMutable<SelectedRows>()->mutable_value();
} else {
PADDLE_THROW("Variable type_id %s, expect LoDTensor/SelectedRows.",
var->Type().name());
}
}
static Tensor* GetMutableTensorFromVar(Variable* var) {
if (var->IsType<LoDTensor>()) {
return var->GetMutable<LoDTensor>();
} else if (var->IsType<SelectedRows>()) {
return var->GetMutable<SelectedRows>()->mutable_value();
} else {
PADDLE_THROW("Variable type_id %s, expect LoDTensor/SelectedRows.",
var->Type().name());
}
}
template <>
const Tensor* ExecutionContext::Input<Tensor>(const std::string& name) const {
auto* var = InputVar(name);
return var == nullptr ? nullptr
: GetTensorFromVar(const_cast<Variable*>(var));
}
template <>
const std::vector<const Tensor*> ExecutionContext::MultiInput<Tensor>(
const std::string& name) const {
auto names = op().Inputs(name);
std::vector<const Tensor*> res;
res.reserve(names.size());
std::transform(names.begin(), names.end(), std::back_inserter(res),
[&](const std::string& sub_name) {
auto var = scope_.FindVar(sub_name);
return var == nullptr ? nullptr : GetTensorFromVar(var);
});
return res;
}
template <>
Tensor* ExecutionContext::Output<Tensor>(const std::string& name) const {
auto var = OutputVar(name);
return var == nullptr ? nullptr : GetMutableTensorFromVar(var);
}
template <>
std::vector<Tensor*> ExecutionContext::MultiOutput<Tensor>(
const std::string& name) const {
auto names = op().Outputs(name);
std::vector<Tensor*> res;
res.reserve(names.size());
std::transform(names.begin(), names.end(), std::back_inserter(res),
[&](const std::string& sub_name) {
auto var = scope_.FindVar(sub_name);
return var == nullptr ? nullptr
: GetMutableTensorFromVar(var);
});
return res;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants