Skip to content

Commit 73cde15

Browse files
atrickatrick
authored
Merge pull request #36299 from atrick/poison-destroy
[NFC] Add a poison flag to SIL destroy_value.
2 parents 531b080 + e0a440c commit 73cde15

File tree

13 files changed

+75
-10
lines changed

13 files changed

+75
-10
lines changed

‎docs/SIL.rst

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5197,7 +5197,7 @@ destroy_value
51975197

51985198
::
51995199

5200-
sil-instruction ::= 'destroy_value' sil-operand
5200+
sil-instruction ::= 'destroy_value' '[poison]'? sil-operand
52015201

52025202
destroy_value %0 : $A
52035203

‎include/swift/SIL/SILBuilder.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1245,13 +1245,14 @@ class SILBuilder {
12451245
CopyValueInst(getSILDebugLocation(Loc), operand));
12461246
}
12471247

1248-
DestroyValueInst *createDestroyValue(SILLocation Loc, SILValue operand) {
1248+
DestroyValueInst *createDestroyValue(SILLocation Loc, SILValue operand,
1249+
bool poisonRefs = false) {
12491250
assert(isLoadableOrOpaque(operand->getType()));
12501251
assert(!operand->getType().isTrivial(getFunction()) &&
12511252
"Should not be passing trivial values to this api. Use instead "
12521253
"emitDestroyValueOperation");
1253-
return insert(new (getModule())
1254-
DestroyValueInst(getSILDebugLocation(Loc), operand));
1254+
return insert(new (getModule()) DestroyValueInst(getSILDebugLocation(Loc),
1255+
operand, poisonRefs));
12551256
}
12561257

12571258
UnconditionalCheckedCastInst *

‎include/swift/SIL/SILCloner.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1807,7 +1807,8 @@ void SILCloner<ImplClass>::visitDestroyValueInst(DestroyValueInst *Inst) {
18071807

18081808
recordClonedInstruction(
18091809
Inst, getBuilder().createDestroyValue(getOpLocation(Inst->getLoc()),
1810-
getOpValue(Inst->getOperand())));
1810+
getOpValue(Inst->getOperand()),
1811+
Inst->poisonRefs()));
18111812
}
18121813

18131814
template <typename ImplClass>

‎include/swift/SIL/SILInstruction.h

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7140,8 +7140,17 @@ class DestroyValueInst
71407140
NonValueInstruction> {
71417141
friend class SILBuilder;
71427142

7143-
DestroyValueInst(SILDebugLocation DebugLoc, SILValue operand)
7144-
: UnaryInstructionBase(DebugLoc, operand) {}
7143+
DestroyValueInst(SILDebugLocation DebugLoc, SILValue operand, bool poisonRefs)
7144+
: UnaryInstructionBase(DebugLoc, operand) {
7145+
setPoisonRefs(poisonRefs);
7146+
}
7147+
7148+
public:
7149+
bool poisonRefs() const { return SILNode::Bits.DestroyValueInst.PoisonRefs; }
7150+
7151+
void setPoisonRefs(bool poisonRefs = true) {
7152+
SILNode::Bits.DestroyValueInst.PoisonRefs = poisonRefs;
7153+
}
71457154
};
71467155

71477156
/// Given an object reference, return true iff it is non-nil and refers

‎include/swift/SIL/SILNode.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,9 @@ class alignas(8) SILNode {
338338
Immutable : 1
339339
);
340340

341+
SWIFT_INLINE_BITFIELD(DestroyValueInst, NonValueInstruction, 1,
342+
PoisonRefs : 1);
343+
341344
SWIFT_INLINE_BITFIELD(EndCOWMutationInst, NonValueInstruction, 1,
342345
KeepUnique : 1
343346
);

‎lib/SIL/IR/SILInstruction.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -384,6 +384,11 @@ namespace {
384384
return true;
385385
}
386386

387+
bool visitDestroyValueInst(const DestroyValueInst *RHS) {
388+
auto *left = cast<DestroyValueInst>(LHS);
389+
return left->poisonRefs() == RHS->poisonRefs();
390+
}
391+
387392
bool visitBeginCOWMutationInst(const BeginCOWMutationInst *RHS) {
388393
auto *left = cast<BeginCOWMutationInst>(LHS);
389394
return left->isNative() == RHS->isNative();

‎lib/SIL/IR/SILPrinter.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1715,6 +1715,8 @@ class SILPrinter : public SILInstructionVisitor<SILPrinter> {
17151715
#include "swift/AST/ReferenceStorage.def"
17161716

17171717
void visitDestroyValueInst(DestroyValueInst *I) {
1718+
if (I->poisonRefs())
1719+
*this << "[poison] ";
17181720
*this << getIDAndType(I->getOperand());
17191721
}
17201722

‎lib/SIL/Parser/ParseSIL.cpp

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3030,7 +3030,6 @@ bool SILParser::parseSpecificSILInstruction(SILBuilder &B,
30303030
UNARY_INSTRUCTION(IsUnique)
30313031
UNARY_INSTRUCTION(DestroyAddr)
30323032
UNARY_INSTRUCTION(CopyValue)
3033-
UNARY_INSTRUCTION(DestroyValue)
30343033
UNARY_INSTRUCTION(EndBorrow)
30353034
UNARY_INSTRUCTION(DestructureStruct)
30363035
UNARY_INSTRUCTION(DestructureTuple)
@@ -3057,6 +3056,14 @@ bool SILParser::parseSpecificSILInstruction(SILBuilder &B,
30573056
#undef UNARY_INSTRUCTION
30583057
#undef REFCOUNTING_INSTRUCTION
30593058

3059+
case SILInstructionKind::DestroyValueInst: {
3060+
bool poisonRefs = false;
3061+
if (parseSILOptional(poisonRefs, *this, "poison")
3062+
|| parseTypedValueRef(Val, B) || parseSILDebugLocation(InstLoc, B))
3063+
return true;
3064+
ResultVal = B.createDestroyValue(InstLoc, Val, poisonRefs);
3065+
break;
3066+
}
30603067
case SILInstructionKind::BeginCOWMutationInst: {
30613068
bool native = false;
30623069
if (parseSILOptional(native, *this, "native") ||

‎lib/SIL/Verifier/SILVerifier.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2482,7 +2482,8 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
24822482
"Source value should be an object value");
24832483
require(!I->getOperand()->getType().isTrivial(*I->getFunction()),
24842484
"Source value should be non-trivial");
2485-
require(!fnConv.useLoweredAddresses() || F.hasOwnership(),
2485+
require(I->poisonRefs() || !fnConv.useLoweredAddresses()
2486+
|| F.hasOwnership(),
24862487
"destroy_value is only valid in functions with qualified "
24872488
"ownership");
24882489
}

‎lib/Serialization/DeserializeSIL.cpp

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1858,7 +1858,6 @@ bool SILDeserializer::readSILInstruction(SILFunction *Fn,
18581858
REFCOUNTING_INSTRUCTION(RetainValueAddr)
18591859
REFCOUNTING_INSTRUCTION(UnmanagedRetainValue)
18601860
UNARY_INSTRUCTION(CopyValue)
1861-
UNARY_INSTRUCTION(DestroyValue)
18621861
REFCOUNTING_INSTRUCTION(ReleaseValue)
18631862
REFCOUNTING_INSTRUCTION(ReleaseValueAddr)
18641863
REFCOUNTING_INSTRUCTION(UnmanagedReleaseValue)
@@ -1898,6 +1897,17 @@ bool SILDeserializer::readSILInstruction(SILFunction *Fn,
18981897
break;
18991898
}
19001899

1900+
case SILInstructionKind::DestroyValueInst: {
1901+
assert(RecordKind == SIL_ONE_OPERAND && "Layout should be OneOperand.");
1902+
unsigned poisonRefs = Attr;
1903+
ResultInst = Builder.createDestroyValue(
1904+
Loc,
1905+
getLocalValue(ValID, getSILType(MF->getType(TyID),
1906+
(SILValueCategory)TyCategory, Fn)),
1907+
poisonRefs != 0);
1908+
break;
1909+
}
1910+
19011911
case SILInstructionKind::BeginCOWMutationInst: {
19021912
assert(RecordKind == SIL_ONE_OPERAND && "Layout should be OneOperand.");
19031913
unsigned isNative = Attr;

‎lib/Serialization/SerializeSIL.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1399,6 +1399,8 @@ void SILSerializer::writeSILInstruction(const SILInstruction &SI) {
13991399
Attr = unsigned(SILValue(UOCI).getOwnershipKind());
14001400
} else if (auto *IEC = dyn_cast<IsEscapingClosureInst>(&SI)) {
14011401
Attr = IEC->getVerificationType();
1402+
} else if (auto *DVI = dyn_cast<DestroyValueInst>(&SI)) {
1403+
Attr = DVI->poisonRefs();
14021404
} else if (auto *BCMI = dyn_cast<BeginCOWMutationInst>(&SI)) {
14031405
Attr = BCMI->isNative();
14041406
} else if (auto *ECMI = dyn_cast<EndCOWMutationInst>(&SI)) {

‎test/SIL/Parser/basic2.sil

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,3 +31,15 @@ bb0(%0 : $@sil_unmanaged Builtin.NativeObject):
3131
%1 = strong_copy_unmanaged_value %0 : $@sil_unmanaged Builtin.NativeObject
3232
return %1 : $Builtin.NativeObject
3333
}
34+
35+
// CHECK-LABEL: sil [ossa] @test_poison
36+
// CHECK: bb0([[T0:%[0-9]+]] : @owned $Builtin.NativeObject):
37+
// CHECK-NEXT: destroy_value [poison] [[T0]] : $Builtin.NativeObject
38+
// CHECK-NEXT: tuple
39+
// CHECK-NEXT: return
40+
sil [ossa] @test_poison : $@convention(thin) (@owned Builtin.NativeObject) -> () {
41+
bb0(%0 : @owned $Builtin.NativeObject):
42+
destroy_value [poison] %0 : $Builtin.NativeObject
43+
%2 = tuple ()
44+
return %2 : $()
45+
}

‎test/SIL/Serialization/copy_value_destroy_value.sil

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,17 @@ bb0(%0 : @owned $Builtin.NativeObject):
2020
return %1 : $Builtin.NativeObject
2121
}
2222

23+
// CHECK-LABEL: sil [serialized] [ossa] @test_poison
24+
// CHECK: bb0(%0 : @owned $Builtin.NativeObject):
25+
// CHECK-NEXT: destroy_value [poison] %0 : $Builtin.NativeObject
26+
// CHECK-NEXT: tuple
27+
// CHECK-NEXT: return
28+
sil [serialized] [ossa] @test_poison : $@convention(thin) (@owned Builtin.NativeObject) -> () {
29+
bb0(%0 : @owned $Builtin.NativeObject):
30+
destroy_value [poison] %0 : $Builtin.NativeObject
31+
%2 = tuple ()
32+
return %2 : $()
33+
}
2334

2435
// CHECK-LABEL: sil [serialized] [ossa] @test_strong_copy_unowned_value : $@convention(thin) (@owned @sil_unowned Builtin.NativeObject) -> @owned Builtin.NativeObject {
2536
// CHECK: bb0([[T0:%[0-9]+]] : @owned $@sil_unowned Builtin.NativeObject):
@@ -32,3 +43,4 @@ bb0(%0 : @owned $@sil_unowned Builtin.NativeObject):
3243
destroy_value %0 : $@sil_unowned Builtin.NativeObject
3344
return %1 : $Builtin.NativeObject
3445
}
46+

0 commit comments

Comments
 (0)