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] Make use of !invariant.group metadata for const allocas #1159

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Lancern
Copy link
Member

@Lancern Lancern commented Nov 24, 2024

This PR updates the LLVM lowering part of load and stores to const allocas and makes use of the !invariant.group metadata in the result LLVM IR.

The HoistAlloca pass is also updated. The const flag on a hoisted alloca is removed for now since their uses are not always invariants. Will update in later PRs to teach their invariants.

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.

Overall looks good to me, thanks for the incremental approach! One remaining question: should this be enabled only -O1+ or do we want this for -O0 as well?

This patch updates the LLVM lowering part of load and stores to const allocas
and makes use of the !invariant.group metadata in the result LLVM IR.

The HoistAlloca pass is also updated. The const flag on a hoisted alloca is
removed for now since their uses are not always invariants. Will update in later
patches to teach their invariants.
@Lancern
Copy link
Member Author

Lancern commented Nov 26, 2024

One remaining question: should this be enabled only -O1+ or do we want this for -O0 as well?

I think we should emit the metadata under -O1+ to prevent LLVM from any possible optimization under -O0.

And I realized that LowerToLLVM code does not have access to the code generation option which keeps the optimization flags. Sent #1171 for this and I'm marking this PR as draft until that PR lands.

@Lancern Lancern marked this pull request as draft November 26, 2024 16:51
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