Fused elementwises kernels and ops #51427
Conversation
|
你的PR提交成功，感谢你对开源项目的贡献! |
| @@ -175,7 +175,7 @@ if(WITH_MKLDNN) | |||
| pass_library(softplus_activation_mkldnn_fuse_pass inference DIR mkldnn) | |||
| pass_library(shuffle_channel_mkldnn_detect_pass inference DIR mkldnn) | |||
| pass_library(fc_act_mkldnn_fuse_pass inference DIR mkldnn) | |||
| pass_library(elt_act_mkldnn_fuse_pass inference DIR mkldnn) | |||
| pass_library(elt_act_onednn_fuse_pass inference DIR mkldnn) | |||
There was a problem hiding this comment.
As we change it already, please change elt to either eltwise or elementwise as it's more common among other passes.
| } | ||
| extra { | ||
| attrs { | ||
| name: "x_data_format" |
There was a problem hiding this comment.
x_data_format and y_data_format seem to be deprecated attributes. Please don't add them here. They should be also removed from base operators
| type: FLOAT | ||
| } | ||
| attrs { | ||
| name: "Scale_x" |
There was a problem hiding this comment.
| name: "Scale_x" | |
| name: "scale_x" |
All attributes should be lowercase. We must unify it, as there are currently too many duplicates with quantization scales.
| attrs { | ||
| name: "fuse_activation" | ||
| type: STRING | ||
| } |
There was a problem hiding this comment.
| attrs { | |
| name: "fuse_activation" | |
| type: STRING | |
| } | |
| attrs { | |
| name: "fuse_activation" | |
| type: STRING | |
| } | |
| attrs { | |
| name: "fuse_alpha" | |
| type: FLOAT | |
| } | |
| attrs { | |
| name: "fuse_beta" | |
| type: FLOAT | |
| } |
| attrs { | ||
| name: "alpha" | ||
| type: FLOAT | ||
| } | ||
| attrs { | ||
| name: "beta" | ||
| type: FLOAT | ||
| } |
There was a problem hiding this comment.
| attrs { | |
| name: "alpha" | |
| type: FLOAT | |
| } | |
| attrs { | |
| name: "beta" | |
| type: FLOAT | |
| } |
| name: "fuse_activation" | ||
| type: STRING | ||
| } | ||
| } |
There was a problem hiding this comment.
There is also float fused_output_scale from operator_scale_onednn_fuse_pass
| name: "fuse_activation" | ||
| type: STRING | ||
| } | ||
| } |
There was a problem hiding this comment.
All comments to fused_elementwise_add.pbtxt are valid for other fused_elementwise_.pbtxt
| name: "fuse_activation" | ||
| type: STRING | ||
| } | ||
| } |
There was a problem hiding this comment.
elementwise_mul can have fused_unsqueeze2_axes attribute fromoperator_unsqueeze2_onednn_fuse_pass
Fused elementwise ops
YuanRisheng
requested review from
YuanRisheng,
XieYunshen,
XiaoguangHu01,
zyfncg,
JiabinYang and
Silv3S
|
I am sorry to inform you that you need update code again. I have analyzed the CI-Coverage and found the problem. The test |
HulekJakub
dismissed stale reviews from Silv3S, Aurelius84, XieYunshen, zyfncg, and YuanRisheng
via
7d2fa1a
|
@YuanRisheng Done. I've seen that PR-CI-Coverage first checks approves and fails if there aren't required ones, and only once the approves are there it runs tests, which really slows development (tests work on local, have to wait for approves, test doesn't work on CI, approves get dismissed which is just a waste of time) |
@HulekJakub Thank you for your feedback. I will report this issue to my team. And CI-Windows fails because of |
PR types
Others
PR changes
Others
Description