Skip to content

Commit 4f9e008

Browse files
committed
Add verification to make sure guaranteed forwarding phi has guaranteed forwarding operands on all paths
1 parent cd66ca7 commit 4f9e008

File tree

3 files changed

+70
-1
lines changed

3 files changed

+70
-1
lines changed

lib/SIL/Verifier/SILOwnershipVerifier.cpp

Lines changed: 42 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
@@ -624,6 +626,39 @@ bool SILValueOwnershipChecker::isSubobjectProjectionWithLifetimeEndingUses(
624626
});
625627
}
626628

629+
bool SILValueOwnershipChecker::
630+
hasGuaranteedForwardingIncomingPhiOperandsOnZeroOrAllPaths(
631+
SILPhiArgument *phi) const {
632+
bool foundGuaranteedForwardingPhiOperand = false;
633+
bool foundNonGuaranteedForwardingPhiOperand = false;
634+
phi->visitTransitiveIncomingPhiOperands([&](auto *, auto *operand) -> bool {
635+
auto value = operand->get();
636+
if (canOpcodeForwardGuaranteedValues(value) ||
637+
isa<SILFunctionArgument>(value)) {
638+
foundGuaranteedForwardingPhiOperand = true;
639+
if (foundNonGuaranteedForwardingPhiOperand) {
640+
return false; /* found error, stop visiting */
641+
}
642+
return true;
643+
}
644+
foundNonGuaranteedForwardingPhiOperand = true;
645+
if (foundGuaranteedForwardingPhiOperand) {
646+
return false; /* found error, stop visiting */
647+
}
648+
return true;
649+
});
650+
if (foundGuaranteedForwardingPhiOperand ^
651+
foundNonGuaranteedForwardingPhiOperand) {
652+
return true;
653+
}
654+
return errorBuilder.handleMalformedSIL([&] {
655+
llvm::errs() << "Malformed @guaranteed phi!\n"
656+
<< "Phi: " << *phi;
657+
llvm::errs() << "Guaranteed forwarding operand on one path and reborrow "
658+
"on another!\n";
659+
});
660+
}
661+
627662
bool SILValueOwnershipChecker::checkUses() {
628663
LLVM_DEBUG(llvm::dbgs() << " Gathering and classifying uses!\n");
629664

@@ -686,6 +721,13 @@ bool SILValueOwnershipChecker::checkUses() {
686721
return false;
687722
}
688723
}
724+
auto *phi = dyn_cast<SILPhiArgument>(value);
725+
if (phi && phi->isPhi() &&
726+
phi->getOwnershipKind() == OwnershipKind::Guaranteed) {
727+
if (!hasGuaranteedForwardingIncomingPhiOperandsOnZeroOrAllPaths(phi)) {
728+
return false;
729+
}
730+
}
689731

690732
if (isa<LoadBorrowInst>(value) || isa<BeginBorrowInst>(value)) {
691733
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 operand on one path and reborrow on another!
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)