[InstCombine] Always fold nonnull assumptions into operand bundles#169923
[InstCombine] Always fold nonnull assumptions into operand bundles#169923philnik777 merged 1 commit intollvm:mainfrom
Conversation
|
@llvm/pr-subscribers-llvm-transforms Author: Nikolas Klauser (philnik777) ChangesAttribute bundles seem to be superior compared to normal assumes in (almost) all cases, so I don't see why folding normal assumes to operand bundles should be behind a flag. Fixes #168688 Full diff: https://github.com/llvm/llvm-project/pull/169923.diff 6 Files Affected:
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
index 743c4f574e131..42d357f372f8a 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
@@ -3478,8 +3478,7 @@ Instruction *InstCombinerImpl::visitCallInst(CallInst &CI) {
// call void @llvm.assume(i1 %A)
// into
// call void @llvm.assume(i1 true) [ "nonnull"(i32* %PTR) ]
- if (EnableKnowledgeRetention &&
- match(IIOperand,
+ if (match(IIOperand,
m_SpecificICmp(ICmpInst::ICMP_NE, m_Value(A), m_Zero())) &&
A->getType()->isPointerTy()) {
if (auto *Replacement = buildAssumeFromKnowledge(
@@ -3499,8 +3498,7 @@ Instruction *InstCombinerImpl::visitCallInst(CallInst &CI) {
// into
// call void @llvm.assume(i1 true) [ "align"(i32* [[A]], i64 Constant + 1)]
uint64_t AlignMask = 1;
- if (EnableKnowledgeRetention &&
- (match(IIOperand, m_Not(m_Trunc(m_Value(A)))) ||
+ if ((match(IIOperand, m_Not(m_Trunc(m_Value(A)))) ||
match(IIOperand,
m_SpecificICmp(ICmpInst::ICMP_EQ,
m_And(m_Value(A), m_ConstantInt(AlignMask)),
diff --git a/llvm/test/Transforms/InstCombine/assume-icmp-null-select.ll b/llvm/test/Transforms/InstCombine/assume-icmp-null-select.ll
index 108ee8cf80755..38475816f17f7 100644
--- a/llvm/test/Transforms/InstCombine/assume-icmp-null-select.ll
+++ b/llvm/test/Transforms/InstCombine/assume-icmp-null-select.ll
@@ -24,8 +24,7 @@ define ptr @example2(ptr %x) {
; CHECK-NEXT: [[Y:%.*]] = load ptr, ptr [[X:%.*]], align 8
; CHECK-NEXT: [[Y_IS_NULL:%.*]] = icmp eq ptr [[Y]], null
; CHECK-NEXT: [[RES:%.*]] = select i1 [[Y_IS_NULL]], ptr null, ptr [[X]]
-; CHECK-NEXT: [[NONNULL:%.*]] = icmp ne ptr [[RES]], null
-; CHECK-NEXT: call void @llvm.assume(i1 [[NONNULL]])
+; CHECK-NEXT: call void @llvm.assume(i1 true) [ "nonnull"(ptr [[RES]]) ]
; CHECK-NEXT: ret ptr [[RES]]
;
%y = load ptr, ptr %x, align 8
diff --git a/llvm/test/Transforms/InstCombine/assume-loop-align.ll b/llvm/test/Transforms/InstCombine/assume-loop-align.ll
index 24fd343d1448e..f6805ba4894f0 100644
--- a/llvm/test/Transforms/InstCombine/assume-loop-align.ll
+++ b/llvm/test/Transforms/InstCombine/assume-loop-align.ll
@@ -10,14 +10,8 @@ target triple = "x86_64-unknown-linux-gnu"
define void @foo(ptr %a, ptr %b) #0 {
; CHECK-LABEL: @foo(
; CHECK-NEXT: entry:
-; CHECK-NEXT: [[PTRINT:%.*]] = ptrtoint ptr [[A:%.*]] to i64
-; CHECK-NEXT: [[MASKEDPTR:%.*]] = and i64 [[PTRINT]], 63
-; CHECK-NEXT: [[MASKCOND:%.*]] = icmp eq i64 [[MASKEDPTR]], 0
-; CHECK-NEXT: tail call void @llvm.assume(i1 [[MASKCOND]])
-; CHECK-NEXT: [[PTRINT1:%.*]] = ptrtoint ptr [[B:%.*]] to i64
-; CHECK-NEXT: [[MASKEDPTR2:%.*]] = and i64 [[PTRINT1]], 63
-; CHECK-NEXT: [[MASKCOND3:%.*]] = icmp eq i64 [[MASKEDPTR2]], 0
-; CHECK-NEXT: tail call void @llvm.assume(i1 [[MASKCOND3]])
+; CHECK-NEXT: call void @llvm.assume(i1 true) [ "align"(ptr [[A:%.*]], i64 64) ]
+; CHECK-NEXT: call void @llvm.assume(i1 true) [ "align"(ptr [[B:%.*]], i64 64) ]
; CHECK-NEXT: br label [[FOR_BODY:%.*]]
; CHECK: for.body:
; CHECK-NEXT: [[INDVARS_IV:%.*]] = phi i64 [ 0, [[ENTRY:%.*]] ], [ [[INDVARS_IV_NEXT:%.*]], [[FOR_BODY]] ]
@@ -66,4 +60,3 @@ declare void @llvm.assume(i1) #1
attributes #0 = { nounwind uwtable }
attributes #1 = { nounwind }
-
diff --git a/llvm/test/Transforms/InstCombine/assume.ll b/llvm/test/Transforms/InstCombine/assume.ll
index cc87d6542fa12..702f85e819b77 100644
--- a/llvm/test/Transforms/InstCombine/assume.ll
+++ b/llvm/test/Transforms/InstCombine/assume.ll
@@ -11,18 +11,10 @@ declare void @llvm.assume(i1) #1
; Check that the assume has not been removed:
define i32 @align_to_bundle(ptr %a) #0 {
-; DEFAULT-LABEL: @align_to_bundle(
-; DEFAULT-NEXT: [[T0:%.*]] = load i32, ptr [[A:%.*]], align 4
-; DEFAULT-NEXT: [[PTRINT:%.*]] = ptrtoint ptr [[A]] to i64
-; DEFAULT-NEXT: [[MASKEDPTR:%.*]] = and i64 [[PTRINT]], 31
-; DEFAULT-NEXT: [[MASKCOND:%.*]] = icmp eq i64 [[MASKEDPTR]], 0
-; DEFAULT-NEXT: tail call void @llvm.assume(i1 [[MASKCOND]])
-; DEFAULT-NEXT: ret i32 [[T0]]
-;
-; BUNDLES-LABEL: @align_to_bundle(
-; BUNDLES-NEXT: [[T0:%.*]] = load i32, ptr [[A:%.*]], align 4
-; BUNDLES-NEXT: call void @llvm.assume(i1 true) [ "align"(ptr [[A]], i64 32) ]
-; BUNDLES-NEXT: ret i32 [[T0]]
+; CHECK-LABEL: @align_to_bundle(
+; CHECK-NEXT: [[T0:%.*]] = load i32, ptr [[A:%.*]], align 4
+; CHECK-NEXT: call void @llvm.assume(i1 true) [ "align"(ptr [[A]], i64 32) ]
+; CHECK-NEXT: ret i32 [[T0]]
;
%t0 = load i32, ptr %a, align 4
%ptrint = ptrtoint ptr %a to i64
@@ -33,18 +25,10 @@ define i32 @align_to_bundle(ptr %a) #0 {
}
define i32 @align_to_bundle_ptrtoaddr(ptr %a) #0 {
-; DEFAULT-LABEL: @align_to_bundle_ptrtoaddr(
-; DEFAULT-NEXT: [[T0:%.*]] = load i32, ptr [[A:%.*]], align 4
-; DEFAULT-NEXT: [[PTRINT:%.*]] = ptrtoaddr ptr [[A]] to i64
-; DEFAULT-NEXT: [[MASKEDPTR:%.*]] = and i64 [[PTRINT]], 31
-; DEFAULT-NEXT: [[MASKCOND:%.*]] = icmp eq i64 [[MASKEDPTR]], 0
-; DEFAULT-NEXT: tail call void @llvm.assume(i1 [[MASKCOND]])
-; DEFAULT-NEXT: ret i32 [[T0]]
-;
-; BUNDLES-LABEL: @align_to_bundle_ptrtoaddr(
-; BUNDLES-NEXT: [[T0:%.*]] = load i32, ptr [[A:%.*]], align 4
-; BUNDLES-NEXT: call void @llvm.assume(i1 true) [ "align"(ptr [[A]], i64 32) ]
-; BUNDLES-NEXT: ret i32 [[T0]]
+; CHECK-LABEL: @align_to_bundle_ptrtoaddr(
+; CHECK-NEXT: [[T0:%.*]] = load i32, ptr [[A:%.*]], align 4
+; CHECK-NEXT: call void @llvm.assume(i1 true) [ "align"(ptr [[A]], i64 32) ]
+; CHECK-NEXT: ret i32 [[T0]]
;
%t0 = load i32, ptr %a, align 4
%ptrint = ptrtoaddr ptr %a to i64
@@ -55,18 +39,10 @@ define i32 @align_to_bundle_ptrtoaddr(ptr %a) #0 {
}
define i32 @align_assume_trunc_cond(ptr %a) #0 {
-; DEFAULT-LABEL: @align_assume_trunc_cond(
-; DEFAULT-NEXT: [[T0:%.*]] = load i32, ptr [[A:%.*]], align 4
-; DEFAULT-NEXT: [[PTRINT:%.*]] = ptrtoint ptr [[A]] to i64
-; DEFAULT-NEXT: [[TRUNC:%.*]] = trunc i64 [[PTRINT]] to i1
-; DEFAULT-NEXT: [[MASKCOND:%.*]] = xor i1 [[TRUNC]], true
-; DEFAULT-NEXT: tail call void @llvm.assume(i1 [[MASKCOND]])
-; DEFAULT-NEXT: ret i32 [[T0]]
-;
-; BUNDLES-LABEL: @align_assume_trunc_cond(
-; BUNDLES-NEXT: [[T0:%.*]] = load i32, ptr [[A:%.*]], align 4
-; BUNDLES-NEXT: call void @llvm.assume(i1 true) [ "align"(ptr [[A]], i64 2) ]
-; BUNDLES-NEXT: ret i32 [[T0]]
+; CHECK-LABEL: @align_assume_trunc_cond(
+; CHECK-NEXT: [[T0:%.*]] = load i32, ptr [[A:%.*]], align 4
+; CHECK-NEXT: call void @llvm.assume(i1 true) [ "align"(ptr [[A]], i64 2) ]
+; CHECK-NEXT: ret i32 [[T0]]
;
%t0 = load i32, ptr %a, align 4
%ptrint = ptrtoint ptr %a to i64
@@ -79,18 +55,10 @@ define i32 @align_assume_trunc_cond(ptr %a) #0 {
; Same check as in @foo1, but make sure it works if the assume is first too.
define i32 @foo2(ptr %a) #0 {
-; DEFAULT-LABEL: @foo2(
-; DEFAULT-NEXT: [[PTRINT:%.*]] = ptrtoint ptr [[A:%.*]] to i64
-; DEFAULT-NEXT: [[MASKEDPTR:%.*]] = and i64 [[PTRINT]], 31
-; DEFAULT-NEXT: [[MASKCOND:%.*]] = icmp eq i64 [[MASKEDPTR]], 0
-; DEFAULT-NEXT: tail call void @llvm.assume(i1 [[MASKCOND]])
-; DEFAULT-NEXT: [[T0:%.*]] = load i32, ptr [[A]], align 4
-; DEFAULT-NEXT: ret i32 [[T0]]
-;
-; BUNDLES-LABEL: @foo2(
-; BUNDLES-NEXT: call void @llvm.assume(i1 true) [ "align"(ptr [[A:%.*]], i64 32) ]
-; BUNDLES-NEXT: [[T0:%.*]] = load i32, ptr [[A]], align 4
-; BUNDLES-NEXT: ret i32 [[T0]]
+; CHECK-LABEL: @foo2(
+; CHECK-NEXT: call void @llvm.assume(i1 true) [ "align"(ptr [[A:%.*]], i64 32) ]
+; CHECK-NEXT: [[T0:%.*]] = load i32, ptr [[A]], align 4
+; CHECK-NEXT: ret i32 [[T0]]
;
%ptrint = ptrtoint ptr %a to i64
%maskedptr = and i64 %ptrint, 31
@@ -343,28 +311,16 @@ define i1 @nonnull2(ptr %a) {
; if the assume is control dependent on something else
define i1 @nonnull3(ptr %a, i1 %control) {
-; FIXME: in the BUNDLES version we could duplicate the load and keep the assume nonnull.
-; DEFAULT-LABEL: @nonnull3(
-; DEFAULT-NEXT: entry:
-; DEFAULT-NEXT: [[LOAD:%.*]] = load ptr, ptr [[A:%.*]], align 8
-; DEFAULT-NEXT: [[CMP:%.*]] = icmp ne ptr [[LOAD]], null
-; DEFAULT-NEXT: br i1 [[CONTROL:%.*]], label [[TAKEN:%.*]], label [[NOT_TAKEN:%.*]]
-; DEFAULT: taken:
-; DEFAULT-NEXT: tail call void @llvm.assume(i1 [[CMP]])
-; DEFAULT-NEXT: ret i1 false
-; DEFAULT: not_taken:
-; DEFAULT-NEXT: [[RVAL_2:%.*]] = icmp sgt ptr [[LOAD]], null
-; DEFAULT-NEXT: ret i1 [[RVAL_2]]
-;
-; BUNDLES-LABEL: @nonnull3(
-; BUNDLES-NEXT: entry:
-; BUNDLES-NEXT: br i1 [[CONTROL:%.*]], label [[TAKEN:%.*]], label [[NOT_TAKEN:%.*]]
-; BUNDLES: taken:
-; BUNDLES-NEXT: ret i1 false
-; BUNDLES: not_taken:
-; BUNDLES-NEXT: [[LOAD:%.*]] = load ptr, ptr [[A:%.*]], align 8
-; BUNDLES-NEXT: [[RVAL_2:%.*]] = icmp sgt ptr [[LOAD]], null
-; BUNDLES-NEXT: ret i1 [[RVAL_2]]
+; FIXME: we could duplicate the load and keep the assume nonnull.
+; CHECK-LABEL: @nonnull3(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: br i1 [[CONTROL:%.*]], label [[TAKEN:%.*]], label [[NOT_TAKEN:%.*]]
+; CHECK: taken:
+; CHECK-NEXT: ret i1 false
+; CHECK: not_taken:
+; CHECK-NEXT: [[LOAD:%.*]] = load ptr, ptr [[A:%.*]], align 8
+; CHECK-NEXT: [[RVAL_2:%.*]] = icmp sgt ptr [[LOAD]], null
+; CHECK-NEXT: ret i1 [[RVAL_2]]
;
entry:
%load = load ptr, ptr %a
@@ -384,18 +340,11 @@ not_taken:
; interrupted by an exception being thrown
define i1 @nonnull4(ptr %a) {
-; DEFAULT-LABEL: @nonnull4(
-; DEFAULT-NEXT: [[LOAD:%.*]] = load ptr, ptr [[A:%.*]], align 8
-; DEFAULT-NEXT: tail call void @escape(ptr [[LOAD]])
-; DEFAULT-NEXT: [[CMP:%.*]] = icmp ne ptr [[LOAD]], null
-; DEFAULT-NEXT: tail call void @llvm.assume(i1 [[CMP]])
-; DEFAULT-NEXT: ret i1 false
-;
-; BUNDLES-LABEL: @nonnull4(
-; BUNDLES-NEXT: [[LOAD:%.*]] = load ptr, ptr [[A:%.*]], align 8
-; BUNDLES-NEXT: tail call void @escape(ptr [[LOAD]])
-; BUNDLES-NEXT: call void @llvm.assume(i1 true) [ "nonnull"(ptr [[LOAD]]) ]
-; BUNDLES-NEXT: ret i1 false
+; CHECK-LABEL: @nonnull4(
+; CHECK-NEXT: [[LOAD:%.*]] = load ptr, ptr [[A:%.*]], align 8
+; CHECK-NEXT: tail call void @escape(ptr [[LOAD]])
+; CHECK-NEXT: call void @llvm.assume(i1 true) [ "nonnull"(ptr [[LOAD]]) ]
+; CHECK-NEXT: ret i1 false
;
%load = load ptr, ptr %a
;; This call may throw!
@@ -482,27 +431,15 @@ define i32 @PR40940(<4 x i8> %x) {
}
define i1 @nonnull3A(ptr %a, i1 %control) {
-; DEFAULT-LABEL: @nonnull3A(
-; DEFAULT-NEXT: entry:
-; DEFAULT-NEXT: [[LOAD:%.*]] = load ptr, ptr [[A:%.*]], align 8
-; DEFAULT-NEXT: br i1 [[CONTROL:%.*]], label [[TAKEN:%.*]], label [[NOT_TAKEN:%.*]]
-; DEFAULT: taken:
-; DEFAULT-NEXT: [[CMP:%.*]] = icmp ne ptr [[LOAD]], null
-; DEFAULT-NEXT: call void @llvm.assume(i1 [[CMP]])
-; DEFAULT-NEXT: ret i1 [[CMP]]
-; DEFAULT: not_taken:
-; DEFAULT-NEXT: [[RVAL_2:%.*]] = icmp sgt ptr [[LOAD]], null
-; DEFAULT-NEXT: ret i1 [[RVAL_2]]
-;
-; BUNDLES-LABEL: @nonnull3A(
-; BUNDLES-NEXT: entry:
-; BUNDLES-NEXT: br i1 [[CONTROL:%.*]], label [[TAKEN:%.*]], label [[NOT_TAKEN:%.*]]
-; BUNDLES: taken:
-; BUNDLES-NEXT: ret i1 true
-; BUNDLES: not_taken:
-; BUNDLES-NEXT: [[LOAD:%.*]] = load ptr, ptr [[A:%.*]], align 8
-; BUNDLES-NEXT: [[RVAL_2:%.*]] = icmp sgt ptr [[LOAD]], null
-; BUNDLES-NEXT: ret i1 [[RVAL_2]]
+; CHECK-LABEL: @nonnull3A(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: br i1 [[CONTROL:%.*]], label [[TAKEN:%.*]], label [[NOT_TAKEN:%.*]]
+; CHECK: taken:
+; CHECK-NEXT: ret i1 true
+; CHECK: not_taken:
+; CHECK-NEXT: [[LOAD:%.*]] = load ptr, ptr [[A:%.*]], align 8
+; CHECK-NEXT: [[RVAL_2:%.*]] = icmp sgt ptr [[LOAD]], null
+; CHECK-NEXT: ret i1 [[RVAL_2]]
;
entry:
%load = load ptr, ptr %a
@@ -602,16 +539,10 @@ 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: [[A:%.*]] = load ptr, ptr [[P:%.*]], align 8
+; CHECK-NEXT: call void @llvm.assume(i1 true) [ "nonnull"(ptr [[A]]) ]
+; CHECK-NEXT: ret void
;
%a = load ptr, ptr %p
%cmp = icmp ne ptr %a, null
@@ -1075,4 +1006,3 @@ declare void @llvm.dbg.value(metadata, metadata, metadata)
attributes #0 = { nounwind uwtable }
attributes #1 = { nounwind }
-
diff --git a/llvm/test/Transforms/InstCombine/assume_inevitable.ll b/llvm/test/Transforms/InstCombine/assume_inevitable.ll
index 5f27ff1e609ba..f899be41e0584 100644
--- a/llvm/test/Transforms/InstCombine/assume_inevitable.ll
+++ b/llvm/test/Transforms/InstCombine/assume_inevitable.ll
@@ -16,10 +16,7 @@ define i32 @assume_inevitable(ptr %a, ptr %b, ptr %c) {
; CHECK-NEXT: [[M_A:%.*]] = call ptr @llvm.ptr.annotation.p0.p0(ptr nonnull [[M]], ptr nonnull @.str, ptr nonnull @.str1, i32 2, ptr null)
; CHECK-NEXT: [[OBJSZ:%.*]] = call i64 @llvm.objectsize.i64.p0(ptr [[C:%.*]], i1 false, i1 false, i1 false)
; CHECK-NEXT: store i64 [[OBJSZ]], ptr [[M_A]], align 4
-; CHECK-NEXT: [[PTRINT:%.*]] = ptrtoint ptr [[A]] to i64
-; CHECK-NEXT: [[MASKEDPTR:%.*]] = and i64 [[PTRINT]], 31
-; CHECK-NEXT: [[MASKCOND:%.*]] = icmp eq i64 [[MASKEDPTR]], 0
-; CHECK-NEXT: tail call void @llvm.assume(i1 [[MASKCOND]])
+; CHECK-NEXT: call void @llvm.assume(i1 true) [ "align"(ptr [[A]], i64 32) ]
; CHECK-NEXT: ret i32 [[TMP0]]
;
entry:
diff --git a/llvm/test/Transforms/PhaseOrdering/pr98799-inline-simplifycfg-ub.ll b/llvm/test/Transforms/PhaseOrdering/pr98799-inline-simplifycfg-ub.ll
index 5c4ada38bd302..793a2006330fc 100644
--- a/llvm/test/Transforms/PhaseOrdering/pr98799-inline-simplifycfg-ub.ll
+++ b/llvm/test/Transforms/PhaseOrdering/pr98799-inline-simplifycfg-ub.ll
@@ -34,9 +34,10 @@ define i32 @foo(ptr %arg, i1 %arg1) {
; O2-LABEL: define i32 @foo(
; O2-SAME: ptr captures(none) [[ARG:%.*]], i1 [[ARG1:%.*]]) local_unnamed_addr #[[ATTR0:[0-9]+]] {
; O2-NEXT: [[BB:.*:]]
-; O2-NEXT: [[I_I:%.*]] = load ptr, ptr [[ARG]], align 8, !nonnull [[META0:![0-9]+]], !noundef [[META0]]
+; O2-NEXT: [[I_I:%.*]] = load ptr, ptr [[ARG]], align 8
; O2-NEXT: [[I3_I:%.*]] = getelementptr inbounds nuw i8, ptr [[I_I]], i64 1
; O2-NEXT: store ptr [[I3_I]], ptr [[ARG]], align 8
+; O2-NEXT: call void @llvm.assume(i1 true) [ "nonnull"(ptr [[I_I]]) ]
; O2-NEXT: [[I3:%.*]] = load i32, ptr [[I_I]], align 4
; O2-NEXT: ret i32 [[I3]]
;
@@ -49,6 +50,3 @@ bb:
}
declare void @llvm.assume(i1)
-;.
-; O2: [[META0]] = !{}
-;.
|
dtcxzyw
left a comment
There was a problem hiding this comment.
I'd suggest making EnableKnowledgeRetention default to true and checking if the comptime impact and IR diff are acceptable. I guess some components don't support operand bundles yet, because there are very few users of getKnowledgeForValue/getKnowledgeFromBundle. BTW DropUnnecessaryAssumesPass might need to be tuned in different phases (See #166947).
|
@dtcxzyw I've looked into that a bit and that seems to be a rather big project (i.e. there seem to be quite a few places that need to be improved before we can generate assumptions this liberally). Do you think the rest of the knowledge retention mechanism is necessary for these transforms to be useful? |
|
I think the But Regarding EnableKnowledgeRetention, I don't think it's possible to enable it, it's way too aggressive and impact lots of things at the same time. |
6ca1004 to
6f23529
Compare
|
Gentle ping @nikic |
|
Crash reproducer for |
|
The nonnull handling in PredicateInfo is missing a shouldRename() check. |
|
It looks like we're currently also missing support for optimizing redundant nonnull assumptions. Should at least drop these based on isKnownNonZero in InstCombine. |
6f23529 to
f758b9c
Compare
nikic
left a comment
There was a problem hiding this comment.
We're missing the optimization that converts nonnull assumes to !nonnull metadata on loads: https://github.com/philnik777/llvm-project/blob/f758b9c1ffc8e96ae84f039b1d757cffda0ddb23/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp#L3607-L3622
fhahn
left a comment
There was a problem hiding this comment.
We're missing the optimization that converts nonnull assumes to
!nonnullmetadata on loads: https://github.com/philnik777/llvm-project/blob/f758b9c1ffc8e96ae84f039b1d757cffda0ddb23/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp#L3607-L3622
I had #123247 for this a while ago, it required a few other changes to avoid regressions after removing the assumes. I can check again what the regerssions are now.
|
@fhahn That's for aligns though. I wouldn't expect doing this for nonnull would cause regressions, as we currently don't use nonnull bundles and already do the opt for |
f758b9c to
c946984
Compare
|
@nikic gentle ping |
|
ping |
nikic
left a comment
There was a problem hiding this comment.
LGTM
Based on llvm-opt-benchmark, one of the big impacts of this change is that we retain nonnull assumptions that were previously eliminated by SCCP/CVP based on dominating information. Similarly to #183688 (review), I'm inclined to say that this is the behavior we want, as assumes only eliminated by SCCP/CVP (but not VT) are probably not sufficiently trivial as to be considered redundant. If we do see issues with too many assumes being retained, we can always adjust those passes to also remove nonnull assumes. Let's give this a try.
…crum Adapt codegen test to accept operand bundles The updated test current fails when rustc is built with HEAD LLVM: https://buildkite.com/llvm-project/rust-llvm-integrate-prototype/builds/44013/steps/canvas?sid=019cafec-3cca-44b2-aa9f-b41c0a940e8b Likely as a result of llvm/llvm-project#169923 Since the new codegen merges two lines into one, I couldn't figure out a way to make the test work on both LLVM versions without introducing revisions. (Though I could instead make the test run on only LLVM 23+). @rustbot label llvm-main
…crum Adapt codegen test to accept operand bundles The updated test current fails when rustc is built with HEAD LLVM: https://buildkite.com/llvm-project/rust-llvm-integrate-prototype/builds/44013/steps/canvas?sid=019cafec-3cca-44b2-aa9f-b41c0a940e8b Likely as a result of llvm/llvm-project#169923 Since the new codegen merges two lines into one, I couldn't figure out a way to make the test work on both LLVM versions without introducing revisions. (Though I could instead make the test run on only LLVM 23+). @rustbot label llvm-main
Rollup merge of #153305 - TimNN:bundle-assert, r=Mark-Simulacrum Adapt codegen test to accept operand bundles The updated test current fails when rustc is built with HEAD LLVM: https://buildkite.com/llvm-project/rust-llvm-integrate-prototype/builds/44013/steps/canvas?sid=019cafec-3cca-44b2-aa9f-b41c0a940e8b Likely as a result of llvm/llvm-project#169923 Since the new codegen merges two lines into one, I couldn't figure out a way to make the test work on both LLVM versions without introducing revisions. (Though I could instead make the test run on only LLVM 23+). @rustbot label llvm-main
Fixes #168688