FC + elementwise_add (Residual connection)

[Silv3S]

PR types

Performance optimization

PR changes

OPs

Describe

• Fuse FC with elementwise_add so residual data is added inside FC operator,
• add unit tests with multiple cases for this fuse pass,

Benchmarks

Test were conducted with oneDNN 2.5.4 , fc_mkldnn_pass and fc_act_mkldnn_pass were enabled:

• VIT-OCR is performing 4% faster (12 fc+residual fusions),
• BERT is performing 8% faster (24 fc+residual fusions).

[jczaja]

LGTM

[jczaja]

@zhiqiu Could you please review changes at fc_mkldnn_op.cc related to ShareDataWith ? Changes are modeled after conv_mkldnn_op.cc where residual connections part is also using ShareDataWith(). We tried to use TensorCopy but there was significant performance degradation, so FC and CONV both are using ShareDataWith. We need your approval to pass CI. Feel free to ask questions

[jczaja]

@baoachun Please review

[baoachun]

Hi @Silv3S, well done！Could you please attach performance and accuracy comparison data for this pass?

[lidanqing-intel]

@baoachun After @Silv3S provide performance, could you please help with PR-CI-APPROVAL? Thanks !
From the PR description, I can see with this fuse pass VIT-OCR is performing 4% faster.

[zhiqiu]

@zhiqiu Could you please review changes at fc_mkldnn_op.cc related to ShareDataWith ? Changes are modeled after conv_mkldnn_op.cc where residual connections part is also using ShareDataWith(). We tried to use TensorCopy but there was significant performance degradation, so FC and CONV both are using ShareDataWith. We need your approval to pass CI. Feel free to ask questions

Hi, it is not suggested to ShareData of the input and output of an operator, since it makes the op be an implicit in-place indeed. So is it possible to make residual the same varariable as Output, just like Param and ParamOut in sgd operator.

[Silv3S]

Hi @Silv3S, well done！Could you please attach performance and accuracy comparison data for this pass?

Hi @baoachun, thank you! I updated PR description to provide performance comparison for affected models. I also prepared test which compares results between NativeConfig and AnalysisConfig, but I will need your help with uploading needed data.txt file to vit-ocr.tgz repository.

[Silv3S]

Hi, it is not suggested to ShareData of the input and output of an operator, since it makes the op be an implicit in-place indeed. So is it possible to make residual the same varariable as Output, just like Param and ParamOut in sgd operator.

Thank you for review @zhiqiu. Since we don't modify residual_param inside FC_op, I set it explicitly as output. Now ShareDataWith is called on two outputs of operator. Please advise if this approach is good.

[zhiqiu]

LGTM

[zhiqiu]

Hi, it is not suggested to ShareData of the input and output of an operator, since it makes the op be an implicit in-place indeed. So is it possible to make residual the same varariable as Output, just like Param and ParamOut in sgd operator.

Thank you for review @zhiqiu. Since we don't modify residual_param inside FC_op, I set it explicitly as output. Now ShareDataWith is called on two outputs of operator. Please advise if this approach is good.

Thanks, LGTM

[Silv3S]

Hi @XieYunshen, could you please approve setting timeout for new unit test?

[Silv3S]

Continued in #41776 due to CI bug (non refreshing checks). Please approve second PR (code didn't change)