[IR] Add dead_on_unwind attribute #74289
Merged
Conversation
This file contains 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
Add the `dead_on_unwind` attribute, which states that the caller will not read from this argument if the call unwinds. This allows eliding stores that could otherwise be visible on the unwind path, for example: ``` declare void @may_unwind() define void @src(ptr noalias dead_on_unwind %out) { store i32 0, ptr %out call void @may_unwind() store i32 1, ptr %out ret void } define void @tgt(ptr noalias dead_on_unwind %out) { call void @may_unwind() store i32 1, ptr %out ret void } ``` The optimization is not valid without `dead_on_unwind`, because the `i32 0` value might be read if `@may_unwind` unwinds. This attribute is primarily intended to be used on sret arguments. In fact, I previously wanted to change the semantics of sret to include this "no read after unwind" property (see D116998), but based on the feedback there it is better to keep these attributes orthogonal (sret is an ABI attribute, dead_on_unwind is an optimization attribute). This is a reboot of that change with a separate attribute.
nikic
requested review from
rnk,
alinas,
jdoerfert,
aeubanks and
efriedma-quic
|
@llvm/pr-subscribers-llvm-analysis @llvm/pr-subscribers-llvm-ir Author: Nikita Popov (nikic) Changes> This patch has been migrated from https://reviews.llvm.org/D157499, because apparently Phabricator stopped sending notification emails, which means that review comments go unnoticed. Add the The optimization is not valid without This attribute is primarily intended to be used on sret arguments. In fact, I previously wanted to change the semantics of sret to include this "no read after unwind" property (see D116998), but based on the feedback there it is better to keep these attributes orthogonal (sret is an ABI attribute, dead_on_unwind is an optimization attribute). This is a reboot of that change with a separate attribute. Full diff: https://github.com/llvm/llvm-project/pull/74289.diff 13 Files Affected:
diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index af064d7ac2195..9125f95075779 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -1554,6 +1554,20 @@ Currently, only the following parameter attributes are defined:
``readonly`` or a ``memory`` attribute that does not contain
``argmem: write``.
+``dead_on_unwind``
+ At a high level, this attribute indicates that the pointer argument is dead
+ if the call unwinds, in the sense that the caller will not depend on the
+ contents of the memory. Stores that would only be visible on the unwind
+ path can be elided.
+
+ More precisely, the behavior is as-if any memory written through the
+ pointer during the execution of the function is overwritten with a poison
+ value on unwind. This includes memory written by the implicit write implied
+ by the ``writable`` attribute. The caller is allowed to access the affected
+ memory, but all loads that are not preceded by a store will return poison.
+
+ This attribute cannot be applied to return values.
+
.. _gc:
Garbage Collector Strategy Names
diff --git a/llvm/include/llvm/Bitcode/LLVMBitCodes.h b/llvm/include/llvm/Bitcode/LLVMBitCodes.h
index 99a41fa107d08..16a5fbc6d180e 100644
--- a/llvm/include/llvm/Bitcode/LLVMBitCodes.h
+++ b/llvm/include/llvm/Bitcode/LLVMBitCodes.h
@@ -723,6 +723,7 @@ enum AttributeKindCodes {
ATTR_KIND_OPTIMIZE_FOR_DEBUGGING = 88,
ATTR_KIND_WRITABLE = 89,
ATTR_KIND_CORO_ONLY_DESTROY_WHEN_COMPLETE = 90,
+ ATTR_KIND_DEAD_ON_UNWIND = 91,
};
enum ComdatSelectionKindCodes {
diff --git a/llvm/include/llvm/IR/Attributes.td b/llvm/include/llvm/IR/Attributes.td
index fc38e68ad273b..864f87f338389 100644
--- a/llvm/include/llvm/IR/Attributes.td
+++ b/llvm/include/llvm/IR/Attributes.td
@@ -151,6 +151,9 @@ def NoDuplicate : EnumAttr<"noduplicate", [FnAttr]>;
/// Function does not deallocate memory.
def NoFree : EnumAttr<"nofree", [FnAttr, ParamAttr]>;
+/// Argument is dead if the call unwinds.
+def DeadOnUnwind : EnumAttr<"dead_on_unwind", [ParamAttr]>;
+
/// Disable implicit floating point insts.
def NoImplicitFloat : EnumAttr<"noimplicitfloat", [FnAttr]>;
diff --git a/llvm/lib/Analysis/AliasAnalysis.cpp b/llvm/lib/Analysis/AliasAnalysis.cpp
index b8a22184d9602..da18279ae9b93 100644
--- a/llvm/lib/Analysis/AliasAnalysis.cpp
+++ b/llvm/lib/Analysis/AliasAnalysis.cpp
@@ -896,7 +896,7 @@ bool llvm::isNotVisibleOnUnwind(const Value *Object,
// Byval goes out of scope on unwind.
if (auto *A = dyn_cast<Argument>(Object))
- return A->hasByValAttr();
+ return A->hasByValAttr() || A->hasAttribute(Attribute::DeadOnUnwind);
// A noalias return is not accessible from any other code. If the pointer
// does not escape prior to the unwind, then the caller cannot access the
diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
index e4c3770946b3a..8ae3caaea604a 100644
--- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -2065,6 +2065,8 @@ static Attribute::AttrKind getAttrFromCode(uint64_t Code) {
return Attribute::Writable;
case bitc::ATTR_KIND_CORO_ONLY_DESTROY_WHEN_COMPLETE:
return Attribute::CoroDestroyOnlyWhenComplete;
+ case bitc::ATTR_KIND_DEAD_ON_UNWIND:
+ return Attribute::DeadOnUnwind;
}
}
diff --git a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
index 8239775d04865..d4f62f1f0bc4c 100644
--- a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -827,6 +827,8 @@ static uint64_t getAttrKindEncoding(Attribute::AttrKind Kind) {
return bitc::ATTR_KIND_WRITABLE;
case Attribute::CoroDestroyOnlyWhenComplete:
return bitc::ATTR_KIND_CORO_ONLY_DESTROY_WHEN_COMPLETE;
+ case Attribute::DeadOnUnwind:
+ return bitc::ATTR_KIND_DEAD_ON_UNWIND;
case Attribute::EndAttrKinds:
llvm_unreachable("Can not encode end-attribute kinds marker.");
case Attribute::None:
diff --git a/llvm/lib/IR/Attributes.cpp b/llvm/lib/IR/Attributes.cpp
index 10a74ed68a392..fd5160209506f 100644
--- a/llvm/lib/IR/Attributes.cpp
+++ b/llvm/lib/IR/Attributes.cpp
@@ -1962,7 +1962,8 @@ AttributeMask AttributeFuncs::typeIncompatible(Type *Ty,
.addAttribute(Attribute::ReadOnly)
.addAttribute(Attribute::Dereferenceable)
.addAttribute(Attribute::DereferenceableOrNull)
- .addAttribute(Attribute::Writable);
+ .addAttribute(Attribute::Writable)
+ .addAttribute(Attribute::DeadOnUnwind);
if (ASK & ASK_UNSAFE_TO_DROP)
Incompatible.addAttribute(Attribute::Nest)
.addAttribute(Attribute::SwiftError)
diff --git a/llvm/lib/Transforms/Utils/CodeExtractor.cpp b/llvm/lib/Transforms/Utils/CodeExtractor.cpp
index 9c1186232e024..f5abed0dd5178 100644
--- a/llvm/lib/Transforms/Utils/CodeExtractor.cpp
+++ b/llvm/lib/Transforms/Utils/CodeExtractor.cpp
@@ -998,6 +998,7 @@ Function *CodeExtractor::constructFunction(const ValueSet &inputs,
case Attribute::ByRef:
case Attribute::WriteOnly:
case Attribute::Writable:
+ case Attribute::DeadOnUnwind:
// These are not really attributes.
case Attribute::None:
case Attribute::EndAttrKinds:
diff --git a/llvm/test/Bitcode/attributes.ll b/llvm/test/Bitcode/attributes.ll
index c4bced5c003be..6921f11a352dd 100644
--- a/llvm/test/Bitcode/attributes.ll
+++ b/llvm/test/Bitcode/attributes.ll
@@ -521,6 +521,11 @@ define void @f90(ptr writable %p) {
ret void
}
+; CHECK: define void @f91(ptr dead_on_unwind %p)
+define void @f91(ptr dead_on_unwind %p) {
+ ret void
+}
+
; CHECK: attributes #0 = { noreturn }
; CHECK: attributes #1 = { nounwind }
; CHECK: attributes #2 = { memory(none) }
diff --git a/llvm/test/Transforms/DeadStoreElimination/simple.ll b/llvm/test/Transforms/DeadStoreElimination/simple.ll
index 4cfbbe16667ca..e5d3dd09fa148 100644
--- a/llvm/test/Transforms/DeadStoreElimination/simple.ll
+++ b/llvm/test/Transforms/DeadStoreElimination/simple.ll
@@ -514,13 +514,12 @@ define void @test34(ptr noalias %p) {
store i32 0, ptr %p
ret void
}
-; Same as previous case, but with an sret argument.
-; TODO: The first store could be eliminated if sret is not visible on unwind.
-define void @test34_sret(ptr noalias sret(i32) %p) {
-; CHECK-LABEL: @test34_sret(
-; CHECK-NEXT: store i32 1, ptr [[P:%.*]], align 4
+
+; Same as previous case, but with a dead_on_unwind argument.
+define void @test34_dead_on_unwind(ptr noalias dead_on_unwind %p) {
+; CHECK-LABEL: @test34_dead_on_unwind(
; CHECK-NEXT: call void @unknown_func()
-; CHECK-NEXT: store i32 0, ptr [[P]], align 4
+; CHECK-NEXT: store i32 0, ptr [[P:%.*]], align 4
; CHECK-NEXT: ret void
;
store i32 1, ptr %p
diff --git a/llvm/test/Transforms/LICM/scalar-promote-unwind.ll b/llvm/test/Transforms/LICM/scalar-promote-unwind.ll
index ac79aec5ffc2f..f4197c65f3cb3 100644
--- a/llvm/test/Transforms/LICM/scalar-promote-unwind.ll
+++ b/llvm/test/Transforms/LICM/scalar-promote-unwind.ll
@@ -147,9 +147,8 @@ for.cond.cleanup:
ret void
}
-; TODO: sret could be specified to not be accessed on unwind either.
-define void @test_sret(ptr noalias sret(i32) %a, i1 zeroext %y) uwtable {
-; CHECK-LABEL: @test_sret(
+define void @test_dead_on_unwind(ptr noalias dead_on_unwind %a, i1 zeroext %y) uwtable {
+; CHECK-LABEL: @test_dead_on_unwind(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[A_PROMOTED:%.*]] = load i32, ptr [[A:%.*]], align 4
; CHECK-NEXT: br label [[FOR_BODY:%.*]]
@@ -157,7 +156,6 @@ define void @test_sret(ptr noalias sret(i32) %a, i1 zeroext %y) uwtable {
; CHECK-NEXT: [[ADD1:%.*]] = phi i32 [ [[A_PROMOTED]], [[ENTRY:%.*]] ], [ [[ADD:%.*]], [[FOR_INC:%.*]] ]
; CHECK-NEXT: [[I_03:%.*]] = phi i32 [ 0, [[ENTRY]] ], [ [[INC:%.*]], [[FOR_INC]] ]
; CHECK-NEXT: [[ADD]] = add nsw i32 [[ADD1]], 1
-; CHECK-NEXT: store i32 [[ADD]], ptr [[A]], align 4
; CHECK-NEXT: br i1 [[Y:%.*]], label [[IF_THEN:%.*]], label [[FOR_INC]]
; CHECK: if.then:
; CHECK-NEXT: tail call void @f()
@@ -167,6 +165,8 @@ define void @test_sret(ptr noalias sret(i32) %a, i1 zeroext %y) uwtable {
; CHECK-NEXT: [[EXITCOND:%.*]] = icmp eq i32 [[INC]], 10000
; CHECK-NEXT: br i1 [[EXITCOND]], label [[FOR_COND_CLEANUP:%.*]], label [[FOR_BODY]]
; CHECK: for.cond.cleanup:
+; CHECK-NEXT: [[ADD_LCSSA:%.*]] = phi i32 [ [[ADD]], [[FOR_INC]] ]
+; CHECK-NEXT: store i32 [[ADD_LCSSA]], ptr [[A]], align 4
; CHECK-NEXT: ret void
;
entry:
diff --git a/llvm/test/Transforms/MemCpyOpt/callslot_throw.ll b/llvm/test/Transforms/MemCpyOpt/callslot_throw.ll
index 8ae188c6fd3d4..912fae074895d 100644
--- a/llvm/test/Transforms/MemCpyOpt/callslot_throw.ll
+++ b/llvm/test/Transforms/MemCpyOpt/callslot_throw.ll
@@ -56,9 +56,23 @@ entry:
ret void
}
-; TODO: With updated semantics, sret could also be invisible on unwind.
-define void @test_sret(ptr nocapture noalias dereferenceable(4) sret(i32) %x) {
-; CHECK-LABEL: @test_sret(
+define void @test_dead_on_unwind(ptr nocapture noalias writable dead_on_unwind dereferenceable(4) %x) {
+; CHECK-LABEL: @test_dead_on_unwind(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[T:%.*]] = alloca i32, align 4
+; CHECK-NEXT: call void @may_throw(ptr nonnull [[X:%.*]])
+; CHECK-NEXT: ret void
+;
+entry:
+ %t = alloca i32, align 4
+ call void @may_throw(ptr nonnull %t)
+ %load = load i32, ptr %t, align 4
+ store i32 %load, ptr %x, align 4
+ ret void
+}
+
+define void @test_dead_on_unwind_missing_writable(ptr nocapture noalias dead_on_unwind dereferenceable(4) %x) {
+; CHECK-LABEL: @test_dead_on_unwind_missing_writable(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[T:%.*]] = alloca i32, align 4
; CHECK-NEXT: call void @may_throw(ptr nonnull [[T]])
diff --git a/llvm/test/Verifier/dead-on-unwind.ll b/llvm/test/Verifier/dead-on-unwind.ll
new file mode 100644
index 0000000000000..a67c002ee6d8f
--- /dev/null
+++ b/llvm/test/Verifier/dead-on-unwind.ll
@@ -0,0 +1,7 @@
+; RUN: not llvm-as -disable-output %s 2>&1 | FileCheck %s
+
+; CHECK: Attribute 'dead_on_unwind' applied to incompatible type!
+; CHECK-NEXT: ptr @not_pointer
+define void @not_pointer(i32 dead_on_unwind %arg) {
+ ret void
+}
|
rnk
approved these changes
Dec 4, 2023
|
LG. Please add a test with |
|
@efriedma-quic Any concerns from your side? |
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.
Add the
dead_on_unwindattribute, which states that the caller will not read from this argument if the call unwinds. This allows eliding stores that could otherwise be visible on the unwind path, for example:The optimization is not valid without
dead_on_unwind, because thei32 0value might be read if@may_unwindunwinds.This attribute is primarily intended to be used on sret arguments. In fact, I previously wanted to change the semantics of sret to include this "no read after unwind" property (see D116998), but based on the feedback there it is better to keep these attributes orthogonal (sret is an ABI attribute, dead_on_unwind is an optimization attribute). This is a reboot of that change with a separate attribute.