Skip to content

[InstCombine] Split GEPs with multiple variable indices #137297

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

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

Conversation

nikic
Copy link
Contributor

@nikic nikic commented Apr 25, 2025

Split GEPs that have more than one variable index into two. This is in preparation for the ptradd migration, which will not support multi-index GEPs.

; CHECK-NEXT: [[GEP1:%.*]] = getelementptr nuw [2 x i16], ptr [[GEP1_SPLIT]], i64 0, i64 [[B:%.*]]
; CHECK-NEXT: [[GEP2_SPLIT:%.*]] = getelementptr nuw [2 x i32], ptr [[P]], i64 [[C:%.*]]
; CHECK-NEXT: [[GEP2:%.*]] = getelementptr nuw [2 x i32], ptr [[GEP2_SPLIT]], i64 0, i64 [[D:%.*]]
; CHECK-NEXT: [[CMP:%.*]] = icmp eq ptr [[GEP1]], [[GEP2]]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Regression. We fail to fold icmp of geps with same base if there is more than one GEP involved.

nikic added a commit to nikic/llvm-project that referenced this pull request Apr 25, 2025
Support the ptrtoint(gep null, x) -> x and
ptrtoint(gep inttoptr(x), y) -> x+y folds for the case where there
is a chain of geps that ends in null or inttoptr. This avoids some
regressions from llvm#137297.

While here, also be a bit more careful about edge cases like
pointer to vector splats and mismatched pointer and index size.
nikic added a commit to nikic/llvm-project that referenced this pull request Apr 25, 2025
Support the ptrtoint(gep null, x) -> x and
ptrtoint(gep inttoptr(x), y) -> x+y folds for the case where there
is a chain of geps that ends in null or inttoptr. This avoids some
regressions from llvm#137297.

While here, also be a bit more careful about edge cases like
pointer to vector splats and mismatched pointer and index size.
nikic added a commit that referenced this pull request Apr 28, 2025
Support the ptrtoint(gep null, x) -> x and ptrtoint(gep inttoptr(x), y)
-> x+y folds for the case where there is a chain of geps that ends in
null or inttoptr. This avoids some regressions from #137297.
    
While here, also be a bit more careful about edge cases like pointer to
vector splats and mismatched pointer and index size.
jyli0116 pushed a commit to jyli0116/llvm-project that referenced this pull request Apr 28, 2025
…37323)

Support the ptrtoint(gep null, x) -> x and ptrtoint(gep inttoptr(x), y)
-> x+y folds for the case where there is a chain of geps that ends in
null or inttoptr. This avoids some regressions from llvm#137297.
    
While here, also be a bit more careful about edge cases like pointer to
vector splats and mismatched pointer and index size.
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…37323)

Support the ptrtoint(gep null, x) -> x and ptrtoint(gep inttoptr(x), y)
-> x+y folds for the case where there is a chain of geps that ends in
null or inttoptr. This avoids some regressions from llvm#137297.
    
While here, also be a bit more careful about edge cases like pointer to
vector splats and mismatched pointer and index size.
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…37323)

Support the ptrtoint(gep null, x) -> x and ptrtoint(gep inttoptr(x), y)
-> x+y folds for the case where there is a chain of geps that ends in
null or inttoptr. This avoids some regressions from llvm#137297.
    
While here, also be a bit more careful about edge cases like pointer to
vector splats and mismatched pointer and index size.
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…37323)

Support the ptrtoint(gep null, x) -> x and ptrtoint(gep inttoptr(x), y)
-> x+y folds for the case where there is a chain of geps that ends in
null or inttoptr. This avoids some regressions from llvm#137297.
    
While here, also be a bit more careful about edge cases like pointer to
vector splats and mismatched pointer and index size.
Ankur-0429 pushed a commit to Ankur-0429/llvm-project that referenced this pull request May 9, 2025
…37323)

Support the ptrtoint(gep null, x) -> x and ptrtoint(gep inttoptr(x), y)
-> x+y folds for the case where there is a chain of geps that ends in
null or inttoptr. This avoids some regressions from llvm#137297.
    
While here, also be a bit more careful about edge cases like pointer to
vector splats and mismatched pointer and index size.
nikic added a commit that referenced this pull request Jun 10, 2025
Currently OptimizePointerDifference() only handles single GEPs with a
common base, not GEP chains. This patch generalizes the support to
nested GEPs with a common base.

Finding the common base is a bit annoying because we want to stop as
soon as possible and not recurse into common GEP prefixes.

This helps avoids regressions from
#137297.
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Jun 10, 2025
…ence (#142958)

Currently OptimizePointerDifference() only handles single GEPs with a
common base, not GEP chains. This patch generalizes the support to
nested GEPs with a common base.

Finding the common base is a bit annoying because we want to stop as
soon as possible and not recurse into common GEP prefixes.

This helps avoids regressions from
llvm/llvm-project#137297.
Split GEPs that have more than one variable index into two. This
is in preparation for the ptradd migration, which will not support
multi-index GEPs.
rorth pushed a commit to rorth/llvm-project that referenced this pull request Jun 11, 2025
…142958)

Currently OptimizePointerDifference() only handles single GEPs with a
common base, not GEP chains. This patch generalizes the support to
nested GEPs with a common base.

Finding the common base is a bit annoying because we want to stop as
soon as possible and not recurse into common GEP prefixes.

This helps avoids regressions from
llvm#137297.
nikic added a commit to nikic/llvm-project that referenced this pull request Jun 11, 2025
This depth limits a linear search (rather than the usual potentially
exponential one) and is not particularly important for compile-time
in practice.

The change in llvm#137297 is going to increase the length of GEP chains,
so I'd like to increase this limit a bit to reduce the chance of
regressions.
nikic added a commit that referenced this pull request Jun 12, 2025
…143714)

This depth limits a linear search (rather than the usual potentially
exponential one) and is not particularly important for compile-time in
practice.

The change in #137297 is going to increase the length of GEP chains, so
I'd like to increase this limit a bit to reduce the chance of
regressions (dtcxzyw/llvm-opt-benchmark#2419
showed a 13% increase in SearchLimitReached). There is no particular
significance to the new value of 10.

Compile-time is neutral.
tomtor pushed a commit to tomtor/llvm-project that referenced this pull request Jun 14, 2025
…142958)

Currently OptimizePointerDifference() only handles single GEPs with a
common base, not GEP chains. This patch generalizes the support to
nested GEPs with a common base.

Finding the common base is a bit annoying because we want to stop as
soon as possible and not recurse into common GEP prefixes.

This helps avoids regressions from
llvm#137297.
tomtor pushed a commit to tomtor/llvm-project that referenced this pull request Jun 14, 2025
…lvm#143714)

This depth limits a linear search (rather than the usual potentially
exponential one) and is not particularly important for compile-time in
practice.

The change in llvm#137297 is going to increase the length of GEP chains, so
I'd like to increase this limit a bit to reduce the chance of
regressions (dtcxzyw/llvm-opt-benchmark#2419
showed a 13% increase in SearchLimitReached). There is no particular
significance to the new value of 10.

Compile-time is neutral.
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.

1 participant