| CARVIEW |
Navigation Menu
-
Notifications
You must be signed in to change notification settings - Fork 5.8k
add fc te #71931
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
add fc te #71931
Conversation
|
|
|
你的PR提交成功，感谢你对开源项目的贡献! |
linkk08
force-pushed
the
add_fc_te
branch
2 times, most recently
from
48f0f09 to
a9c6382
Compare
| char *endptr; | ||
| errno = 0; | ||
| auto val = std::strtol(value, &endptr, 16); | ||
| if (*endptr != '\0' || errno != 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
errno 在这里是不是没什么作用？
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
errno 在这里是不是没什么作用？
已删除
| errno = 0; | ||
| auto val = std::strtol(value, &endptr, 16); | ||
| if (*endptr != '\0' || errno != 0) { | ||
| std::cerr << "Invalid XPU_PADDLE_FC_PATTERN: " << value << std::endl; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这里使用 std::cerr 是符合预期的吗？有 log(warning) 之类的不
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这里使用 std::cerr 是符合预期的吗？有 log(warning) 之类的不
已修改成LOG(WARNING)
| PADDLE_ENFORCE_XBLAS_SUCCESS(r, "xblas_fc_fusion"); | ||
| bool is_xte = false; | ||
| if constexpr (std::is_same<XPUTypeBF16, XPUType>::value) { | ||
| if (std::getenv("XPU_PADDLE_FC_BFLOAT16_XTE") != nullptr) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
328行和329行是不是应该调换下顺序？现在这个写法，如果 x 是 bfloat16 且没有设置 XPU_PADDLE_FC_BFLOAT16_XTE 的话，这个函数什么都不做就 return 了
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
328行和329行是不是应该调换下顺序？现在这个写法，如果 x 是 bfloat16 且没有设置 XPU_PADDLE_FC_BFLOAT16_XTE 的话，这个函数什么都不做就 return 了
会进入if (!is_xte) { 的逻辑
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
| PADDLE_ENFORCE_XBLAS_SUCCESS(r, "xblas_fc_fusion"); | ||
| bool is_xte = false; | ||
| if constexpr (std::is_same<XPUTypeBF16, XPUType>::value) { | ||
| if (std::getenv("XPU_PADDLE_FC_BFLOAT16_XTE") != nullptr) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
| scale_w_mode); | ||
|
|
||
| PADDLE_ENFORCE_XBLAS_SUCCESS(r, "xblas_fc_fusion"); | ||
| bool is_xte = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个is_xte感觉是不是没必要呀？直接用std::is_same<XPUTypeBF16, XPUType>::value && std::getenv(XPU_PADDLE_FC_BFLOAT16_XTE) != nullptr控制两个分支就行了？
是有啥编译问题吗？
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个is_xte感觉是不是没必要呀？直接用
std::is_same<XPUTypeBF16, XPUType>::value && std::getenv(XPU_PADDLE_FC_BFLOAT16_XTE) != nullptr控制两个分支就行了？是有啥编译问题吗？
感觉前面写了if constexpr (std::is_same<XPUTypeBF16, XPUType>::value) {
if (std::getenv("XPU_PADDLE_FC_BFLOAT16_XTE") != nullptr) {
后面的逻辑是这个逻辑的补充, 再写一个类似的逻辑会显得比较复杂且易读性没那么好
| @@ -348,7 +422,8 @@ static void xblas_fc_wrapper(xpu::Context* ctx, | |||
| scale_x_mode, | |||
| scale_w_mode); | |||
|
|
|||
| PADDLE_ENFORCE_XBLAS_SUCCESS(r, "xblas_fc_fusion"); | |||
| PADDLE_ENFORCE_XDNN_SUCCESS(r, "xblas_fc_fusion"); | |||
| return; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这里感觉没啥必要吧 ，函数后面已经没有其他语句了？
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这里感觉没啥必要吧 ，函数后面已经没有其他语句了？
done
| @@ -370,6 +445,7 @@ static void xblas_fc_wrapper(xpu::Context* ctx, | |||
| bias, | |||
| act); | |||
| PADDLE_ENFORCE_XDNN_SUCCESS(r, "xpu_fc_fusion"); | |||
| return; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
同上
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
同上
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
PR Category
Custom Device
PR Types
New features
Description
add fc te scheme to the dynamic graph