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

[CIR] Split cir-simplify into two passes #868

Merged
merged 1 commit into from
Sep 19, 2024

Conversation

Lancern
Copy link
Collaborator

@Lancern Lancern commented Sep 19, 2024

This PR splits the old cir-simplify pass into two new passes, namely cir-canonicalize and cir-simplify (the new cir-simplify). The cir-canonicalize pass runs transformations that do not affect CIR-to-source fidelity much, such as operation folding and redundant operation elimination. On the other hand, the new cir-simplify pass runs transformations that may significantly change the code and break high-level code analysis passes, such as more aggresive code optimizations.

This PR also updates the CIR-to-CIR pipeline to fit these two new passes. The cir-canonicalize pass is moved to the very front of the pipeline, while the new cir-simplify pass is moved to the back of the pipeline (but still before lowering prepare of course). Additionally, the new cir-simplify now only runs when the user specifies a non-zero optimization level on the frontend.

Also fixed some typos and resolved some clang-tidy complaints along the way.

Resolves #827 .

This patch splits the old cir-simplify pass into two new passes, namely
cir-canonicalize and cir-simplify (the new cir-simplify). The cir-canonicalize
pass runs transformations that do not affect CIR-to-source fidelity much, such
as operation folding and redundant operation elimination. On the other hand, the
new cir-simplify pass runs transformations that may significantly change the
code and break high-level code analysis passes, such as more aggresive code
optimizations.

This patch also updates the CIR-to-CIR pipeline to fit these two new passes. The
cir-canonicalize pass is moved to the very front of the pipeline, while the new
cir-simplify pass is moved to the back of the pipeline (but still before
lowering prepare of course). Additionally, the new cir-simplify now only runs
when the user specifies a non-zero optimization level on the frontend.
Copy link
Member

@bcardosolopes bcardosolopes left a comment

Choose a reason for hiding this comment

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

LGTM, one minor note for post consideration

@@ -293,11 +169,7 @@ void CIRSimplifyPass::runOnOperation() {
// Collect operations to apply patterns.
SmallVector<Operation *, 16> ops;
getOperation()->walk([&](Operation *op) {
// CastOp here is to perform a manual `fold` in
// applyOpPatternsAndFold
if (isa<BrOp, BrCondOp, ScopeOp, SwitchOp, CastOp, TryOp, UnaryOp,
Copy link
Member

Choose a reason for hiding this comment

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

We can tackle this in followup PRs, but shouldn't UnaryOp be part of CIRSimplify? Some of its folding definitely affects source fidelity, perhaps we need to move some of that logic out of the ::fold into a rewriter in CIRSimplify.

@bcardosolopes bcardosolopes merged commit 923264b into llvm:main Sep 19, 2024
6 of 7 checks passed
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.

Split CIRSimplify into two new passes
2 participants