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

[PIR]polish iterator of block #59118

Merged
merged 1 commit into from
Nov 20, 2023
Merged

Conversation

winter-wang
Copy link
Contributor

@winter-wang winter-wang commented Nov 18, 2023

PR types

Others

PR changes

Others

Description

规范PIR下的Opetion、Region、Block的迭代器接口及使用方式。

此PR合入之前:PIR组件的遍历方式为:

pir::Operation op = .....;
// Operation没有定义迭代器,只能根据下标遍历包含的region。
for(size_t index = 0; index < op.num_regions(); ++index) {
  auto& region = op.region(index);
  
// Region定义的迭代器类型借用了std::list<Block*>::iterator。 显然,该迭代器必须解引用两次才可以获取具体的Block对象。
// block的类型是 Block*。要想获得具体的Block,必须再次解引用。
for(auto block : region) {
      
      // Block定义的迭代器类型借用了std::list<Operation*>::iterator。 显然,该迭代器必须解引用两次才可以获取具体的Operation对象。
      // sub_op的类型是 Operation*。要想获得具体的Operation,必须再次解引用。
     for(auto sub_op : *block)
        pir::Operation& inner_op = *sub_op;
        ......       
  }
}

如上所述,Operation没有定义迭代器,Region、Block的迭代器必须经两次解引用才可以获得相应的Block、Operaiton。
本PR为Operation、Region、Block定义了自己的专属迭代器。
本PR合入后,IR组件的遍历方式为:

pir::Operation op = .....;
// Operation定义了迭代器,可以直接范围for循环,获取包含的Region的引用。
for(auto& region: op) {
  // Region的迭代器变更为自定义的Region::Iterator, 该迭代器解引用会直接得到Block&。
  // Region可以直接范围for循环,获取包含的Block的引用。
 for(auto& block : region) {
     // Block的迭代器变更为自定义的Block::Iterator, 该迭代器解引用会直接得到Operation&。
     // Block可以直接范围for循环,获取包含的Operation的引用。
     for(auto& sub_op : block)
        pir::Operation& inner_op = sub_op;
        ......       
  }
}

本PR统一了Operaiton、Region、Block这三个PIR组件的遍历方式。

Other

Pcard-67164

Copy link

paddle-bot bot commented Nov 18, 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.

@winter-wang winter-wang force-pushed the cf_develop branch 15 times, most recently from 5547d3c to 70c003f Compare November 18, 2023 16:10
@PaddlePaddle PaddlePaddle locked and limited conversation to collaborators Nov 18, 2023
@PaddlePaddle PaddlePaddle unlocked this conversation Nov 18, 2023
@winter-wang winter-wang reopened this Nov 18, 2023
@PaddlePaddle PaddlePaddle locked and limited conversation to collaborators Nov 18, 2023
@PaddlePaddle PaddlePaddle unlocked this conversation Nov 18, 2023
@winter-wang winter-wang reopened this Nov 18, 2023
@PaddlePaddle PaddlePaddle locked and limited conversation to collaborators Nov 18, 2023
@PaddlePaddle PaddlePaddle unlocked this conversation Nov 18, 2023
@winter-wang winter-wang reopened this Nov 18, 2023
@winter-wang winter-wang reopened this Nov 18, 2023
@PaddlePaddle PaddlePaddle locked and limited conversation to collaborators Nov 18, 2023
@PaddlePaddle PaddlePaddle unlocked this conversation Nov 18, 2023
@winter-wang winter-wang force-pushed the cf_develop branch 6 times, most recently from 6799572 to 7345a80 Compare November 19, 2023 17:08
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.

Great work! 这个「用法统一」太有用了

Copy link
Contributor

@XieYunshen XieYunshen 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

@XiaoguangHu01 XiaoguangHu01 left a comment

Choose a reason for hiding this comment

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

LGTM

@winter-wang winter-wang merged commit 3525e9b into PaddlePaddle:develop Nov 20, 2023
28 checks passed
SecretXV pushed a commit to SecretXV/Paddle that referenced this pull request Nov 28, 2023
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.

7 participants