Skip to content

Conversation

@wanghuancoder
Copy link
Contributor

@wanghuancoder wanghuancoder commented Jan 11, 2024

PR types

New features

PR changes

Others

Description

Pcard-67164

PIR适配OneDNN 支持OneDNNMixedPhiKernelInstruction,并支持pad3d。

此外还做了如下处理:

  1. 修复了data_format_tensors处理存在的bug
  2. 增加了如果有OneDNN kernel则单线程执行的逻辑(这里将来还要斟酌是否对性能有影响)
  3. Predictor的Pass全部加在了GPU里,CPU没有Pass处理。本PR加了一些基础Pass给CPU
  4. 由于Predictor不能有Fetch,将Fetch改成了ShadowOutputOp。如果OneDNN Tensor作为ShadowOutputOp,需要加trans layout
  5. 为了支持mixedInstruction,为PHI Kernel基类增加了check_if_onednn_kernel_support_接口
  6. 为预测测试基类auto_scan_test增加了pir_run_test方法

@paddle-bot
Copy link

paddle-bot bot commented Jan 11, 2024

你的PR提交成功,感谢你对开源项目的贡献!
请关注后续CI自动化测试结果,详情请参考Paddle-CI手册
Your PR has been submitted. Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

kangguangli
kangguangli previously approved these changes Jan 11, 2024
gpu_pm.Run(pir_program_.get());
} else {
::pir::PassManager cpu_pm(::pir::IrContext::Instance(), 2);
cpu_pm.AddPass(::pir::CreateReplaceFetchWithShadowOutputPass());
Copy link
Contributor

Choose a reason for hiding this comment

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

这行重复了吧?可以删掉这个

Copy link
Contributor Author

Choose a reason for hiding this comment

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

好的好的感谢!

data_format_tensors : x, out, mid_out, out_grad

- op : pad3d
extra_args :
Copy link
Contributor

Choose a reason for hiding this comment

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

没有extra_args仍需要指定这个key吗?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这个底层能力上是可以不指定的。但是我写这里的时候,想了一下,还是想放个空的。大多数op都有extra_args,写一个空的extra_args更能让看代码的人一眼就知道他的extra_args是空的。

dic["use_gpu"] = enable_gpu
return str(dic)

def pir_run_test_config(
Copy link
Contributor

Choose a reason for hiding this comment

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

给MkldnnAutoScanTest派生出PirMkldnnAutoScanTest是不是更好点?而不是给MkldnnAutoScanTest增加新成员函数。之后的单测可继承PirMkldnnAutoScanTest

Copy link
Contributor

Choose a reason for hiding this comment

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

能复用的代码,还是建议抽出来尽量复用下,减少后期维护成本

Copy link
Contributor Author

Choose a reason for hiding this comment

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

好的好的

Comment on lines +1367 to +1376
#ifdef PADDLE_WITH_DNNL
auto new_in_type = new_in.type();
if (new_in_type.isa<AllocatedDenseTensorType>()) {
if (new_in_type.dyn_cast<AllocatedDenseTensorType>().data_layout() ==
phi::DataLayout::ONEDNN) {
new_in = AddOneDNN2PaddleLayoutTransferOp(
new_in, phi::DataLayout::ANY, block);
}
}
#endif
Copy link
Contributor

Choose a reason for hiding this comment

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

这个不太合理吧,大家编译的时候带上WITH_MKLDNN,但是不跑mkldnn模式,这里会影响其他模式逻辑?

Copy link
Contributor

Choose a reason for hiding this comment

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

看错了,有这个判断不会的if (new_in_type.dyn_cast().data_layout() == phi::DataLayout::ONEDNN)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

在不跑mkldnn模式的时候,new_in_type.dyn_cast().data_layout() == phi::DataLayout::ONEDNN 为 false。所以不会影响其它模式。但位置在当前pd_op_to_kernel_pass.cc的代码框架下,只能放在这里。

#ifdef PADDLE_WITH_DNNL
for (auto& ins : vec_instruction_base_) {
if (static_cast<OneDNNPhiKernelInstruction*>(ins.get()) != nullptr ||
static_cast<OneDNNLegacyKernelInstruction*>(ins.get()) != nullptr ||
Copy link
Contributor

Choose a reason for hiding this comment

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

C++11 有 std::is_base_of<A, B>::Value ,另外只需要判断是否是OnDNN的基类是否就可以了?

@wanghuancoder wanghuancoder merged commit d5f606f into PaddlePaddle:develop Jan 18, 2024
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.

5 participants