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

[handshake] StandardToHandshake conversion now removes blocks. #45

Merged
merged 1 commit into from
Jul 16, 2020

Conversation

stephenneuendorffer
Copy link
Contributor

This is a requirement imposed by the final version of Graph Regions.

This is a requirement imposed by the final version of Graph Regions.
Comment on lines 1422 to 1427
: public PassWrapper<HandshakeRemoveBlockPass,
OperationPass<handshake::FuncOp>> {
void runOnOperation() override {
auto funcOp = getOperation();
auto &entryBlock = funcOp.getBody().front().getOperations();

// Erase all TerminatorOp, and move ReturnOp to the end of entry block.
for (auto &block : funcOp) {
Operation &termOp = block.back();
if (isa<handshake::TerminatorOp>(termOp))
termOp.erase();
else if (isa<handshake::ReturnOp>(termOp))
entryBlock.splice(entryBlock.end(), block.getOperations(), termOp);
}

// Move all operations to entry block and erase other blocks.
for (auto &block :
llvm::make_early_inc_range(llvm::drop_begin(funcOp, 1))) {
entryBlock.splice(--entryBlock.end(), block.getOperations());
block.erase();
}
removeBasicBlocks(getOperation());
}
};
Copy link
Member

Choose a reason for hiding this comment

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

Maybe it's reasonable to delete the -remove-block-structure pass now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe. I think we should keep it for the moment. I'd like to think that we get multi-block graph regions supported before too long.

@stephenneuendorffer stephenneuendorffer merged commit 9d2042f into llvm:master Jul 16, 2020
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.

2 participants