Skip to content

[CIR] Add get_element operation for computing pointer to array element #1748

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

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

tommymcm
Copy link
Contributor

Overview

Currently, getting the pointer to an element of an array requires a pointer decay and a (possible) pointer stride. A similar pattern for records has been eliminated with the cir.get_member operation. This PR provides a similar level of abstraction for arrays with the get_element operation.
get_element replaces the above pattern with a single operation, which takes a pointer to an array and an index, and produces a pointer to the element at that index.
There are many places in CIR analysis and lowering where the ptr_stride(array_to_ptrdecay(x), i) pattern is handled as a special case. By subsuming the special case pattern with an explicit operation, we make these analyses and lowering more robust.

Changes

Adds the cir.get_element operation.
Extends CIRGen to emit cir.get_element for array subscript expressions.
Updated LifetimeCheck to handle get_element operation, subsuming special case analysis of cir.ptr_stride operation (did not remove the special case).
Extends CIR-to-LLVM lowering to lower cir.get_element to llvm.getelementptr
Extends CIR-to-MLIR lowering to lower cir.get_element to memref operations, matching existing special case cir.ptr_stride lowering.

Additional Notes

Currently, 47.6% of cir.ptr_stride operations in the llvm-test-suite (SingleSource and MultiSource) can be replaced by cir.get_element operations.

Operator Breakdown (current)

name count %
cir.load 825221 22.27%
cir.br 429822 11.60%
cir.const 380381 10.26%
cir.cast 325646 8.79%
cir.store 309586 8.35%
cir.get_member 226895 6.12%
cir.get_global 186851 5.04%
cir.ptr_stride 158094 4.27%
cir.call 144522 3.90%
cir.binop 141142 3.81%
cir.alloca 134346 3.63%
cir.brcond 112864 3.05%
cir.cmp 83532 2.25%

Operator Breakdown (with get_element)

name count %
cir.load 825221 22.74%
cir.br 429822 11.84%
cir.const 380381 10.48%
cir.store 309586 8.53%
cir.cast 248645 6.85%
cir.get_member 226895 6.25%
cir.get_global 186851 5.15%
cir.call 144522 3.98%
cir.binop 141142 3.89%
cir.alloca 134346 3.70%
cir.brcond 112864 3.11%
cir.cmp 83532 2.30%
cir.ptr_stride 81093 2.23%
cir.get_elem 77001 2.12%

@erichkeane
Copy link
Collaborator

I don't really have a comment and didn't come up with anything on my review, but IMO, this seems like a good idea.

@andykaylor
Copy link
Collaborator

I think this is a very good addition to the dialect.

@tommymcm
Copy link
Contributor Author

Thanks for the review @andykaylor! I've applied the requested changes and responded to your comments above.

Copy link
Collaborator

@andykaylor andykaylor left a comment

Choose a reason for hiding this comment

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

lgtm, with one last nit

@tommymcm
Copy link
Contributor Author

Wicked, thanks for the help @andykaylor

@tommymcm
Copy link
Contributor Author

tommymcm commented Jul 22, 2025

I have rebased the PR after the incubator rebase this morning. Looks like there are some errors, digging into that now.

@tommymcm
Copy link
Contributor Author

Rebased PR again, works with the new main branch.

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.

4 participants