Skip to content

Commit 35d45de

Browse files
committed
Add verification to make sure guaranteed forwarding phi has guaranteed forwarding operands on all paths
1 parent 511739b commit 35d45de

File tree

3 files changed

+69
-1
lines changed

3 files changed

+69
-1
lines changed

lib/SIL/Verifier/SILOwnershipVerifier.cpp

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,8 @@ class SILValueOwnershipChecker {
145145
bool isSubobjectProjectionWithLifetimeEndingUses(
146146
SILValue value,
147147
const SmallVectorImpl<Operand *> &lifetimeEndingUsers) const;
148+
bool hasGuaranteedForwardingIncomingPhiOperandsOnZeroOrAllPaths(
149+
SILPhiArgument *phi) const;
148150
};
149151

150152
} // namespace swift
@@ -615,6 +617,38 @@ bool SILValueOwnershipChecker::isSubobjectProjectionWithLifetimeEndingUses(
615617
});
616618
}
617619

620+
bool SILValueOwnershipChecker::
621+
hasGuaranteedForwardingIncomingPhiOperandsOnZeroOrAllPaths(
622+
SILPhiArgument *phi) const {
623+
bool foundGuaranteedForwardingPhiOperand = false;
624+
bool foundNonGuaranteedForwardingPhiOperand = false;
625+
phi->visitTransitiveIncomingPhiOperands([&](auto *, auto *operand) -> bool {
626+
auto value = operand->get();
627+
if (canOpcodeForwardInnerGuaranteedValues(value) ||
628+
isa<SILFunctionArgument>(value)) {
629+
foundGuaranteedForwardingPhiOperand = true;
630+
if (foundNonGuaranteedForwardingPhiOperand) {
631+
return false; /* found error, stop visiting */
632+
}
633+
return true;
634+
}
635+
foundNonGuaranteedForwardingPhiOperand = true;
636+
if (foundGuaranteedForwardingPhiOperand) {
637+
return false; /* found error, stop visiting */
638+
}
639+
return true;
640+
});
641+
if (foundGuaranteedForwardingPhiOperand ^
642+
foundNonGuaranteedForwardingPhiOperand) {
643+
return true;
644+
}
645+
return errorBuilder.handleMalformedSIL([&] {
646+
llvm::errs() << "Malformed @guaranteed phi!\n"
647+
<< "Phi: " << *phi;
648+
llvm::errs() << "Guaranteed forwarding operands not found on all paths!\n";
649+
});
650+
}
651+
618652
bool SILValueOwnershipChecker::checkUses() {
619653
LLVM_DEBUG(llvm::dbgs() << " Gathering and classifying uses!\n");
620654

@@ -677,6 +711,13 @@ bool SILValueOwnershipChecker::checkUses() {
677711
return false;
678712
}
679713
}
714+
auto *phi = dyn_cast<SILPhiArgument>(value);
715+
if (phi && phi->isPhi() &&
716+
phi->getOwnershipKind() == OwnershipKind::Guaranteed) {
717+
if (!hasGuaranteedForwardingIncomingPhiOperandsOnZeroOrAllPaths(phi)) {
718+
return false;
719+
}
720+
}
680721

681722
if (isa<LoadBorrowInst>(value) || isa<BeginBorrowInst>(value)) {
682723
guaranteedPhiVerifier.verifyGuaranteedForwardingPhis(BorrowedValue(value));

test/SIL/OwnershipVerifier/guaranteed_phis_errors.sil

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,32 @@ bb2(%phi : @guaranteed $SuperKlass):
180180
return %9999 : $()
181181
}
182182

183+
// CHECK: Error#: 0. Begin Error in Function: 'test7'
184+
// CHECK: Malformed @guaranteed phi!
185+
// CHECK: Phi: %7 = argument of bb3 : $SuperKlass
186+
// CHECK: Guaranteed forwarding operands not found on all paths!
187+
// CHECK: Error#: 0. End Error in Function: 'test7'
188+
sil [ossa] @test7 : $@convention(thin) (@guaranteed Klass, @owned SuperKlass) -> () {
189+
bb0(%0 : @guaranteed $Klass, %1 : @owned $SuperKlass):
190+
cond_br undef, bb1, bb2
191+
192+
bb1:
193+
%3 = unchecked_ref_cast %0 : $Klass to $SuperKlass
194+
br bb3(%3 : $SuperKlass)
195+
196+
bb2:
197+
%5 = begin_borrow %1 : $SuperKlass
198+
br bb3(%5 : $SuperKlass)
199+
200+
bb3(%7 : @guaranteed $SuperKlass):
201+
%8 = function_ref @use_superklass : $@convention(thin) (@guaranteed SuperKlass) -> ()
202+
%9 = apply %8(%7) : $@convention(thin) (@guaranteed SuperKlass) -> ()
203+
end_borrow %7 : $SuperKlass
204+
destroy_value %1 : $SuperKlass
205+
%12 = tuple ()
206+
return %12 : $()
207+
}
208+
183209
enum FakeOptional<T> {
184210
case none
185211
case some(T)

test/SILOptimizer/copy_propagation_canonicalize_with_reborrows.sil

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,8 @@ sil [ossa] @nohoist_destroy_over_reborrow_loop : $@convention(thin) () -> () {
149149
entry:
150150
%none1 = enum $FakeOptional<X>, #FakeOptional.none!enumelt
151151
%none2 = enum $FakeOptional<X>, #FakeOptional.none!enumelt
152-
br head(%none2 : $FakeOptional<X>, %none1 : $FakeOptional<X>)
152+
%none_borrow_2 = begin_borrow %none2 : $FakeOptional<X>
153+
br head(%none_borrow_2 : $FakeOptional<X>, %none1 : $FakeOptional<X>)
153154

154155
head(%reborrow : @guaranteed $FakeOptional<X>, %value : @owned $FakeOptional<X>):
155156
cond_br undef, body, exit

0 commit comments

Comments
 (0)