Skip to content

Commit 4016e25

Browse files
nate-chandlernate-chandler
authored
Merge pull request #78651 from nate-chandler/rdar142636711
[SIL] Only visit final partial_apply [on_stack] lifetime ends.
2 parents 4fb0399 + 41bee47 commit 4016e25

File tree

5 files changed

+283
-113
lines changed

5 files changed

+283
-113
lines changed

‎lib/SIL/IR/SILInstruction.cpp‎

Lines changed: 132 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,17 @@
1515
//===----------------------------------------------------------------------===//
1616

1717
#include "swift/SIL/SILInstruction.h"
18-
#include "swift/Basic/Assertions.h"
1918
#include "swift/Basic/AssertImplements.h"
19+
#include "swift/Basic/Assertions.h"
2020
#include "swift/Basic/Unicode.h"
2121
#include "swift/Basic/type_traits.h"
2222
#include "swift/SIL/ApplySite.h"
2323
#include "swift/SIL/DynamicCasts.h"
24+
#include "swift/SIL/InstWrappers.h"
2425
#include "swift/SIL/InstructionUtils.h"
26+
#include "swift/SIL/NodeDatastructures.h"
2527
#include "swift/SIL/OwnershipUtils.h"
28+
#include "swift/SIL/PrunedLiveness.h"
2629
#include "swift/SIL/SILBuilder.h"
2730
#include "swift/SIL/SILCloner.h"
2831
#include "swift/SIL/SILDebugScope.h"
@@ -1828,6 +1831,134 @@ bool SILInstruction::maySuspend() const {
18281831
return false;
18291832
}
18301833

1834+
static SILValue lookThroughOwnershipAndForwardingInsts(SILValue value) {
1835+
auto current = value;
1836+
while (true) {
1837+
if (auto *inst = current->getDefiningInstruction()) {
1838+
switch (inst->getKind()) {
1839+
case SILInstructionKind::MoveValueInst:
1840+
case SILInstructionKind::CopyValueInst:
1841+
case SILInstructionKind::BeginBorrowInst:
1842+
current = inst->getOperand(0);
1843+
continue;
1844+
default:
1845+
break;
1846+
}
1847+
auto forward = ForwardingOperation(inst);
1848+
Operand *op = nullptr;
1849+
if (forward && (op = forward.getSingleForwardingOperand())) {
1850+
current = op->get();
1851+
continue;
1852+
}
1853+
} else if (auto *result = SILArgument::isTerminatorResult(current)) {
1854+
auto *op = result->forwardedTerminatorResultOperand();
1855+
if (!op) {
1856+
break;
1857+
}
1858+
current = op->get();
1859+
continue;
1860+
}
1861+
break;
1862+
}
1863+
return current;
1864+
}
1865+
1866+
bool
1867+
PartialApplyInst::visitOnStackLifetimeEnds(
1868+
llvm::function_ref<bool (Operand *)> func) const {
1869+
assert(getFunction()->hasOwnership()
1870+
&& isOnStack()
1871+
&& "only meaningful for OSSA stack closures");
1872+
bool noUsers = true;
1873+
1874+
auto *function = getFunction();
1875+
1876+
SmallVector<SILBasicBlock *, 32> discoveredBlocks;
1877+
SSAPrunedLiveness liveness(function, &discoveredBlocks);
1878+
liveness.initializeDef(this);
1879+
1880+
StackList<SILValue> values(function);
1881+
values.push_back(this);
1882+
1883+
while (!values.empty()) {
1884+
auto value = values.pop_back_val();
1885+
for (auto *use : value->getUses()) {
1886+
if (!use->isConsuming()) {
1887+
if (auto *cvi = dyn_cast<CopyValueInst>(use->getUser())) {
1888+
values.push_back(cvi);
1889+
}
1890+
continue;
1891+
}
1892+
noUsers = false;
1893+
if (isa<DestroyValueInst>(use->getUser())) {
1894+
liveness.updateForUse(use->getUser(), /*lifetimeEnding=*/true);
1895+
continue;
1896+
}
1897+
auto forward = ForwardingOperand(use);
1898+
if (!forward) {
1899+
// There shouldn't be any non-forwarding consumptions of a nonescaping
1900+
// partial_apply that don't forward it along, aside from destroy_value.
1901+
//
1902+
// On-stack partial_apply cannot be cloned, so it should never be used
1903+
// by a BranchInst.
1904+
//
1905+
// This is a fatal error because it performs SIL verification that is
1906+
// not separately checked in the verifier. It is the only check that
1907+
// verifies the structural requirements of on-stack partial_apply uses.
1908+
if (lookThroughOwnershipAndForwardingInsts(use->get()) !=
1909+
SILValue(this)) {
1910+
// Consumes of values which aren't "essentially" the
1911+
// partial_apply [on_stack]
1912+
// are okay. For example, a not-on_stack partial_apply that captures
1913+
// it.
1914+
continue;
1915+
}
1916+
llvm::errs() << "partial_apply [on_stack] use:\n";
1917+
auto *user = use->getUser();
1918+
user->printInContext(llvm::errs());
1919+
if (isa<BranchInst>(user)) {
1920+
llvm::report_fatal_error("partial_apply [on_stack] cannot be cloned");
1921+
}
1922+
llvm::report_fatal_error("partial_apply [on_stack] must be directly "
1923+
"forwarded to a destroy_value");
1924+
}
1925+
forward.visitForwardedValues([&values](auto value) {
1926+
values.push_back(value);
1927+
return true;
1928+
});
1929+
}
1930+
}
1931+
PrunedLivenessBoundary boundary;
1932+
liveness.computeBoundary(boundary);
1933+
1934+
for (auto *inst : boundary.lastUsers) {
1935+
// Only destroy_values were added to liveness, so only destroy_values can be
1936+
// the last users.
1937+
auto *dvi = cast<DestroyValueInst>(inst);
1938+
auto keepGoing = func(&dvi->getOperandRef());
1939+
if (!keepGoing) {
1940+
return false;
1941+
}
1942+
}
1943+
return !noUsers;
1944+
}
1945+
1946+
namespace swift::test {
1947+
FunctionTest PartialApplyPrintOnStackLifetimeEnds(
1948+
"partial_apply_print_on_stack_lifetime_ends",
1949+
[](auto &function, auto &arguments, auto &test) {
1950+
auto *inst = arguments.takeInstruction();
1951+
auto *pai = cast<PartialApplyInst>(inst);
1952+
function.print(llvm::outs());
1953+
auto result = pai->visitOnStackLifetimeEnds([](auto *operand) {
1954+
operand->print(llvm::outs());
1955+
return true;
1956+
});
1957+
const char *resultString = result ? "true" : "false";
1958+
llvm::outs() << "returned: " << resultString << "\n";
1959+
});
1960+
} // end namespace swift::test
1961+
18311962
static bool
18321963
visitRecursivelyLifetimeEndingUses(
18331964
SILValue i, bool &noUsers,
@@ -1869,41 +2000,6 @@ visitRecursivelyLifetimeEndingUses(
18692000
return true;
18702001
}
18712002

1872-
bool
1873-
PartialApplyInst::visitOnStackLifetimeEnds(
1874-
llvm::function_ref<bool (Operand *)> func) const {
1875-
assert(getFunction()->hasOwnership()
1876-
&& isOnStack()
1877-
&& "only meaningful for OSSA stack closures");
1878-
bool noUsers = true;
1879-
1880-
auto visitUnknownUse = [](Operand *unknownUse) {
1881-
// There shouldn't be any dead-end consumptions of a nonescaping
1882-
// partial_apply that don't forward it along, aside from destroy_value.
1883-
//
1884-
// On-stack partial_apply cannot be cloned, so it should never be used by a
1885-
// BranchInst.
1886-
//
1887-
// This is a fatal error because it performs SIL verification that is not
1888-
// separately checked in the verifier. It is the only check that verifies
1889-
// the structural requirements of on-stack partial_apply uses.
1890-
llvm::errs() << "partial_apply [on_stack] use:\n";
1891-
auto *user = unknownUse->getUser();
1892-
user->printInContext(llvm::errs());
1893-
if (isa<BranchInst>(user)) {
1894-
llvm::report_fatal_error("partial_apply [on_stack] cannot be cloned");
1895-
}
1896-
llvm::report_fatal_error("partial_apply [on_stack] must be directly "
1897-
"forwarded to a destroy_value");
1898-
return false;
1899-
};
1900-
if (!visitRecursivelyLifetimeEndingUses(this, noUsers, func,
1901-
visitUnknownUse)) {
1902-
return false;
1903-
}
1904-
return !noUsers;
1905-
}
1906-
19072003
// FIXME: Rather than recursing through all results, this should only recurse
19082004
// through ForwardingInstruction and OwnershipTransitionInstruction and the
19092005
// client should prove that any other uses cannot be upstream from a consume of

‎lib/SIL/Utils/OwnershipLiveness.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -414,7 +414,7 @@ static FunctionTest LinearLivenessTest("linear_liveness", [](auto &function,
414414
// - the computed pruned liveness
415415
// - the liveness boundary
416416
static FunctionTest
417-
InteriorLivenessTest("interior-liveness",
417+
InteriorLivenessTest("interior_liveness",
418418
[](auto &function, auto &arguments, auto &test) {
419419
SILValue value = arguments.takeValue();
420420
function.print(llvm::outs());

‎test/SIL/Instruction/partial_apply_on_stack_lifetime_ends.sil‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
// RUN: %target-sil-opt \
2+
// RUN: -test-runner \
3+
// RUN: %s \
4+
// RUN: -o /dev/null \
5+
// RUN: 2>&1 | %FileCheck %s
6+
7+
sil_stage canonical
8+
9+
class C {}
10+
11+
sil @borrowC : $@convention(thin) (@guaranteed C) -> ()
12+
13+
// CHECK-LABEL: begin running test {{.*}} on copied_partial_apply
14+
// CHECK-LABEL: sil [ossa] @copied_partial_apply : {{.*}} {
15+
// CHECK: bb0([[C:%[^,]+]] :
16+
// CHECK: [[BORROW_C:%[^,]+]] = function_ref @borrowC
17+
// CHECK: [[PA:%[^,]+]] = partial_apply [callee_guaranteed] [on_stack] [[BORROW_C]]([[C]])
18+
// CHECK: [[PA2:%[^,]+]] = copy_value [[PA]]
19+
// CHECK: destroy_value [[PA]]
20+
// CHECK: destroy_value [[PA2]]
21+
// CHECK: destroy_value [[C]]
22+
// CHECK-LABEL: } // end sil function 'copied_partial_apply'
23+
// CHECK: Operand.
24+
// CHECK: Owner: destroy_value [[PA2]]
25+
// CHECK: returned: true
26+
// CHECK-LABEL: end running test {{.*}} on copied_partial_apply
27+
sil [ossa] @copied_partial_apply : $@convention(thin) (@owned C) -> () {
28+
entry(%c: @owned $C):
29+
specify_test "partial_apply_print_on_stack_lifetime_ends %pa"
30+
31+
%callee = function_ref @borrowC : $@convention(thin) (@guaranteed C) -> ()
32+
%pa = partial_apply [callee_guaranteed] [on_stack] %callee(%c) : $@convention(thin) (@guaranteed C) -> ()
33+
%pa2 = copy_value %pa
34+
destroy_value %pa
35+
36+
destroy_value %pa2
37+
destroy_value %c
38+
%retval = tuple ()
39+
return %retval
40+
}

‎test/SILOptimizer/canonicalize_ossa_lifetime_unit.sil‎

Lines changed: 43 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -162,9 +162,9 @@ exit(%phi : @owned $C, %typhi : $S):
162162
sil @empty : $@convention(thin) () -> () {
163163
[global: ]
164164
bb0:
165-
%0 = tuple ()
166-
return %0 : $()
167-
}
165+
%0 = tuple ()
166+
return %0 : $()
167+
}
168168

169169
// Even though the apply of %empty is not a deinit barrier, verify that the
170170
// destroy is not hoisted, because MoS is move-only.
@@ -568,16 +568,16 @@ entry(%c1 : @owned $C):
568568
// CHECK: bb0([[C1:%[^,]+]] : @owned $C):
569569
// CHECK: [[TAKE_C:%[^,]+]] = function_ref @takeC
570570
// CHECK: [[BARRIER:%[^,]+]] = function_ref @barrier
571-
// CHECK: cond_br undef, [[LEFT]], [[RIGHT]]
572-
// CHECK: [[LEFT]]:
571+
// CHECK: cond_br undef, [[LEFT]], [[RIGHT]]
572+
// CHECK: [[LEFT]]:
573573
// CHECK: [[M:%[^,]+]] = move_value [[C1]]
574574
// CHECK: apply [[TAKE_C]]([[M]])
575-
// CHECK: br [[EXIT]]
576-
// CHECK: [[RIGHT]]:
575+
// CHECK: br [[EXIT]]
576+
// CHECK: [[RIGHT]]:
577577
// CHECK: apply [[BARRIER]]()
578578
// CHECK: destroy_value [[C1]]
579-
// CHECK: br [[EXIT]]
580-
// CHECK: [[EXIT]]:
579+
// CHECK: br [[EXIT]]
580+
// CHECK: [[EXIT]]:
581581
// CHECK: apply [[BARRIER]]()
582582
// CHECK-LABEL: } // end sil function 'lexical_end_at_end_2'
583583
// CHECK-LABEL: end running test {{.*}} on lexical_end_at_end_2: canonicalize_ossa_lifetime
@@ -884,3 +884,37 @@ die:
884884
apply undef(%reload) : $@convention(thin) (@guaranteed C) -> ()
885885
unreachable
886886
}
887+
888+
// The destroy of a value must not be hoisted over a destroy of a copy of a
889+
// partial_apply [on_stack] which captures the value.
890+
// CHECK-LABEL: begin running test {{.*}} on destroy_after_pa_copy_destroy
891+
// CHECK-LABEL: sil [ossa] @destroy_after_pa_copy_destroy : {{.*}} {
892+
// CHECK: bb0([[A:%[^,]+]] :
893+
// CHECK: [[C:%[^,]+]] = move_value [[A]]
894+
// CHECK: [[BORROW_C:%[^,]+]] = function_ref @borrowC
895+
// CHECK: [[PA:%[^,]+]] = partial_apply [callee_guaranteed] [on_stack] [[BORROW_C]]([[C]])
896+
// CHECK: [[PA2:%[^,]+]] = copy_value [[PA]]
897+
// CHECK: destroy_value [[PA]]
898+
// CHECK: destroy_value [[PA2]]
899+
// CHECK: destroy_value [[C]]
900+
// CHECK: apply undef()
901+
// CHECK-LABEL: } // end sil function 'destroy_after_pa_copy_destroy'
902+
// CHECK-LABEL: end running test {{.*}} on destroy_after_pa_copy_destroy
903+
sil [ossa] @destroy_after_pa_copy_destroy : $@convention(thin) (@owned C) -> () {
904+
entry(%a: @owned $C):
905+
// To defeat deinit barriers (apply undef).
906+
%c = move_value %a
907+
specify_test "canonicalize_ossa_lifetime true false true %c"
908+
909+
%callee = function_ref @borrowC : $@convention(thin) (@guaranteed C) -> ()
910+
%pa = partial_apply [callee_guaranteed] [on_stack] %callee(%c) : $@convention(thin) (@guaranteed C) -> ()
911+
%pa2 = copy_value %pa
912+
destroy_value %pa
913+
914+
destroy_value %pa2
915+
// To defeat ignoredByDestroyHoisting.
916+
apply undef() : $@convention(thin) () -> ()
917+
destroy_value %c
918+
%retval = tuple ()
919+
return %retval
920+
}

0 commit comments

Comments
 (0)