[IR] Allow poison argument to lifetime markers #151148
Open
+81
−36
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This slightly relaxes the invariant established in llvm#149310, by also allowing the lifetime argument to be poison. This is to support the typical pattern of RAUWing with poison when removing an instruction. It's worth noting that this does not require any conservative assumptions, lifetimes with poison arguments can simply be skipped. Fixes llvm#151119.
llvmbot
added
llvm:globalisel
llvm:instcombine
|
@llvm/pr-subscribers-llvm-globalisel @llvm/pr-subscribers-llvm-selectiondag Author: Nikita Popov (nikic) ChangesThis slightly relaxes the invariant established in #149310, by also allowing the lifetime argument to be poison. This is to support the typical pattern of RAUWing with poison when removing an instruction. It's worth noting that this does not require any conservative assumptions, lifetimes with poison arguments can simply be skipped. Fixes #151119. Full diff: https://github.com/llvm/llvm-project/pull/151148.diff 11 Files Affected:
diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index eb2ef6bc35742..6d2ff9201faf9 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -26656,14 +26656,17 @@ Arguments:
The first argument is a constant integer, which is ignored and will be removed
in the future.
-The second argument is a pointer to an ``alloca`` instruction.
+The second argument is either a pointer to an ``alloca`` instruction or
+a ``poison`` value.
Semantics:
""""""""""
-The stack-allocated object that ``ptr`` points to is initially marked as dead.
-After '``llvm.lifetime.start``', the stack object is marked as alive and has an
-uninitialized value.
+If ``ptr`` is a ``poison`` value, the intrinsic has no effect.
+
+Otherwise, the stack-allocated object that ``ptr`` points to is initially
+marked as dead. After '``llvm.lifetime.start``', the stack object is marked as
+alive and has an uninitialized value.
The stack object is marked as dead when either
:ref:`llvm.lifetime.end <int_lifeend>` to the alloca is executed or the
function returns.
@@ -26697,13 +26700,16 @@ Arguments:
The first argument is a constant integer, which is ignored and will be removed
in the future.
-The second argument is a pointer to an ``alloca`` instruction.
+The second argument is either a pointer to an ``alloca`` instruction or
+a ``poison`` value.
Semantics:
""""""""""
-The stack-allocated object that ``ptr`` points to becomes dead after the call
-to this intrinsic.
+If ``ptr`` is a ``poison`` value, the intrinsic has no effect.
+
+Otherwise, the stack-allocated object that ``ptr`` points to becomes dead after
+the call to this intrinsic.
Calling ``llvm.lifetime.end`` on an already dead alloca is no-op.
diff --git a/llvm/lib/Analysis/StackLifetime.cpp b/llvm/lib/Analysis/StackLifetime.cpp
index 34a7a0416d290..b3f999400f154 100644
--- a/llvm/lib/Analysis/StackLifetime.cpp
+++ b/llvm/lib/Analysis/StackLifetime.cpp
@@ -63,7 +63,10 @@ bool StackLifetime::isAliveAfter(const AllocaInst *AI,
// markers has the same size and points to the alloca start.
static const AllocaInst *findMatchingAlloca(const IntrinsicInst &II,
const DataLayout &DL) {
- const AllocaInst *AI = cast<AllocaInst>(II.getArgOperand(1));
+ const AllocaInst *AI = dyn_cast<AllocaInst>(II.getArgOperand(1));
+ if (!AI)
+ return nullptr;
+
auto AllocaSize = AI->getAllocationSize(DL);
if (!AllocaSize)
return nullptr;
diff --git a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
index dc5dfab4418e5..500244de56c54 100644
--- a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
@@ -2189,8 +2189,8 @@ bool IRTranslator::translateKnownIntrinsic(const CallInst &CI, Intrinsic::ID ID,
unsigned Op = ID == Intrinsic::lifetime_start ? TargetOpcode::LIFETIME_START
: TargetOpcode::LIFETIME_END;
- const AllocaInst *AI = cast<AllocaInst>(CI.getArgOperand(1));
- if (!AI->isStaticAlloca())
+ const AllocaInst *AI = dyn_cast<AllocaInst>(CI.getArgOperand(1));
+ if (!AI || !AI->isStaticAlloca())
return true;
MIRBuilder.buildInstr(Op).addFrameIndex(getOrCreateFrameIndex(*AI));
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index 306e068f1c1da..ac0440fef5f60 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -7598,7 +7598,9 @@ void SelectionDAGBuilder::visitIntrinsicCall(const CallInst &I,
if (TM.getOptLevel() == CodeGenOptLevel::None)
return;
- const AllocaInst *LifetimeObject = cast<AllocaInst>(I.getArgOperand(1));
+ const AllocaInst *LifetimeObject = dyn_cast<AllocaInst>(I.getArgOperand(1));
+ if (!LifetimeObject)
+ return;
// First check that the Alloca is static, otherwise it won't have a
// valid frame index.
diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index 3ff9895e161c4..ca3f148f881a4 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -6769,10 +6769,13 @@ void Verifier::visitIntrinsicCall(Intrinsic::ID ID, CallBase &Call) {
break;
}
case Intrinsic::lifetime_start:
- case Intrinsic::lifetime_end:
- Check(isa<AllocaInst>(Call.getArgOperand(1)),
- "llvm.lifetime.start/end can only be used on alloca", &Call);
+ case Intrinsic::lifetime_end: {
+ Value *Ptr = Call.getArgOperand(1);
+ Check(isa<AllocaInst>(Ptr) || isa<PoisonValue>(Ptr),
+ "llvm.lifetime.start/end can only be used on alloca or poison",
+ &Call);
break;
+ }
};
// Verify that there aren't any unmediated control transfers between funclets.
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index e87bee79a6a69..8da65c597116f 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -1222,9 +1222,9 @@ struct FunctionStackPoisoner : public InstVisitor<FunctionStackPoisoner> {
!ConstantInt::isValueValidForType(IntptrTy, SizeValue))
return;
// Find alloca instruction that corresponds to llvm.lifetime argument.
- AllocaInst *AI = cast<AllocaInst>(II.getArgOperand(1));
+ AllocaInst *AI = dyn_cast<AllocaInst>(II.getArgOperand(1));
// We're interested only in allocas we can handle.
- if (!ASan.isInterestingAlloca(*AI))
+ if (!AI || !ASan.isInterestingAlloca(*AI))
return;
bool DoPoison = (ID == Intrinsic::lifetime_end);
AllocaPoisonCall APC = {&II, AI, SizeValue, DoPoison};
diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
index df31f07097f82..71f07ba7f5cd1 100644
--- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -3301,8 +3301,9 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
void handleLifetimeStart(IntrinsicInst &I) {
if (!PoisonStack)
return;
- AllocaInst *AI = cast<AllocaInst>(I.getArgOperand(1));
- LifetimeStartList.push_back(std::make_pair(&I, AI));
+ AllocaInst *AI = dyn_cast<AllocaInst>(I.getArgOperand(1));
+ if (AI)
+ LifetimeStartList.push_back(std::make_pair(&I, AI));
}
void handleBswap(IntrinsicInst &I) {
diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index babd7f6b3a058..3852f1aa40ac5 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -482,6 +482,9 @@ bool llvm::wouldInstructionBeTriviallyDead(const Instruction *I,
if (II->isLifetimeStartOrEnd()) {
auto *Arg = II->getArgOperand(1);
+ if (isa<PoisonValue>(Arg))
+ return true;
+
// If the only uses of the alloca are lifetime intrinsics, then the
// intrinsics are dead.
return llvm::all_of(Arg->uses(), [](Use &Use) {
diff --git a/llvm/lib/Transforms/Utils/MemoryTaggingSupport.cpp b/llvm/lib/Transforms/Utils/MemoryTaggingSupport.cpp
index bea76d39bb216..472c03f7fc6ca 100644
--- a/llvm/lib/Transforms/Utils/MemoryTaggingSupport.cpp
+++ b/llvm/lib/Transforms/Utils/MemoryTaggingSupport.cpp
@@ -155,8 +155,9 @@ void StackInfoBuilder::visit(OptimizationRemarkEmitter &ORE,
return;
}
if (auto *II = dyn_cast<LifetimeIntrinsic>(&Inst)) {
- AllocaInst *AI = cast<AllocaInst>(II->getArgOperand(1));
- if (getAllocaInterestingness(*AI) != AllocaInterestingness::kInteresting)
+ AllocaInst *AI = dyn_cast<AllocaInst>(II->getArgOperand(1));
+ if (!AI ||
+ getAllocaInterestingness(*AI) != AllocaInterestingness::kInteresting)
return;
if (II->getIntrinsicID() == Intrinsic::lifetime_start)
Info.AllocasToInstrument[AI].LifetimeStart.push_back(II);
diff --git a/llvm/test/Transforms/InstCombine/pr150338.ll b/llvm/test/Transforms/InstCombine/pr150338.ll
deleted file mode 100644
index 2ad454ec60f13..0000000000000
--- a/llvm/test/Transforms/InstCombine/pr150338.ll
+++ /dev/null
@@ -1,16 +0,0 @@
-; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
-; RUN: opt -S -passes=instcombine < %s | FileCheck %s
-
-; Make sure this does not crash.
-define void @test(ptr %arg) {
-; CHECK-LABEL: define void @test(
-; CHECK-SAME: ptr [[ARG:%.*]]) {
-; CHECK-NEXT: store i1 true, ptr poison, align 1
-; CHECK-NEXT: ret void
-;
- %a = alloca i32
- store ptr %a, ptr %arg
- store i1 true, ptr poison
- call void @llvm.lifetime.end.p0(i64 4, ptr %a)
- ret void
-}
diff --git a/llvm/test/Transforms/InstCombine/unreachable-alloca-lifetime-markers.ll b/llvm/test/Transforms/InstCombine/unreachable-alloca-lifetime-markers.ll
new file mode 100644
index 0000000000000..340f3862870ac
--- /dev/null
+++ b/llvm/test/Transforms/InstCombine/unreachable-alloca-lifetime-markers.ll
@@ -0,0 +1,42 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt -S -passes=instcombine < %s | FileCheck %s
+
+; Make sure this does not crash.
+
+define void @pr150338(ptr %arg) {
+; CHECK-LABEL: define void @pr150338(
+; CHECK-SAME: ptr [[ARG:%.*]]) {
+; CHECK-NEXT: store i1 true, ptr poison, align 1
+; CHECK-NEXT: ret void
+;
+ %a = alloca i32
+ store ptr %a, ptr %arg
+ store i1 true, ptr poison
+ call void @llvm.lifetime.end.p0(i64 4, ptr %a)
+ ret void
+}
+
+define ptr @pr151119() {
+; CHECK-LABEL: define ptr @pr151119() {
+; CHECK-NEXT: [[ENTRY:.*:]]
+; CHECK-NEXT: store i1 false, ptr poison, align 1
+; CHECK-NEXT: br i1 false, label %[[BB1:.*]], label %[[BB2:.*]]
+; CHECK: [[BB1]]:
+; CHECK-NEXT: br label %[[BB2]]
+; CHECK: [[BB2]]:
+; CHECK-NEXT: br label %[[BB1]]
+;
+entry:
+ %a = alloca i32, align 4
+ store i1 false, ptr poison
+ br i1 false, label %bb1, label %bb2
+
+bb1:
+ %phi1 = phi ptr [ null, %entry ], [ %phi2, %bb2 ]
+ call void @llvm.lifetime.start.p0(i64 4, ptr %a)
+ br label %bb2
+
+bb2:
+ %phi2 = phi ptr [ null, %entry ], [ %a, %bb1 ]
+ br label %bb1
+}
|
|
@llvm/pr-subscribers-llvm-transforms Author: Nikita Popov (nikic) ChangesThis slightly relaxes the invariant established in #149310, by also allowing the lifetime argument to be poison. This is to support the typical pattern of RAUWing with poison when removing an instruction. It's worth noting that this does not require any conservative assumptions, lifetimes with poison arguments can simply be skipped. Fixes #151119. Full diff: https://github.com/llvm/llvm-project/pull/151148.diff 11 Files Affected:
diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index eb2ef6bc35742..6d2ff9201faf9 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -26656,14 +26656,17 @@ Arguments:
The first argument is a constant integer, which is ignored and will be removed
in the future.
-The second argument is a pointer to an ``alloca`` instruction.
+The second argument is either a pointer to an ``alloca`` instruction or
+a ``poison`` value.
Semantics:
""""""""""
-The stack-allocated object that ``ptr`` points to is initially marked as dead.
-After '``llvm.lifetime.start``', the stack object is marked as alive and has an
-uninitialized value.
+If ``ptr`` is a ``poison`` value, the intrinsic has no effect.
+
+Otherwise, the stack-allocated object that ``ptr`` points to is initially
+marked as dead. After '``llvm.lifetime.start``', the stack object is marked as
+alive and has an uninitialized value.
The stack object is marked as dead when either
:ref:`llvm.lifetime.end <int_lifeend>` to the alloca is executed or the
function returns.
@@ -26697,13 +26700,16 @@ Arguments:
The first argument is a constant integer, which is ignored and will be removed
in the future.
-The second argument is a pointer to an ``alloca`` instruction.
+The second argument is either a pointer to an ``alloca`` instruction or
+a ``poison`` value.
Semantics:
""""""""""
-The stack-allocated object that ``ptr`` points to becomes dead after the call
-to this intrinsic.
+If ``ptr`` is a ``poison`` value, the intrinsic has no effect.
+
+Otherwise, the stack-allocated object that ``ptr`` points to becomes dead after
+the call to this intrinsic.
Calling ``llvm.lifetime.end`` on an already dead alloca is no-op.
diff --git a/llvm/lib/Analysis/StackLifetime.cpp b/llvm/lib/Analysis/StackLifetime.cpp
index 34a7a0416d290..b3f999400f154 100644
--- a/llvm/lib/Analysis/StackLifetime.cpp
+++ b/llvm/lib/Analysis/StackLifetime.cpp
@@ -63,7 +63,10 @@ bool StackLifetime::isAliveAfter(const AllocaInst *AI,
// markers has the same size and points to the alloca start.
static const AllocaInst *findMatchingAlloca(const IntrinsicInst &II,
const DataLayout &DL) {
- const AllocaInst *AI = cast<AllocaInst>(II.getArgOperand(1));
+ const AllocaInst *AI = dyn_cast<AllocaInst>(II.getArgOperand(1));
+ if (!AI)
+ return nullptr;
+
auto AllocaSize = AI->getAllocationSize(DL);
if (!AllocaSize)
return nullptr;
diff --git a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
index dc5dfab4418e5..500244de56c54 100644
--- a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
@@ -2189,8 +2189,8 @@ bool IRTranslator::translateKnownIntrinsic(const CallInst &CI, Intrinsic::ID ID,
unsigned Op = ID == Intrinsic::lifetime_start ? TargetOpcode::LIFETIME_START
: TargetOpcode::LIFETIME_END;
- const AllocaInst *AI = cast<AllocaInst>(CI.getArgOperand(1));
- if (!AI->isStaticAlloca())
+ const AllocaInst *AI = dyn_cast<AllocaInst>(CI.getArgOperand(1));
+ if (!AI || !AI->isStaticAlloca())
return true;
MIRBuilder.buildInstr(Op).addFrameIndex(getOrCreateFrameIndex(*AI));
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index 306e068f1c1da..ac0440fef5f60 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -7598,7 +7598,9 @@ void SelectionDAGBuilder::visitIntrinsicCall(const CallInst &I,
if (TM.getOptLevel() == CodeGenOptLevel::None)
return;
- const AllocaInst *LifetimeObject = cast<AllocaInst>(I.getArgOperand(1));
+ const AllocaInst *LifetimeObject = dyn_cast<AllocaInst>(I.getArgOperand(1));
+ if (!LifetimeObject)
+ return;
// First check that the Alloca is static, otherwise it won't have a
// valid frame index.
diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index 3ff9895e161c4..ca3f148f881a4 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -6769,10 +6769,13 @@ void Verifier::visitIntrinsicCall(Intrinsic::ID ID, CallBase &Call) {
break;
}
case Intrinsic::lifetime_start:
- case Intrinsic::lifetime_end:
- Check(isa<AllocaInst>(Call.getArgOperand(1)),
- "llvm.lifetime.start/end can only be used on alloca", &Call);
+ case Intrinsic::lifetime_end: {
+ Value *Ptr = Call.getArgOperand(1);
+ Check(isa<AllocaInst>(Ptr) || isa<PoisonValue>(Ptr),
+ "llvm.lifetime.start/end can only be used on alloca or poison",
+ &Call);
break;
+ }
};
// Verify that there aren't any unmediated control transfers between funclets.
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index e87bee79a6a69..8da65c597116f 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -1222,9 +1222,9 @@ struct FunctionStackPoisoner : public InstVisitor<FunctionStackPoisoner> {
!ConstantInt::isValueValidForType(IntptrTy, SizeValue))
return;
// Find alloca instruction that corresponds to llvm.lifetime argument.
- AllocaInst *AI = cast<AllocaInst>(II.getArgOperand(1));
+ AllocaInst *AI = dyn_cast<AllocaInst>(II.getArgOperand(1));
// We're interested only in allocas we can handle.
- if (!ASan.isInterestingAlloca(*AI))
+ if (!AI || !ASan.isInterestingAlloca(*AI))
return;
bool DoPoison = (ID == Intrinsic::lifetime_end);
AllocaPoisonCall APC = {&II, AI, SizeValue, DoPoison};
diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
index df31f07097f82..71f07ba7f5cd1 100644
--- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -3301,8 +3301,9 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
void handleLifetimeStart(IntrinsicInst &I) {
if (!PoisonStack)
return;
- AllocaInst *AI = cast<AllocaInst>(I.getArgOperand(1));
- LifetimeStartList.push_back(std::make_pair(&I, AI));
+ AllocaInst *AI = dyn_cast<AllocaInst>(I.getArgOperand(1));
+ if (AI)
+ LifetimeStartList.push_back(std::make_pair(&I, AI));
}
void handleBswap(IntrinsicInst &I) {
diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index babd7f6b3a058..3852f1aa40ac5 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -482,6 +482,9 @@ bool llvm::wouldInstructionBeTriviallyDead(const Instruction *I,
if (II->isLifetimeStartOrEnd()) {
auto *Arg = II->getArgOperand(1);
+ if (isa<PoisonValue>(Arg))
+ return true;
+
// If the only uses of the alloca are lifetime intrinsics, then the
// intrinsics are dead.
return llvm::all_of(Arg->uses(), [](Use &Use) {
diff --git a/llvm/lib/Transforms/Utils/MemoryTaggingSupport.cpp b/llvm/lib/Transforms/Utils/MemoryTaggingSupport.cpp
index bea76d39bb216..472c03f7fc6ca 100644
--- a/llvm/lib/Transforms/Utils/MemoryTaggingSupport.cpp
+++ b/llvm/lib/Transforms/Utils/MemoryTaggingSupport.cpp
@@ -155,8 +155,9 @@ void StackInfoBuilder::visit(OptimizationRemarkEmitter &ORE,
return;
}
if (auto *II = dyn_cast<LifetimeIntrinsic>(&Inst)) {
- AllocaInst *AI = cast<AllocaInst>(II->getArgOperand(1));
- if (getAllocaInterestingness(*AI) != AllocaInterestingness::kInteresting)
+ AllocaInst *AI = dyn_cast<AllocaInst>(II->getArgOperand(1));
+ if (!AI ||
+ getAllocaInterestingness(*AI) != AllocaInterestingness::kInteresting)
return;
if (II->getIntrinsicID() == Intrinsic::lifetime_start)
Info.AllocasToInstrument[AI].LifetimeStart.push_back(II);
diff --git a/llvm/test/Transforms/InstCombine/pr150338.ll b/llvm/test/Transforms/InstCombine/pr150338.ll
deleted file mode 100644
index 2ad454ec60f13..0000000000000
--- a/llvm/test/Transforms/InstCombine/pr150338.ll
+++ /dev/null
@@ -1,16 +0,0 @@
-; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
-; RUN: opt -S -passes=instcombine < %s | FileCheck %s
-
-; Make sure this does not crash.
-define void @test(ptr %arg) {
-; CHECK-LABEL: define void @test(
-; CHECK-SAME: ptr [[ARG:%.*]]) {
-; CHECK-NEXT: store i1 true, ptr poison, align 1
-; CHECK-NEXT: ret void
-;
- %a = alloca i32
- store ptr %a, ptr %arg
- store i1 true, ptr poison
- call void @llvm.lifetime.end.p0(i64 4, ptr %a)
- ret void
-}
diff --git a/llvm/test/Transforms/InstCombine/unreachable-alloca-lifetime-markers.ll b/llvm/test/Transforms/InstCombine/unreachable-alloca-lifetime-markers.ll
new file mode 100644
index 0000000000000..340f3862870ac
--- /dev/null
+++ b/llvm/test/Transforms/InstCombine/unreachable-alloca-lifetime-markers.ll
@@ -0,0 +1,42 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt -S -passes=instcombine < %s | FileCheck %s
+
+; Make sure this does not crash.
+
+define void @pr150338(ptr %arg) {
+; CHECK-LABEL: define void @pr150338(
+; CHECK-SAME: ptr [[ARG:%.*]]) {
+; CHECK-NEXT: store i1 true, ptr poison, align 1
+; CHECK-NEXT: ret void
+;
+ %a = alloca i32
+ store ptr %a, ptr %arg
+ store i1 true, ptr poison
+ call void @llvm.lifetime.end.p0(i64 4, ptr %a)
+ ret void
+}
+
+define ptr @pr151119() {
+; CHECK-LABEL: define ptr @pr151119() {
+; CHECK-NEXT: [[ENTRY:.*:]]
+; CHECK-NEXT: store i1 false, ptr poison, align 1
+; CHECK-NEXT: br i1 false, label %[[BB1:.*]], label %[[BB2:.*]]
+; CHECK: [[BB1]]:
+; CHECK-NEXT: br label %[[BB2]]
+; CHECK: [[BB2]]:
+; CHECK-NEXT: br label %[[BB1]]
+;
+entry:
+ %a = alloca i32, align 4
+ store i1 false, ptr poison
+ br i1 false, label %bb1, label %bb2
+
+bb1:
+ %phi1 = phi ptr [ null, %entry ], [ %phi2, %bb2 ]
+ call void @llvm.lifetime.start.p0(i64 4, ptr %a)
+ br label %bb2
+
+bb2:
+ %phi2 = phi ptr [ null, %entry ], [ %a, %bb1 ]
+ br label %bb1
+}
|
|
@llvm/pr-subscribers-llvm-ir Author: Nikita Popov (nikic) ChangesThis slightly relaxes the invariant established in #149310, by also allowing the lifetime argument to be poison. This is to support the typical pattern of RAUWing with poison when removing an instruction. It's worth noting that this does not require any conservative assumptions, lifetimes with poison arguments can simply be skipped. Fixes #151119. Full diff: https://github.com/llvm/llvm-project/pull/151148.diff 11 Files Affected:
diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index eb2ef6bc35742..6d2ff9201faf9 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -26656,14 +26656,17 @@ Arguments:
The first argument is a constant integer, which is ignored and will be removed
in the future.
-The second argument is a pointer to an ``alloca`` instruction.
+The second argument is either a pointer to an ``alloca`` instruction or
+a ``poison`` value.
Semantics:
""""""""""
-The stack-allocated object that ``ptr`` points to is initially marked as dead.
-After '``llvm.lifetime.start``', the stack object is marked as alive and has an
-uninitialized value.
+If ``ptr`` is a ``poison`` value, the intrinsic has no effect.
+
+Otherwise, the stack-allocated object that ``ptr`` points to is initially
+marked as dead. After '``llvm.lifetime.start``', the stack object is marked as
+alive and has an uninitialized value.
The stack object is marked as dead when either
:ref:`llvm.lifetime.end <int_lifeend>` to the alloca is executed or the
function returns.
@@ -26697,13 +26700,16 @@ Arguments:
The first argument is a constant integer, which is ignored and will be removed
in the future.
-The second argument is a pointer to an ``alloca`` instruction.
+The second argument is either a pointer to an ``alloca`` instruction or
+a ``poison`` value.
Semantics:
""""""""""
-The stack-allocated object that ``ptr`` points to becomes dead after the call
-to this intrinsic.
+If ``ptr`` is a ``poison`` value, the intrinsic has no effect.
+
+Otherwise, the stack-allocated object that ``ptr`` points to becomes dead after
+the call to this intrinsic.
Calling ``llvm.lifetime.end`` on an already dead alloca is no-op.
diff --git a/llvm/lib/Analysis/StackLifetime.cpp b/llvm/lib/Analysis/StackLifetime.cpp
index 34a7a0416d290..b3f999400f154 100644
--- a/llvm/lib/Analysis/StackLifetime.cpp
+++ b/llvm/lib/Analysis/StackLifetime.cpp
@@ -63,7 +63,10 @@ bool StackLifetime::isAliveAfter(const AllocaInst *AI,
// markers has the same size and points to the alloca start.
static const AllocaInst *findMatchingAlloca(const IntrinsicInst &II,
const DataLayout &DL) {
- const AllocaInst *AI = cast<AllocaInst>(II.getArgOperand(1));
+ const AllocaInst *AI = dyn_cast<AllocaInst>(II.getArgOperand(1));
+ if (!AI)
+ return nullptr;
+
auto AllocaSize = AI->getAllocationSize(DL);
if (!AllocaSize)
return nullptr;
diff --git a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
index dc5dfab4418e5..500244de56c54 100644
--- a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
@@ -2189,8 +2189,8 @@ bool IRTranslator::translateKnownIntrinsic(const CallInst &CI, Intrinsic::ID ID,
unsigned Op = ID == Intrinsic::lifetime_start ? TargetOpcode::LIFETIME_START
: TargetOpcode::LIFETIME_END;
- const AllocaInst *AI = cast<AllocaInst>(CI.getArgOperand(1));
- if (!AI->isStaticAlloca())
+ const AllocaInst *AI = dyn_cast<AllocaInst>(CI.getArgOperand(1));
+ if (!AI || !AI->isStaticAlloca())
return true;
MIRBuilder.buildInstr(Op).addFrameIndex(getOrCreateFrameIndex(*AI));
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index 306e068f1c1da..ac0440fef5f60 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -7598,7 +7598,9 @@ void SelectionDAGBuilder::visitIntrinsicCall(const CallInst &I,
if (TM.getOptLevel() == CodeGenOptLevel::None)
return;
- const AllocaInst *LifetimeObject = cast<AllocaInst>(I.getArgOperand(1));
+ const AllocaInst *LifetimeObject = dyn_cast<AllocaInst>(I.getArgOperand(1));
+ if (!LifetimeObject)
+ return;
// First check that the Alloca is static, otherwise it won't have a
// valid frame index.
diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index 3ff9895e161c4..ca3f148f881a4 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -6769,10 +6769,13 @@ void Verifier::visitIntrinsicCall(Intrinsic::ID ID, CallBase &Call) {
break;
}
case Intrinsic::lifetime_start:
- case Intrinsic::lifetime_end:
- Check(isa<AllocaInst>(Call.getArgOperand(1)),
- "llvm.lifetime.start/end can only be used on alloca", &Call);
+ case Intrinsic::lifetime_end: {
+ Value *Ptr = Call.getArgOperand(1);
+ Check(isa<AllocaInst>(Ptr) || isa<PoisonValue>(Ptr),
+ "llvm.lifetime.start/end can only be used on alloca or poison",
+ &Call);
break;
+ }
};
// Verify that there aren't any unmediated control transfers between funclets.
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index e87bee79a6a69..8da65c597116f 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -1222,9 +1222,9 @@ struct FunctionStackPoisoner : public InstVisitor<FunctionStackPoisoner> {
!ConstantInt::isValueValidForType(IntptrTy, SizeValue))
return;
// Find alloca instruction that corresponds to llvm.lifetime argument.
- AllocaInst *AI = cast<AllocaInst>(II.getArgOperand(1));
+ AllocaInst *AI = dyn_cast<AllocaInst>(II.getArgOperand(1));
// We're interested only in allocas we can handle.
- if (!ASan.isInterestingAlloca(*AI))
+ if (!AI || !ASan.isInterestingAlloca(*AI))
return;
bool DoPoison = (ID == Intrinsic::lifetime_end);
AllocaPoisonCall APC = {&II, AI, SizeValue, DoPoison};
diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
index df31f07097f82..71f07ba7f5cd1 100644
--- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -3301,8 +3301,9 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
void handleLifetimeStart(IntrinsicInst &I) {
if (!PoisonStack)
return;
- AllocaInst *AI = cast<AllocaInst>(I.getArgOperand(1));
- LifetimeStartList.push_back(std::make_pair(&I, AI));
+ AllocaInst *AI = dyn_cast<AllocaInst>(I.getArgOperand(1));
+ if (AI)
+ LifetimeStartList.push_back(std::make_pair(&I, AI));
}
void handleBswap(IntrinsicInst &I) {
diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index babd7f6b3a058..3852f1aa40ac5 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -482,6 +482,9 @@ bool llvm::wouldInstructionBeTriviallyDead(const Instruction *I,
if (II->isLifetimeStartOrEnd()) {
auto *Arg = II->getArgOperand(1);
+ if (isa<PoisonValue>(Arg))
+ return true;
+
// If the only uses of the alloca are lifetime intrinsics, then the
// intrinsics are dead.
return llvm::all_of(Arg->uses(), [](Use &Use) {
diff --git a/llvm/lib/Transforms/Utils/MemoryTaggingSupport.cpp b/llvm/lib/Transforms/Utils/MemoryTaggingSupport.cpp
index bea76d39bb216..472c03f7fc6ca 100644
--- a/llvm/lib/Transforms/Utils/MemoryTaggingSupport.cpp
+++ b/llvm/lib/Transforms/Utils/MemoryTaggingSupport.cpp
@@ -155,8 +155,9 @@ void StackInfoBuilder::visit(OptimizationRemarkEmitter &ORE,
return;
}
if (auto *II = dyn_cast<LifetimeIntrinsic>(&Inst)) {
- AllocaInst *AI = cast<AllocaInst>(II->getArgOperand(1));
- if (getAllocaInterestingness(*AI) != AllocaInterestingness::kInteresting)
+ AllocaInst *AI = dyn_cast<AllocaInst>(II->getArgOperand(1));
+ if (!AI ||
+ getAllocaInterestingness(*AI) != AllocaInterestingness::kInteresting)
return;
if (II->getIntrinsicID() == Intrinsic::lifetime_start)
Info.AllocasToInstrument[AI].LifetimeStart.push_back(II);
diff --git a/llvm/test/Transforms/InstCombine/pr150338.ll b/llvm/test/Transforms/InstCombine/pr150338.ll
deleted file mode 100644
index 2ad454ec60f13..0000000000000
--- a/llvm/test/Transforms/InstCombine/pr150338.ll
+++ /dev/null
@@ -1,16 +0,0 @@
-; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
-; RUN: opt -S -passes=instcombine < %s | FileCheck %s
-
-; Make sure this does not crash.
-define void @test(ptr %arg) {
-; CHECK-LABEL: define void @test(
-; CHECK-SAME: ptr [[ARG:%.*]]) {
-; CHECK-NEXT: store i1 true, ptr poison, align 1
-; CHECK-NEXT: ret void
-;
- %a = alloca i32
- store ptr %a, ptr %arg
- store i1 true, ptr poison
- call void @llvm.lifetime.end.p0(i64 4, ptr %a)
- ret void
-}
diff --git a/llvm/test/Transforms/InstCombine/unreachable-alloca-lifetime-markers.ll b/llvm/test/Transforms/InstCombine/unreachable-alloca-lifetime-markers.ll
new file mode 100644
index 0000000000000..340f3862870ac
--- /dev/null
+++ b/llvm/test/Transforms/InstCombine/unreachable-alloca-lifetime-markers.ll
@@ -0,0 +1,42 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt -S -passes=instcombine < %s | FileCheck %s
+
+; Make sure this does not crash.
+
+define void @pr150338(ptr %arg) {
+; CHECK-LABEL: define void @pr150338(
+; CHECK-SAME: ptr [[ARG:%.*]]) {
+; CHECK-NEXT: store i1 true, ptr poison, align 1
+; CHECK-NEXT: ret void
+;
+ %a = alloca i32
+ store ptr %a, ptr %arg
+ store i1 true, ptr poison
+ call void @llvm.lifetime.end.p0(i64 4, ptr %a)
+ ret void
+}
+
+define ptr @pr151119() {
+; CHECK-LABEL: define ptr @pr151119() {
+; CHECK-NEXT: [[ENTRY:.*:]]
+; CHECK-NEXT: store i1 false, ptr poison, align 1
+; CHECK-NEXT: br i1 false, label %[[BB1:.*]], label %[[BB2:.*]]
+; CHECK: [[BB1]]:
+; CHECK-NEXT: br label %[[BB2]]
+; CHECK: [[BB2]]:
+; CHECK-NEXT: br label %[[BB1]]
+;
+entry:
+ %a = alloca i32, align 4
+ store i1 false, ptr poison
+ br i1 false, label %bb1, label %bb2
+
+bb1:
+ %phi1 = phi ptr [ null, %entry ], [ %phi2, %bb2 ]
+ call void @llvm.lifetime.start.p0(i64 4, ptr %a)
+ br label %bb2
+
+bb2:
+ %phi2 = phi ptr [ null, %entry ], [ %a, %bb1 ]
+ br label %bb1
+}
|
|
@llvm/pr-subscribers-llvm-analysis Author: Nikita Popov (nikic) ChangesThis slightly relaxes the invariant established in #149310, by also allowing the lifetime argument to be poison. This is to support the typical pattern of RAUWing with poison when removing an instruction. It's worth noting that this does not require any conservative assumptions, lifetimes with poison arguments can simply be skipped. Fixes #151119. Full diff: https://github.com/llvm/llvm-project/pull/151148.diff 11 Files Affected:
diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index eb2ef6bc35742..6d2ff9201faf9 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -26656,14 +26656,17 @@ Arguments:
The first argument is a constant integer, which is ignored and will be removed
in the future.
-The second argument is a pointer to an ``alloca`` instruction.
+The second argument is either a pointer to an ``alloca`` instruction or
+a ``poison`` value.
Semantics:
""""""""""
-The stack-allocated object that ``ptr`` points to is initially marked as dead.
-After '``llvm.lifetime.start``', the stack object is marked as alive and has an
-uninitialized value.
+If ``ptr`` is a ``poison`` value, the intrinsic has no effect.
+
+Otherwise, the stack-allocated object that ``ptr`` points to is initially
+marked as dead. After '``llvm.lifetime.start``', the stack object is marked as
+alive and has an uninitialized value.
The stack object is marked as dead when either
:ref:`llvm.lifetime.end <int_lifeend>` to the alloca is executed or the
function returns.
@@ -26697,13 +26700,16 @@ Arguments:
The first argument is a constant integer, which is ignored and will be removed
in the future.
-The second argument is a pointer to an ``alloca`` instruction.
+The second argument is either a pointer to an ``alloca`` instruction or
+a ``poison`` value.
Semantics:
""""""""""
-The stack-allocated object that ``ptr`` points to becomes dead after the call
-to this intrinsic.
+If ``ptr`` is a ``poison`` value, the intrinsic has no effect.
+
+Otherwise, the stack-allocated object that ``ptr`` points to becomes dead after
+the call to this intrinsic.
Calling ``llvm.lifetime.end`` on an already dead alloca is no-op.
diff --git a/llvm/lib/Analysis/StackLifetime.cpp b/llvm/lib/Analysis/StackLifetime.cpp
index 34a7a0416d290..b3f999400f154 100644
--- a/llvm/lib/Analysis/StackLifetime.cpp
+++ b/llvm/lib/Analysis/StackLifetime.cpp
@@ -63,7 +63,10 @@ bool StackLifetime::isAliveAfter(const AllocaInst *AI,
// markers has the same size and points to the alloca start.
static const AllocaInst *findMatchingAlloca(const IntrinsicInst &II,
const DataLayout &DL) {
- const AllocaInst *AI = cast<AllocaInst>(II.getArgOperand(1));
+ const AllocaInst *AI = dyn_cast<AllocaInst>(II.getArgOperand(1));
+ if (!AI)
+ return nullptr;
+
auto AllocaSize = AI->getAllocationSize(DL);
if (!AllocaSize)
return nullptr;
diff --git a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
index dc5dfab4418e5..500244de56c54 100644
--- a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
@@ -2189,8 +2189,8 @@ bool IRTranslator::translateKnownIntrinsic(const CallInst &CI, Intrinsic::ID ID,
unsigned Op = ID == Intrinsic::lifetime_start ? TargetOpcode::LIFETIME_START
: TargetOpcode::LIFETIME_END;
- const AllocaInst *AI = cast<AllocaInst>(CI.getArgOperand(1));
- if (!AI->isStaticAlloca())
+ const AllocaInst *AI = dyn_cast<AllocaInst>(CI.getArgOperand(1));
+ if (!AI || !AI->isStaticAlloca())
return true;
MIRBuilder.buildInstr(Op).addFrameIndex(getOrCreateFrameIndex(*AI));
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index 306e068f1c1da..ac0440fef5f60 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -7598,7 +7598,9 @@ void SelectionDAGBuilder::visitIntrinsicCall(const CallInst &I,
if (TM.getOptLevel() == CodeGenOptLevel::None)
return;
- const AllocaInst *LifetimeObject = cast<AllocaInst>(I.getArgOperand(1));
+ const AllocaInst *LifetimeObject = dyn_cast<AllocaInst>(I.getArgOperand(1));
+ if (!LifetimeObject)
+ return;
// First check that the Alloca is static, otherwise it won't have a
// valid frame index.
diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index 3ff9895e161c4..ca3f148f881a4 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -6769,10 +6769,13 @@ void Verifier::visitIntrinsicCall(Intrinsic::ID ID, CallBase &Call) {
break;
}
case Intrinsic::lifetime_start:
- case Intrinsic::lifetime_end:
- Check(isa<AllocaInst>(Call.getArgOperand(1)),
- "llvm.lifetime.start/end can only be used on alloca", &Call);
+ case Intrinsic::lifetime_end: {
+ Value *Ptr = Call.getArgOperand(1);
+ Check(isa<AllocaInst>(Ptr) || isa<PoisonValue>(Ptr),
+ "llvm.lifetime.start/end can only be used on alloca or poison",
+ &Call);
break;
+ }
};
// Verify that there aren't any unmediated control transfers between funclets.
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index e87bee79a6a69..8da65c597116f 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -1222,9 +1222,9 @@ struct FunctionStackPoisoner : public InstVisitor<FunctionStackPoisoner> {
!ConstantInt::isValueValidForType(IntptrTy, SizeValue))
return;
// Find alloca instruction that corresponds to llvm.lifetime argument.
- AllocaInst *AI = cast<AllocaInst>(II.getArgOperand(1));
+ AllocaInst *AI = dyn_cast<AllocaInst>(II.getArgOperand(1));
// We're interested only in allocas we can handle.
- if (!ASan.isInterestingAlloca(*AI))
+ if (!AI || !ASan.isInterestingAlloca(*AI))
return;
bool DoPoison = (ID == Intrinsic::lifetime_end);
AllocaPoisonCall APC = {&II, AI, SizeValue, DoPoison};
diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
index df31f07097f82..71f07ba7f5cd1 100644
--- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -3301,8 +3301,9 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
void handleLifetimeStart(IntrinsicInst &I) {
if (!PoisonStack)
return;
- AllocaInst *AI = cast<AllocaInst>(I.getArgOperand(1));
- LifetimeStartList.push_back(std::make_pair(&I, AI));
+ AllocaInst *AI = dyn_cast<AllocaInst>(I.getArgOperand(1));
+ if (AI)
+ LifetimeStartList.push_back(std::make_pair(&I, AI));
}
void handleBswap(IntrinsicInst &I) {
diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index babd7f6b3a058..3852f1aa40ac5 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -482,6 +482,9 @@ bool llvm::wouldInstructionBeTriviallyDead(const Instruction *I,
if (II->isLifetimeStartOrEnd()) {
auto *Arg = II->getArgOperand(1);
+ if (isa<PoisonValue>(Arg))
+ return true;
+
// If the only uses of the alloca are lifetime intrinsics, then the
// intrinsics are dead.
return llvm::all_of(Arg->uses(), [](Use &Use) {
diff --git a/llvm/lib/Transforms/Utils/MemoryTaggingSupport.cpp b/llvm/lib/Transforms/Utils/MemoryTaggingSupport.cpp
index bea76d39bb216..472c03f7fc6ca 100644
--- a/llvm/lib/Transforms/Utils/MemoryTaggingSupport.cpp
+++ b/llvm/lib/Transforms/Utils/MemoryTaggingSupport.cpp
@@ -155,8 +155,9 @@ void StackInfoBuilder::visit(OptimizationRemarkEmitter &ORE,
return;
}
if (auto *II = dyn_cast<LifetimeIntrinsic>(&Inst)) {
- AllocaInst *AI = cast<AllocaInst>(II->getArgOperand(1));
- if (getAllocaInterestingness(*AI) != AllocaInterestingness::kInteresting)
+ AllocaInst *AI = dyn_cast<AllocaInst>(II->getArgOperand(1));
+ if (!AI ||
+ getAllocaInterestingness(*AI) != AllocaInterestingness::kInteresting)
return;
if (II->getIntrinsicID() == Intrinsic::lifetime_start)
Info.AllocasToInstrument[AI].LifetimeStart.push_back(II);
diff --git a/llvm/test/Transforms/InstCombine/pr150338.ll b/llvm/test/Transforms/InstCombine/pr150338.ll
deleted file mode 100644
index 2ad454ec60f13..0000000000000
--- a/llvm/test/Transforms/InstCombine/pr150338.ll
+++ /dev/null
@@ -1,16 +0,0 @@
-; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
-; RUN: opt -S -passes=instcombine < %s | FileCheck %s
-
-; Make sure this does not crash.
-define void @test(ptr %arg) {
-; CHECK-LABEL: define void @test(
-; CHECK-SAME: ptr [[ARG:%.*]]) {
-; CHECK-NEXT: store i1 true, ptr poison, align 1
-; CHECK-NEXT: ret void
-;
- %a = alloca i32
- store ptr %a, ptr %arg
- store i1 true, ptr poison
- call void @llvm.lifetime.end.p0(i64 4, ptr %a)
- ret void
-}
diff --git a/llvm/test/Transforms/InstCombine/unreachable-alloca-lifetime-markers.ll b/llvm/test/Transforms/InstCombine/unreachable-alloca-lifetime-markers.ll
new file mode 100644
index 0000000000000..340f3862870ac
--- /dev/null
+++ b/llvm/test/Transforms/InstCombine/unreachable-alloca-lifetime-markers.ll
@@ -0,0 +1,42 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt -S -passes=instcombine < %s | FileCheck %s
+
+; Make sure this does not crash.
+
+define void @pr150338(ptr %arg) {
+; CHECK-LABEL: define void @pr150338(
+; CHECK-SAME: ptr [[ARG:%.*]]) {
+; CHECK-NEXT: store i1 true, ptr poison, align 1
+; CHECK-NEXT: ret void
+;
+ %a = alloca i32
+ store ptr %a, ptr %arg
+ store i1 true, ptr poison
+ call void @llvm.lifetime.end.p0(i64 4, ptr %a)
+ ret void
+}
+
+define ptr @pr151119() {
+; CHECK-LABEL: define ptr @pr151119() {
+; CHECK-NEXT: [[ENTRY:.*:]]
+; CHECK-NEXT: store i1 false, ptr poison, align 1
+; CHECK-NEXT: br i1 false, label %[[BB1:.*]], label %[[BB2:.*]]
+; CHECK: [[BB1]]:
+; CHECK-NEXT: br label %[[BB2]]
+; CHECK: [[BB2]]:
+; CHECK-NEXT: br label %[[BB1]]
+;
+entry:
+ %a = alloca i32, align 4
+ store i1 false, ptr poison
+ br i1 false, label %bb1, label %bb2
+
+bb1:
+ %phi1 = phi ptr [ null, %entry ], [ %phi2, %bb2 ]
+ call void @llvm.lifetime.start.p0(i64 4, ptr %a)
+ br label %bb2
+
+bb2:
+ %phi2 = phi ptr [ null, %entry ], [ %a, %bb1 ]
+ br label %bb1
+}
|
arsenm
reviewed
Jul 29, 2025
| @@ -26656,14 +26656,17 @@ Arguments: | |||
| The first argument is a constant integer, which is ignored and will be removed | |||
| in the future. | |||
|
|
|||
| The second argument is a pointer to an ``alloca`` instruction. | |||
| The second argument is either a pointer to an ``alloca`` instruction or | |||
| a ``poison`` value. | |||
There was a problem hiding this comment.
Can we relax this to any constant? As it is this will require special casing in llvm-reduce at a minimum, but other places also could replace with null
There was a problem hiding this comment.
I'd rather not broaden this if we can be avoided at all. Allowing any constant would also allow globals, where people might reasonable expect it to do something.
Adding some special cases to llvm-reduce should be fine to avoid invalid reductions, e.g. #93713 is now more relevant. We can also skip the zero replacement for lifetimes, which just is not a useful thing to do (it should be either dropped entirely or left alone -- also the fact that llvm-reduce replaces the size argument with zero is really annoying, though that will go away soon anyway).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This slightly relaxes the invariant established in #149310, by also allowing the lifetime argument to be poison. This is to support the typical pattern of RAUWing with poison when removing an instruction.
It's worth noting that this does not require any conservative assumptions, lifetimes with poison arguments can simply be skipped.
Fixes #151119.