Skip to content

Conversation

@AmrDeveloper
Copy link
Member

Backport #139304

Copy link
Collaborator

@xlauko xlauko left a comment

Choose a reason for hiding this comment

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

LGTM, besides nits

Comment on lines +179 to 180
// CastOp, UnaryOp and VecExtractOp are here to perform a manual `fold` in
// applyOpPatternsGreedily.
Copy link
Collaborator

Choose a reason for hiding this comment

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

@bcardosolopes do you want to keep these operations in the non-canonicalized IR?
It might be possible to directly create them using OpBuilder::createOrFold in codegen which will eliminate the need to fold here. But it might be worth to be able to have unfolded version too, so I am just asking.

Copy link
Member

@bcardosolopes bcardosolopes May 16, 2025

Choose a reason for hiding this comment

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

@xlauko good question, I wasn't aware of createOrFold actually, neat. I guess it really depends on optimization level given we might be folding too much if we are emitting things to be statically analyzed? I don't have a concrete example though. A good answer could be: on a case basis during CIRGen if it doesn't look like affecting analysis quality?

@github-actions
Copy link

github-actions bot commented May 14, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@AmrDeveloper AmrDeveloper force-pushed the backport_vector_extract_fold branch from 163411a to bf8701f Compare May 14, 2025 20:26
@bcardosolopes bcardosolopes merged commit 3166478 into llvm:main May 16, 2025
19 checks passed
terapines-osc-cir pushed a commit to Terapines/clangir that referenced this pull request Sep 2, 2025
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.

3 participants