| CARVIEW |
Navigation Menu
-
Notifications
You must be signed in to change notification settings - Fork 5.8k
[DimExpr] DimExpr simplify #60098
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
[DimExpr] DimExpr simplify #60098
Conversation
… into dim-expr-simplify
|
你的PR提交成功，感谢你对开源项目的贡献! |
| @@ -51,6 +51,16 @@ TEST(DimExpr, constraint) { | |||
| ASSERT_EQ(static_cast<int>(constraints.size()), 1); | |||
| } | |||
|
|
|||
| TEST(Simplify, NumberArithmetic) { | |||
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.
目前是只添加了对int计算的化简功能吗，含有符号的情况还没添加？如果有其他功能，最好也一起添加单测
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.
符号化简的单测位置：paddle/cinn/common/dim_expr_simplify_test.cc
本 PR 的单测在两个文件：
symbol_dim_expr_test.cc：表达式里面不包含符号，都是 std::int64_t 类型，自动直接进行四则运算dim_expr_simplify_test.cc: 表达式里面包含符号，提供了SimplifyDimExpr函数接口，用户需要明确调用SimplifyDimExpr之后，才对符号做化简
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.
提个建议哈，最好把单测整理到一个文件里，这样使用方可以比较方便找到所有支持的用法哈
| DimExpr number = DimExpr(5); | ||
| DimExpr add_minus = number + number - number; | ||
| ASSERT_TRUE((add_minus.Has<std::int64_t>())); | ||
| ASSERT_EQ((add_minus.Get<std::int64_t>()), 5); |
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.
- DimExpr 的判等 PR 已合入：[DimExpr] Support operator== #60041
* Dim Expr Interface * CreateDimExprBuilder * DimExpr ValueShape * Remove one unit test * Fix cmake * DimExpr basic definition * DimExpr basic definition * Change DimExpr comments * Unit test passed * Fix CI error * Delete useless test * DimExpr Support operator== * Fix variant inherit error * Fix Windows cmake error * SimplifyDimExpr * More unit test * DimExpr Simplify * Pass unit test * Remove Redundant Equal * Fix compile error * Fix compile error * Fix compile error * Fix compile error * Add more Unittest * Fix NumberArithmetic
| } else { | ||
| return Op<DimExpr>{ret_operand}; | ||
| } | ||
| LOG(FATAL) << "Dead code."; |
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.
这个 LOG(FATAL) 什么情况下会走进去呢？下同
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.
这是添加的防御性代码，保证这里不应该被执行到，即前面的 if 分支必须命中
| struct GetOrderValue; | ||
|
|
||
| template <> | ||
| struct GetOrderValue<Broadcast<DimExpr>> { |
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.
保证op计算优先级是不是可以用枚举简化一些
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.
实现上也可以做到，但感觉枚举值用来比较大小不太好
|
|
||
| bool IsLhsBeforeRhs(const DimExpr& lhs, const DimExpr& rhs); | ||
|
|
||
| template <template <typename> class Op> |
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.
template <template <typename> class ShapeOpT> 感觉放在 PIR下容易和 pir的 op搞混，下同
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 修了
| */ | ||
| template <template <typename> class Op> | ||
| struct SortOperands { | ||
| using dim_expr_type = Op<DimExpr>; |
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.
using DimExprT = Op<DimExpr>;
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.
C++ std 里也有这样的写法，使用小写字母来实现模版
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.
嗯嗯、只是和 paddle 编码风格不太一致、pir 中至少没有
|
|
||
| namespace { | ||
|
|
||
| DimExpr BD(const DimExpr& lhs, const DimExpr& rhs) { |
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.
BD ——> BC 下同
PR types
New features
PR changes
Others
Description
pcard-76996
[DimExpr] DimExpr simplify
Provide a simplification function for DimExpr. This function supports the basic simplification of DimExpr. In the future, when integrating with the backend's AutoSimplify, an approach similar to that of Boost should be adopted. Each has its strengths, and by working together, they can make the simplification of Expr more robust.
提供对 DimExpr 的化简函数，此函数支持对 DimExpr 的简单化简功能，未来和后端的 AutoSimplify 结合时，应该采用类似于 boost 的思路，两者各有所长，一起工作从而使得 Expr 的化简更鲁棒