Skip to content
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 symbolTable & symbolicDimProduct & symbolicDimMgr. #56351

Merged
merged 6 commits into from
Aug 24, 2023

Conversation

liuruyan
Copy link
Contributor

@liuruyan liuruyan commented Aug 16, 2023

PR types

Others

PR changes

Others

Description

  1. 新增 SymbolTable,用来存储SymbolicDimOp以及后续constraint_func。
  2. 新增SymbolicDimProduct,重写hash方法,重载==。
  3. 新增SymbolicDimMgr,管理SymbolicDim,与constraint强相关method暂未添加。
  4. 增加utils文件夹,存储上述组件。
  5. 独立编包ddim,原因是ir_shape中需要pd_type.cc中DenseTensorType,其依赖Phi中Ddim,独立拆除ddim防止直接依赖Phi
  6. 创建libir库时,使用ir_library收集所有SRCS文件,因此需在编译pd_type.cc时使用ir_library而不是cc_library,或另传入pd_type.cc文件

Other

Pcard-67164

@paddle-bot
Copy link

paddle-bot bot commented Aug 16, 2023

你的PR提交成功,感谢你对开源项目的贡献!
请关注后续CI自动化测试结果,详情请参考Paddle-CI手册
Your PR has been submitted. Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

@liuruyan liuruyan marked this pull request as draft August 23, 2023 09:56
@liuruyan liuruyan marked this pull request as ready for review August 23, 2023 09:58
ir_shape
SRCS
${SHAPE_SRCS}
${PADDLE_SOURCE_DIR}/paddle/fluid/ir/dialect/paddle_dialect/ir/pd_type.cc
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pd_type.cc 还包括了 phi::DataLayout, phi::LoD,这样是否也会导致ir 对 phi 的依赖

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

包含phi::DataLayout的只是一个头文件,而phi::LoD是一个std::vector<std::vector<size_t>>在预处理时就会头文件展开替换,应该不会导致对phi的依赖。

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

长期方案上,如果能把DenseTensorType下放是最安全的。因为后续不确定是否会有其他开发者在pd_type.cc里加额外的代码,可能会打破这个「clean DEPS」的预设。

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

长期方案上,如果能把DenseTensorType下放是最安全的。因为后续不确定是否会有其他开发者在pd_type.cc里加额外的代码,可能会打破这个「clean DEPS」的预设。

同认为应将 DenseTensorType 下沉

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

好的,那接下来PR会将DenseTensorType下沉,此PR作为临时方案。

Copy link
Contributor

@zhangbo9674 zhangbo9674 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@Aurelius84 Aurelius84 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

ir_shape
SRCS
${SHAPE_SRCS}
${PADDLE_SOURCE_DIR}/paddle/fluid/ir/dialect/paddle_dialect/ir/pd_type.cc
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

长期方案上,如果能把DenseTensorType下放是最安全的。因为后续不确定是否会有其他开发者在pd_type.cc里加额外的代码,可能会打破这个「clean DEPS」的预设。

Copy link
Contributor

@zhangbopd zhangbopd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zhangbopd zhangbopd merged commit fb89177 into PaddlePaddle:develop Aug 24, 2023
BeingGod pushed a commit to BeingGod/Paddle that referenced this pull request Sep 9, 2023
…56351)

* add symbolicDimProduct & symbolicDimMgr without method shape_constraint related

* split ddim in phi, add a target ddim, used by pd_type

* add pd_type.cc to ir_shape CMakeLists
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants