Skip to content

Commit aaad438

Browse files
nate-chandlernate-chandler
committed
[SIL] Only visit final on-stack pai lifetime ends.
In OSSA, the `partial_apply [on_stack]` instruction produces a value with odd characteristics that correspond to the fact that it is lowered to a stack-allocating instruction. Among these characteristics is the fact that copies of such values aren't load bearing. When visiting the lifetime-ending uses of a `partial_apply [on_stack]` the lifetime ending uses of (transitive) copies of the partial_apply must be considered as well. Otherwise, the borrow scope it defins may be incorrectly short: ``` %closure = partial_apply %fn(%value) // borrows %value %closure2 = copy_value %closure destroy_value %closure // does _not_ end borrow of %value! ... destroy_value %closure2 // ends borrow of %value ... destroy_value %value ``` Furthermore, _only_ the final such destroys actually count as the real lifetime ends. At least one client (OME) relies on `visitOnStackLifetimeEnds` visiting at most a single lifetime end on any path. Rewrite the utility to use PrunedLiveness, tracking only destroys of copies and forwards. The final destroys are the destroys on the boundary. rdar://142636711
1 parent 942ccd6 commit aaad438

File tree

3 files changed

+113
-34
lines changed

3 files changed

+113
-34
lines changed

‎lib/SIL/IR/SILInstruction.cpp

Lines changed: 69 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,16 @@
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"
2424
#include "swift/SIL/InstructionUtils.h"
25+
#include "swift/SIL/NodeDatastructures.h"
2526
#include "swift/SIL/OwnershipUtils.h"
27+
#include "swift/SIL/PrunedLiveness.h"
2628
#include "swift/SIL/SILBuilder.h"
2729
#include "swift/SIL/SILCloner.h"
2830
#include "swift/SIL/SILDebugScope.h"
@@ -1876,29 +1878,72 @@ PartialApplyInst::visitOnStackLifetimeEnds(
18761878
&& "only meaningful for OSSA stack closures");
18771879
bool noUsers = true;
18781880

1879-
auto visitUnknownUse = [](Operand *unknownUse) {
1880-
// There shouldn't be any dead-end consumptions of a nonescaping
1881-
// partial_apply that don't forward it along, aside from destroy_value.
1882-
//
1883-
// On-stack partial_apply cannot be cloned, so it should never be used by a
1884-
// BranchInst.
1885-
//
1886-
// This is a fatal error because it performs SIL verification that is not
1887-
// separately checked in the verifier. It is the only check that verifies
1888-
// the structural requirements of on-stack partial_apply uses.
1889-
llvm::errs() << "partial_apply [on_stack] use:\n";
1890-
auto *user = unknownUse->getUser();
1891-
user->printInContext(llvm::errs());
1892-
if (isa<BranchInst>(user)) {
1893-
llvm::report_fatal_error("partial_apply [on_stack] cannot be cloned");
1894-
}
1895-
llvm::report_fatal_error("partial_apply [on_stack] must be directly "
1896-
"forwarded to a destroy_value");
1897-
return false;
1898-
};
1899-
if (!visitRecursivelyLifetimeEndingUses(this, noUsers, func,
1900-
visitUnknownUse)) {
1901-
return false;
1881+
auto *function = getFunction();
1882+
1883+
SmallVector<SILBasicBlock *, 32> discoveredBlocks;
1884+
SSAPrunedLiveness liveness(function, &discoveredBlocks);
1885+
liveness.initializeDef(this);
1886+
1887+
StackList<SILValue> values(function);
1888+
values.push_back(this);
1889+
ValueSet seenValues(function);
1890+
1891+
while (!values.empty()) {
1892+
auto value = values.pop_back_val();
1893+
seenValues.insert(value);
1894+
for (auto *use : value->getUses()) {
1895+
if (!use->isConsuming()) {
1896+
if (auto *cvi = dyn_cast<CopyValueInst>(use->getUser())) {
1897+
values.push_back(cvi);
1898+
}
1899+
continue;
1900+
}
1901+
noUsers = false;
1902+
if (isa<DestroyValueInst>(use->getUser())) {
1903+
liveness.updateForUse(use->getUser(), /*lifetimeEnding=*/true);
1904+
continue;
1905+
}
1906+
auto forward = ForwardingOperand(use);
1907+
if (!forward) {
1908+
// There shouldn't be any non-forwarding consumptions of a nonescaping
1909+
// partial_apply that don't forward it along, aside from destroy_value.
1910+
//
1911+
// On-stack partial_apply cannot be cloned, so it should never be used
1912+
// by a BranchInst.
1913+
//
1914+
// This is a fatal error because it performs SIL verification that is
1915+
// not separately checked in the verifier. It is the only check that
1916+
// verifies the structural requirements of on-stack partial_apply uses.
1917+
if (isa<StoreInst>(use->getUser()) && use->get() != SILValue(this)) {
1918+
// Stores of copies are okay. They are lowered to copy_addrs.
1919+
continue;
1920+
}
1921+
llvm::errs() << "partial_apply [on_stack] use:\n";
1922+
auto *user = use->getUser();
1923+
user->printInContext(llvm::errs());
1924+
if (isa<BranchInst>(user)) {
1925+
llvm::report_fatal_error("partial_apply [on_stack] cannot be cloned");
1926+
}
1927+
llvm::report_fatal_error("partial_apply [on_stack] must be directly "
1928+
"forwarded to a destroy_value");
1929+
}
1930+
forward.visitForwardedValues([&values](auto value) {
1931+
values.push_back(value);
1932+
return true;
1933+
});
1934+
}
1935+
}
1936+
PrunedLivenessBoundary boundary;
1937+
liveness.computeBoundary(boundary);
1938+
1939+
for (auto *inst : boundary.lastUsers) {
1940+
// Only destroy_values were added to liveness, so only destroy_values can be
1941+
// the last users.
1942+
auto *dvi = cast<DestroyValueInst>(inst);
1943+
auto keepGoing = func(&dvi->getOperandRef());
1944+
if (!keepGoing) {
1945+
return false;
1946+
}
19021947
}
19031948
return !noUsers;
19041949
}

‎test/SIL/Instruction/partial_apply_on_stack_lifetime_ends.sil

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ sil @borrowC : $@convention(thin) (@guaranteed C) -> ()
2121
// CHECK: destroy_value [[C]]
2222
// CHECK-LABEL: } // end sil function 'copied_partial_apply'
2323
// CHECK: Operand.
24-
// CHECK: Owner: destroy_value [[PA]]
24+
// CHECK: Owner: destroy_value [[PA2]]
2525
// CHECK: returned: true
2626
// CHECK-LABEL: end running test {{.*}} on copied_partial_apply
2727
sil [ossa] @copied_partial_apply : $@convention(thin) (@owned C) -> () {

‎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)