[InstCombine] Simplify nonnull phi nodes

[dtcxzyw]

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

[llvmbot]

@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
+}

[nikic]

Can you please land this separately, with a test for nested GEP?

It might make sense to directly go to MaxAnalysisRecursionDepth, which will also help inform how other code is written. In particular, if the depth is increased, then your phi handling as currently implemented would become very expensive.

[dtcxzyw]

It would be better to use RecursionLimit = 3. To avoid expensive recursion in phi handling, we can pass Depth = RecursionLimit instead.

[nikic]

Sounds reasonable