Skip to content

[InstCombine] Allow Ephemerals when trying to remove assume. #128296

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

Closed
wants to merge 1 commit into from

Conversation

andjo403
Copy link
Contributor

Makes it possible to remove loads of nonnull pointers when the only use is an assume.

@llvmbot
Copy link
Member

llvmbot commented Feb 22, 2025

@llvm/pr-subscribers-llvm-transforms

Author: Andreas Jonson (andjo403)

Changes

Makes it possible to remove loads of nonnull pointers when the only use is an assume.


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

2 Files Affected:

  • (modified) llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp (+2-1)
  • (modified) llvm/test/Transforms/InstCombine/assume.ll (+5-14)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
index 54f777ab20a7a..405b0c2d8d972 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
@@ -3217,7 +3217,8 @@ Instruction *InstCombinerImpl::visitCallInst(CallInst &CI) {
                                         m_Zero())) &&
         LHS->getOpcode() == Instruction::Load &&
         LHS->getType()->isPointerTy() &&
-        isValidAssumeForContext(II, LHS, &DT)) {
+        isValidAssumeForContext(II, LHS, &DT,
+                                /*AllowEphemerals=*/true)) {
       MDNode *MD = MDNode::get(II->getContext(), {});
       LHS->setMetadata(LLVMContext::MD_nonnull, MD);
       LHS->setMetadata(LLVMContext::MD_noundef, MD);
diff --git a/llvm/test/Transforms/InstCombine/assume.ll b/llvm/test/Transforms/InstCombine/assume.ll
index 90fa9e680bb1e..5bd8fbc823756 100644
--- a/llvm/test/Transforms/InstCombine/assume.ll
+++ b/llvm/test/Transforms/InstCombine/assume.ll
@@ -502,10 +502,9 @@ define i1 @nonnull3B(ptr %a, i1 %control) {
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    br i1 [[CONTROL:%.*]], label [[TAKEN:%.*]], label [[NOT_TAKEN:%.*]]
 ; CHECK:       taken:
-; CHECK-NEXT:    [[LOAD:%.*]] = load ptr, ptr [[A:%.*]], align 8
-; CHECK-NEXT:    [[CMP:%.*]] = icmp ne ptr [[LOAD]], null
-; CHECK-NEXT:    call void @llvm.assume(i1 [[CMP]]) [ "nonnull"(ptr [[LOAD]]) ]
-; CHECK-NEXT:    ret i1 [[CMP]]
+; CHECK-NEXT:    [[LOAD:%.*]] = load ptr, ptr [[A:%.*]], align 8, !nonnull [[META6]], !noundef [[META6]]
+; CHECK-NEXT:    call void @llvm.assume(i1 true) [ "nonnull"(ptr [[LOAD]]) ]
+; CHECK-NEXT:    ret i1 true
 ; CHECK:       not_taken:
 ; CHECK-NEXT:    ret i1 false
 ;
@@ -582,16 +581,8 @@ not_taken:
 }
 
 define void @nonnull_only_ephemeral_use(ptr %p) {
-; DEFAULT-LABEL: @nonnull_only_ephemeral_use(
-; DEFAULT-NEXT:    [[A:%.*]] = load ptr, ptr [[P:%.*]], align 8
-; DEFAULT-NEXT:    [[CMP:%.*]] = icmp ne ptr [[A]], null
-; DEFAULT-NEXT:    tail call void @llvm.assume(i1 [[CMP]])
-; DEFAULT-NEXT:    ret void
-;
-; BUNDLES-LABEL: @nonnull_only_ephemeral_use(
-; BUNDLES-NEXT:    [[A:%.*]] = load ptr, ptr [[P:%.*]], align 8
-; BUNDLES-NEXT:    call void @llvm.assume(i1 true) [ "nonnull"(ptr [[A]]) ]
-; BUNDLES-NEXT:    ret void
+; CHECK-LABEL: @nonnull_only_ephemeral_use(
+; CHECK-NEXT:    ret void
 ;
   %a = load ptr, ptr %p
   %cmp = icmp ne ptr %a, null

@andjo403
Copy link
Contributor Author

hmm is it strange to remove the condition in the assume in this fold as it can result in that all the information about the assume is removed if the load is not needed.

@andjo403
Copy link
Contributor Author

andjo403 commented Mar 2, 2025

from what I found here dtcxzyw/llvm-opt-benchmark#2154 (comment) I do not think that this the the way to go.

@andjo403 andjo403 closed this Mar 2, 2025
@andjo403 andjo403 deleted the ephemeralNonnull branch March 2, 2025 12:46
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.

2 participants