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

AIRCopyToDma: Compose a chain of memref ops to src/dst memrefs, as canonicalizer #749

Merged
merged 6 commits into from
Oct 24, 2024

Conversation

erwei-xilinx
Copy link
Collaborator

No description provided.

Comment on lines 1482 to 1484
for (unsigned i = 0; i < op->getNumResults(); i++) {
op->getResult(i).replaceAllUsesWith(newOp->getResult(i));
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use the rewriter instead of direct IR modification:

Suggested change
for (unsigned i = 0; i < op->getNumResults(); i++) {
op->getResult(i).replaceAllUsesWith(newOp->getResult(i));
}
rewriter.replaceOp(op, newOp);

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Great point. Updated to use replaceOpWithNewOp method.

Comment on lines 1347 to 1349
for (unsigned i = 0; i < op->getNumResults(); i++) {
op->getResult(i).replaceAllUsesWith(newOp->getResult(i));
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use the rewriter instead of direct IR modification:

Suggested change
for (unsigned i = 0; i < op->getNumResults(); i++) {
op->getResult(i).replaceAllUsesWith(newOp->getResult(i));
}
rewriter.replaceOp(op, newOp);

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated.

Comment on lines 1434 to 1436
for (unsigned i = 0; i < op->getNumResults(); i++) {
op->getResult(i).replaceAllUsesWith(newOp->getResult(i));
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use the rewriter instead of direct IR modification:

Suggested change
for (unsigned i = 0; i < op->getNumResults(); i++) {
op->getResult(i).replaceAllUsesWith(newOp->getResult(i));
}
rewriter.replaceOp(op, newOp);

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated.

Comment on lines 1385 to 1387
for (unsigned i = 0; i < op->getNumResults(); i++) {
op->getResult(i).replaceAllUsesWith(newOp->getResult(i));
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use the rewriter instead of direct IR modification:

Suggested change
for (unsigned i = 0; i < op->getNumResults(); i++) {
op->getResult(i).replaceAllUsesWith(newOp->getResult(i));
}
rewriter.replaceOp(op, newOp);

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated.

Comment on lines -1618 to -1623
for (auto dmaOp : dma_ops) {
if (failed(condenseMemrefDataReorderingToAIRDma(
std::get<0>(dmaOp), std::get<1>(dmaOp), std::get<2>(dmaOp)))) {
return signalPassFailure();
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Run the moved code here?

Suggested change
for (auto dmaOp : dma_ops) {
if (failed(condenseMemrefDataReorderingToAIRDma(
std::get<0>(dmaOp), std::get<1>(dmaOp), std::get<2>(dmaOp)))) {
return signalPassFailure();
}
}
RewritePatternSet pattern(ctx);
DmaMemcpyNdOp::getCanonicalizationPatterns(pattern, ctx);
(void)applyPatternsAndFoldGreedily(module, std::move(pattern));

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Great point. Updated.

@erwei-xilinx erwei-xilinx enabled auto-merge (squash) October 24, 2024 02:17
@erwei-xilinx erwei-xilinx merged commit 0541d96 into Xilinx:main Oct 24, 2024
11 checks passed
@erwei-xilinx erwei-xilinx deleted the compose_memref_ops_as_cano branch October 24, 2024 03:10
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