| CARVIEW |
Navigation Menu
-
Notifications
You must be signed in to change notification settings - Fork 5.8k
【CINN】Narrow the range from longlong to int #68791
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
Conversation
|
你的PR提交成功，感谢你对开源项目的贡献! |
|
Sorry to inform you that ea7fabf's CIs have passed for more than 7 days. To prevent PR conflicts, you need to re-run all CIs manually. |
paddle/cinn/optim/longlong2int.cc
Outdated
| bool is_overflow_ = false; | ||
| }; | ||
|
|
||
| class NarrowLonglong2Int : public ir::IRMutator<> { |
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.
这里叫cast或者convert会更直观些吗？
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, 已改名为 CastLonglong2Int.
paddle/cinn/optim/longlong2int.cc
Outdated
| void Visit(const ir::_Var_* op, Expr* expr) override { | ||
| if (expr->type().is_int(64)) { | ||
| expr->get()->set_type(Int(32)); | ||
| } |
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.
所有var都改成int32会不会有风险，有没有可能有些kernel里int64类型的var就是算值的，和下标没关系？
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.
THX, 已控制转换范围。
paddle/cinn/optim/longlong2int.cc
Outdated
| } | ||
| } | ||
| void Visit(const ir::IntImm* op, Expr* expr) override { | ||
| if (expr->type().is_int(64)) { |
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.
这里是否需要加强下检查，万一expr本身是越界的转了也会有问题
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, 已加强检查。
paddle/cinn/optim/longlong2int.cc
Outdated
| NarrowLonglong2Int narrow; | ||
| narrow(expr); |
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.
这里的转换感觉需要和前面的check逻辑对齐，check的时候是针对for做的，如果body内部的整数是下标计算理论上没问题，但本身是int计算的body是不是需要排除在外？
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.
| if (lb.defined()) lb->convert_int64_to_int32(); | ||
| if (ub.defined()) ub->convert_int64_to_int32(); | ||
| } | ||
| void CastBuffer(cinn::ir::Buffer& bf) { // NOLINT |
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.
这个是不叫CastBufferMeta更准确一些？
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.
thx, 下个PR中修改
| @@ -296,6 +296,10 @@ PD_DEFINE_bool(cinn_check_tensor_buffer_map, | |||
| BoolFromEnv("FLAGS_cinn_check_tensor_buffer_map", false), | |||
| "Whether to check tensor buffer mapping in cinn ir."); | |||
|
|
|||
| PD_DEFINE_bool(cinn_longlong2int_for_integer, | |||
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.
_for_integer的后缀需要加吗？
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.
后缀可以去掉，将在下个PR中去除
PR Category
Performance Optimization
PR Types
Performance
Description
Narrow the integer range if possible. (longlong -> int32)
Warnning: This pr may cause the performance degradation in some kernel, because the live register number will reduce to 32 when change the dtype longlong to int32, and unroll in nvcc may use too many register.
Pcard-67164