Skip to content

release/20.x: [InstSimplify] Add additional checks when substituting pointers (#125385) #125398

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 1 commit into from
Feb 11, 2025

Conversation

llvmbot
Copy link
Member

@llvmbot llvmbot commented Feb 2, 2025

Backport 1af627b

Requested by: @dtcxzyw

@llvmbot llvmbot requested a review from nikic as a code owner February 2, 2025 11:10
@llvmbot llvmbot added this to the LLVM 20.X Release milestone Feb 2, 2025
@llvmbot
Copy link
Member Author

llvmbot commented Feb 2, 2025

@nikic What do you think about merging this PR to the release branch?

@llvmbot
Copy link
Member Author

llvmbot commented Feb 2, 2025

@llvm/pr-subscribers-llvm-transforms

Author: None (llvmbot)

Changes

Backport 1af627b

Requested by: @dtcxzyw


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

2 Files Affected:

  • (modified) llvm/lib/Analysis/InstructionSimplify.cpp (+11-6)
  • (modified) llvm/test/Transforms/InstSimplify/select-icmp.ll (+32)
diff --git a/llvm/lib/Analysis/InstructionSimplify.cpp b/llvm/lib/Analysis/InstructionSimplify.cpp
index d69747e30f884d7..4e550f8fa6a9fd1 100644
--- a/llvm/lib/Analysis/InstructionSimplify.cpp
+++ b/llvm/lib/Analysis/InstructionSimplify.cpp
@@ -27,6 +27,7 @@
 #include "llvm/Analysis/CmpInstAnalysis.h"
 #include "llvm/Analysis/ConstantFolding.h"
 #include "llvm/Analysis/InstSimplifyFolder.h"
+#include "llvm/Analysis/Loads.h"
 #include "llvm/Analysis/LoopAnalysisManager.h"
 #include "llvm/Analysis/MemoryBuiltins.h"
 #include "llvm/Analysis/OverflowInstAnalysis.h"
@@ -4737,12 +4738,16 @@ static Value *simplifySelectWithICmpCond(Value *CondVal, Value *TrueVal,
   // the arms of the select. See if substituting this value into the arm and
   // simplifying the result yields the same value as the other arm.
   if (Pred == ICmpInst::ICMP_EQ) {
-    if (Value *V = simplifySelectWithEquivalence({{CmpLHS, CmpRHS}}, TrueVal,
-                                                 FalseVal, Q, MaxRecurse))
-      return V;
-    if (Value *V = simplifySelectWithEquivalence({{CmpRHS, CmpLHS}}, TrueVal,
-                                                 FalseVal, Q, MaxRecurse))
-      return V;
+    if (CmpLHS->getType()->isIntOrIntVectorTy() ||
+        canReplacePointersIfEqual(CmpLHS, CmpRHS, Q.DL))
+      if (Value *V = simplifySelectWithEquivalence({{CmpLHS, CmpRHS}}, TrueVal,
+                                                   FalseVal, Q, MaxRecurse))
+        return V;
+    if (CmpLHS->getType()->isIntOrIntVectorTy() ||
+        canReplacePointersIfEqual(CmpRHS, CmpLHS, Q.DL))
+      if (Value *V = simplifySelectWithEquivalence({{CmpRHS, CmpLHS}}, TrueVal,
+                                                   FalseVal, Q, MaxRecurse))
+        return V;
 
     Value *X;
     Value *Y;
diff --git a/llvm/test/Transforms/InstSimplify/select-icmp.ll b/llvm/test/Transforms/InstSimplify/select-icmp.ll
index a6ef937760a5899..64c0d1d7553feb9 100755
--- a/llvm/test/Transforms/InstSimplify/select-icmp.ll
+++ b/llvm/test/Transforms/InstSimplify/select-icmp.ll
@@ -244,3 +244,35 @@ cond.true:                                        ; preds = %entry
 cond.end:                                         ; preds = %entry, %cond.true
   ret i8 0
 }
+
+define ptr @icmp_ptr_eq_replace(ptr %a, ptr %b) {
+; CHECK-LABEL: @icmp_ptr_eq_replace(
+; CHECK-NEXT:    [[CMP:%.*]] = icmp eq ptr [[A:%.*]], [[B1:%.*]]
+; CHECK-NEXT:    [[B:%.*]] = select i1 [[CMP]], ptr [[A]], ptr [[B1]]
+; CHECK-NEXT:    ret ptr [[B]]
+;
+  %cmp = icmp eq ptr %a, %b
+  %sel = select i1 %cmp, ptr %a, ptr %b
+  ret ptr %sel
+}
+
+define ptr @icmp_ptr_eq_replace_null(ptr %a) {
+; CHECK-LABEL: @icmp_ptr_eq_replace_null(
+; CHECK-NEXT:    ret ptr [[A:%.*]]
+;
+  %cmp = icmp eq ptr %a, null
+  %sel = select i1 %cmp, ptr null, ptr %a
+  ret ptr %sel
+}
+
+define ptr @ptr_eq_replace_same_underlying_object(ptr %st, i64 %i, i64 %j) {
+; CHECK-LABEL: @ptr_eq_replace_same_underlying_object(
+; CHECK-NEXT:    [[B:%.*]] = getelementptr inbounds i8, ptr [[ST:%.*]], i64 [[J:%.*]]
+; CHECK-NEXT:    ret ptr [[B]]
+;
+  %a = getelementptr inbounds i8, ptr %st, i64 %i
+  %b = getelementptr inbounds i8, ptr %st, i64 %j
+  %cmp = icmp eq ptr %a, %b
+  %sel = select i1 %cmp, ptr %a, ptr %b
+  ret ptr %sel
+}

@llvmbot
Copy link
Member Author

llvmbot commented Feb 2, 2025

@llvm/pr-subscribers-llvm-analysis

Author: None (llvmbot)

Changes

Backport 1af627b

Requested by: @dtcxzyw


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

2 Files Affected:

  • (modified) llvm/lib/Analysis/InstructionSimplify.cpp (+11-6)
  • (modified) llvm/test/Transforms/InstSimplify/select-icmp.ll (+32)
diff --git a/llvm/lib/Analysis/InstructionSimplify.cpp b/llvm/lib/Analysis/InstructionSimplify.cpp
index d69747e30f884d..4e550f8fa6a9fd 100644
--- a/llvm/lib/Analysis/InstructionSimplify.cpp
+++ b/llvm/lib/Analysis/InstructionSimplify.cpp
@@ -27,6 +27,7 @@
 #include "llvm/Analysis/CmpInstAnalysis.h"
 #include "llvm/Analysis/ConstantFolding.h"
 #include "llvm/Analysis/InstSimplifyFolder.h"
+#include "llvm/Analysis/Loads.h"
 #include "llvm/Analysis/LoopAnalysisManager.h"
 #include "llvm/Analysis/MemoryBuiltins.h"
 #include "llvm/Analysis/OverflowInstAnalysis.h"
@@ -4737,12 +4738,16 @@ static Value *simplifySelectWithICmpCond(Value *CondVal, Value *TrueVal,
   // the arms of the select. See if substituting this value into the arm and
   // simplifying the result yields the same value as the other arm.
   if (Pred == ICmpInst::ICMP_EQ) {
-    if (Value *V = simplifySelectWithEquivalence({{CmpLHS, CmpRHS}}, TrueVal,
-                                                 FalseVal, Q, MaxRecurse))
-      return V;
-    if (Value *V = simplifySelectWithEquivalence({{CmpRHS, CmpLHS}}, TrueVal,
-                                                 FalseVal, Q, MaxRecurse))
-      return V;
+    if (CmpLHS->getType()->isIntOrIntVectorTy() ||
+        canReplacePointersIfEqual(CmpLHS, CmpRHS, Q.DL))
+      if (Value *V = simplifySelectWithEquivalence({{CmpLHS, CmpRHS}}, TrueVal,
+                                                   FalseVal, Q, MaxRecurse))
+        return V;
+    if (CmpLHS->getType()->isIntOrIntVectorTy() ||
+        canReplacePointersIfEqual(CmpRHS, CmpLHS, Q.DL))
+      if (Value *V = simplifySelectWithEquivalence({{CmpRHS, CmpLHS}}, TrueVal,
+                                                   FalseVal, Q, MaxRecurse))
+        return V;
 
     Value *X;
     Value *Y;
diff --git a/llvm/test/Transforms/InstSimplify/select-icmp.ll b/llvm/test/Transforms/InstSimplify/select-icmp.ll
index a6ef937760a589..64c0d1d7553feb 100755
--- a/llvm/test/Transforms/InstSimplify/select-icmp.ll
+++ b/llvm/test/Transforms/InstSimplify/select-icmp.ll
@@ -244,3 +244,35 @@ cond.true:                                        ; preds = %entry
 cond.end:                                         ; preds = %entry, %cond.true
   ret i8 0
 }
+
+define ptr @icmp_ptr_eq_replace(ptr %a, ptr %b) {
+; CHECK-LABEL: @icmp_ptr_eq_replace(
+; CHECK-NEXT:    [[CMP:%.*]] = icmp eq ptr [[A:%.*]], [[B1:%.*]]
+; CHECK-NEXT:    [[B:%.*]] = select i1 [[CMP]], ptr [[A]], ptr [[B1]]
+; CHECK-NEXT:    ret ptr [[B]]
+;
+  %cmp = icmp eq ptr %a, %b
+  %sel = select i1 %cmp, ptr %a, ptr %b
+  ret ptr %sel
+}
+
+define ptr @icmp_ptr_eq_replace_null(ptr %a) {
+; CHECK-LABEL: @icmp_ptr_eq_replace_null(
+; CHECK-NEXT:    ret ptr [[A:%.*]]
+;
+  %cmp = icmp eq ptr %a, null
+  %sel = select i1 %cmp, ptr null, ptr %a
+  ret ptr %sel
+}
+
+define ptr @ptr_eq_replace_same_underlying_object(ptr %st, i64 %i, i64 %j) {
+; CHECK-LABEL: @ptr_eq_replace_same_underlying_object(
+; CHECK-NEXT:    [[B:%.*]] = getelementptr inbounds i8, ptr [[ST:%.*]], i64 [[J:%.*]]
+; CHECK-NEXT:    ret ptr [[B]]
+;
+  %a = getelementptr inbounds i8, ptr %st, i64 %i
+  %b = getelementptr inbounds i8, ptr %st, i64 %j
+  %cmp = icmp eq ptr %a, %b
+  %sel = select i1 %cmp, ptr %a, ptr %b
+  ret ptr %sel
+}

@tstellar
Copy link
Collaborator

The test failure seems unrelated. I'm going to re-run the tests.

@tstellar tstellar merged commit 56b4c11 into llvm:release/20.x Feb 11, 2025
7 of 10 checks passed
Copy link

@dtcxzyw (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

4 participants