Skip to content

[InstCombine] Increase recursion limit to 3 in simplifyNonNullOperand #128695

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

Merged
merged 3 commits into from
Feb 25, 2025

Conversation

dtcxzyw
Copy link
Member

@dtcxzyw dtcxzyw commented Feb 25, 2025

@llvmbot
Copy link
Member

llvmbot commented Feb 25, 2025

@llvm/pr-subscribers-llvm-transforms

Author: Yingwei Zheng (dtcxzyw)

Changes

Address review comment #128466 (comment)


Full diff: https://github.com/llvm/llvm-project/pull/128695.diff

2 Files Affected:

  • (modified) llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp (+2-1)
  • (modified) llvm/test/Transforms/InstCombine/load.ll (+30)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
index 622884ea1eb46..221511e9aba0b 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
@@ -996,7 +996,8 @@ Value *InstCombinerImpl::simplifyNonNullOperand(Value *V,
   if (!V->hasOneUse())
     return nullptr;
 
-  if (Depth == 1)
+  constexpr unsigned RecursionLimit = 3;
+  if (Depth == RecursionLimit)
     return nullptr;
 
   if (auto *GEP = dyn_cast<GetElementPtrInst>(V)) {
diff --git a/llvm/test/Transforms/InstCombine/load.ll b/llvm/test/Transforms/InstCombine/load.ll
index a5ad1e0c21526..99185a4e6f043 100644
--- a/llvm/test/Transforms/InstCombine/load.ll
+++ b/llvm/test/Transforms/InstCombine/load.ll
@@ -451,3 +451,33 @@ define i32 @load_select_with_null_gep(i1 %cond, ptr %p, i64 %off) {
   %res = load i32, ptr %gep, align 4
   ret i32 %res
 }
+
+define i16 @load_select_with_null_gep2(i1 %cond, ptr %p, i64 %x) {
+; CHECK-LABEL: @load_select_with_null_gep2(
+; CHECK-NEXT:    [[INVARIANT_GEP:%.*]] = getelementptr i8, ptr [[SEL:%.*]], i64 -2
+; CHECK-NEXT:    [[GEP:%.*]] = getelementptr i16, ptr [[INVARIANT_GEP]], i64 [[X:%.*]]
+; CHECK-NEXT:    [[RES:%.*]] = load i16, ptr [[GEP]], align 2
+; CHECK-NEXT:    ret i16 [[RES]]
+;
+  %sel = select i1 %cond, ptr %p, ptr null
+  %invariant.gep = getelementptr i8, ptr %sel, i64 -2
+  %gep = getelementptr i16, ptr %invariant.gep, i64 %x
+  %res = load i16, ptr %gep, align 2
+  ret i16 %res
+}
+
+define i16 @load_select_with_null_ge3(i1 %cond, ptr %p, i64 %x, i64 %y) {
+; CHECK-LABEL: @load_select_with_null_ge3(
+; CHECK-NEXT:    [[INVARIANT_GEP:%.*]] = getelementptr i8, ptr [[SEL:%.*]], i64 -2
+; CHECK-NEXT:    [[GEP:%.*]] = getelementptr i16, ptr [[INVARIANT_GEP]], i64 [[X:%.*]]
+; CHECK-NEXT:    [[GEP2:%.*]] = getelementptr i16, ptr [[GEP]], i64 [[Y:%.*]]
+; CHECK-NEXT:    [[RES:%.*]] = load i16, ptr [[GEP2]], align 2
+; CHECK-NEXT:    ret i16 [[RES]]
+;
+  %sel = select i1 %cond, ptr %p, ptr null
+  %invariant.gep = getelementptr i8, ptr %sel, i64 -2
+  %gep = getelementptr i16, ptr %invariant.gep, i64 %x
+  %gep2 = getelementptr i16, ptr %gep, i64 %y
+  %res = load i16, ptr %gep2, align 2
+  ret i16 %res
+}

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

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

LGTM

@dtcxzyw dtcxzyw merged commit d23da7d into llvm:main Feb 25, 2025
9 of 10 checks passed
@dtcxzyw dtcxzyw deleted the perf/simplify-nonnull-depth branch February 25, 2025 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants