Skip to content

[InstCombine] Simplify nonnull phi nodes #128466

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
Mar 1, 2025

Conversation

dtcxzyw
Copy link
Member

@dtcxzyw dtcxzyw commented Feb 24, 2025

@dtcxzyw dtcxzyw requested a review from goldsteinn February 24, 2025 07:28
@dtcxzyw dtcxzyw marked this pull request as ready for review February 24, 2025 07:28
@dtcxzyw dtcxzyw requested a review from nikic as a code owner February 24, 2025 07:28
@llvmbot llvmbot added llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:transforms labels Feb 24, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 24, 2025

@llvm/pr-subscribers-llvm-transforms

Author: Yingwei Zheng (dtcxzyw)

Changes

Fix some regressions caused by #128111.
Compile-time impact: https://llvm-compile-time-tracker.com/compare.php?from=353cb5334842a6969f9958f1548110a33c143de8&to=c213cfd89a4a4ead8516351e136a4c572fd4de28&stat=instructions:u


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

2 Files Affected:

  • (modified) llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp (+15-1)
  • (modified) llvm/test/Transforms/InstCombine/load.ll (+27)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
index 622884ea1eb46..e7dd0858ac687 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
@@ -996,7 +996,7 @@ Value *InstCombinerImpl::simplifyNonNullOperand(Value *V,
   if (!V->hasOneUse())
     return nullptr;
 
-  if (Depth == 1)
+  if (Depth == 2)
     return nullptr;
 
   if (auto *GEP = dyn_cast<GetElementPtrInst>(V)) {
@@ -1010,6 +1010,20 @@ Value *InstCombinerImpl::simplifyNonNullOperand(Value *V,
     }
   }
 
+  if (auto *PHI = dyn_cast<PHINode>(V)) {
+    bool Changed = false;
+    for (Use &U : PHI->incoming_values()) {
+      if (auto *Res =
+              simplifyNonNullOperand(U.get(), HasDereferenceable, Depth + 1)) {
+        replaceUse(U, Res);
+        Changed = true;
+      }
+    }
+    if (Changed)
+      addToWorklist(PHI);
+    return nullptr;
+  }
+
   return nullptr;
 }
 
diff --git a/llvm/test/Transforms/InstCombine/load.ll b/llvm/test/Transforms/InstCombine/load.ll
index a5ad1e0c21526..891e3a031b711 100644
--- a/llvm/test/Transforms/InstCombine/load.ll
+++ b/llvm/test/Transforms/InstCombine/load.ll
@@ -451,3 +451,30 @@ define i32 @load_select_with_null_gep(i1 %cond, ptr %p, i64 %off) {
   %res = load i32, ptr %gep, align 4
   ret i32 %res
 }
+
+define i32 @test_load_phi_with_select(ptr %p, i1 %cond1) {
+; CHECK-LABEL: @test_load_phi_with_select(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    br label [[LOOP_BODY:%.*]]
+; CHECK:       loop.body:
+; CHECK-NEXT:    [[TARGET:%.*]] = getelementptr inbounds nuw i8, ptr [[BASE:%.*]], i64 24
+; CHECK-NEXT:    [[LOAD:%.*]] = load i32, ptr [[TARGET]], align 4
+; CHECK-NEXT:    [[COND21:%.*]] = icmp eq i32 [[LOAD]], 0
+; CHECK-NEXT:    br i1 [[COND21]], label [[LOOP_BODY]], label [[EXIT:%.*]]
+; CHECK:       exit:
+; CHECK-NEXT:    ret i32 [[LOAD]]
+;
+entry:
+  br label %loop.body
+
+loop.body:
+  %base = phi ptr [ %p, %entry ], [ %sel, %loop.body ]
+  %target = getelementptr inbounds i8, ptr %base, i64 24
+  %load = load i32, ptr %target, align 4
+  %sel = select i1 %cond1, ptr null, ptr %p
+  %cond2 = icmp eq i32 %load, 0
+  br i1 %cond2, label %loop.body, label %exit
+
+exit:
+  ret i32 %load
+}

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 37374fb into llvm:main Mar 1, 2025
11 checks passed
@dtcxzyw dtcxzyw deleted the perf/simplify-nonnull-phi branch March 1, 2025 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:transforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants