Skip to content

6.0: [PrunedLiveness] Fix boundary check for dead-ends. #73691

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 23 additions & 1 deletion lib/SIL/Utils/BasicBlockUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,17 @@
//===----------------------------------------------------------------------===//

#include "swift/SIL/BasicBlockUtils.h"
#include "swift/SIL/BasicBlockDatastructures.h"
#include "swift/Basic/Defer.h"
#include "swift/Basic/STLExtras.h"
#include "swift/SIL/BasicBlockDatastructures.h"
#include "swift/SIL/Dominance.h"
#include "swift/SIL/LoopInfo.h"
#include "swift/SIL/SILArgument.h"
#include "swift/SIL/SILBasicBlock.h"
#include "swift/SIL/SILBuilder.h"
#include "swift/SIL/SILFunction.h"
#include "swift/SIL/TerminatorUtils.h"
#include "swift/SIL/Test.h"
#include "llvm/ADT/STLExtras.h"

using namespace swift;
Expand Down Expand Up @@ -413,6 +414,27 @@ bool DeadEndBlocks::triviallyEndsInUnreachable(SILBasicBlock *block) {
return isa<UnreachableInst>(block->getTerminator());
}

namespace swift::test {
// Arguments:
// - none
// Dumps:
// - the function
// - the blocks which are dead-end blocks
static FunctionTest DeadEndBlocksTest("dead_end_blocks", [](auto &function,
auto &arguments,
auto &test) {
std::unique_ptr<DeadEndBlocks> DeadEnds;
DeadEnds.reset(new DeadEndBlocks(&function));
function.print(llvm::outs());
#ifndef NDEBUG
for (auto &block : function) {
if (DeadEnds->isDeadEnd(&block))
block.printID(llvm::outs(), true);
}
#endif
});
} // end namespace swift::test

//===----------------------------------------------------------------------===//
// Post Dominance Set Completion Utilities
//===----------------------------------------------------------------------===//
Expand Down
59 changes: 51 additions & 8 deletions lib/SIL/Utils/PrunedLiveness.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -489,17 +489,62 @@ bool PrunedLiveRange<LivenessWithDefs>::isWithinBoundary(
llvm_unreachable("instruction must be in its parent block");
}

/// Whether \p parent is a dead (reported to be dead by `liveBlocks`), dead-end
/// (such as an infinite loop) block within the availability boundary (where
/// the value has not been consumed).
static bool checkDeadEnd(SILBasicBlock *parent, DeadEndBlocks *deadEndBlocks,
PrunedLiveBlocks const &liveBlocks) {
if (!deadEndBlocks) {
return false;
}
if (!deadEndBlocks->isDeadEnd(parent)) {
return false;
}
if (liveBlocks.getBlockLiveness(parent) != PrunedLiveBlocks::Dead) {
return false;
}
// Check whether the value is available in `parent` (i.e. not consumed on any
// path to it):
//
// Search backward until LiveOut or LiveWithin blocks are reached.
// (1) If ALL the reached blocks are LiveOut, then `parent` IS within the
// availability boundary.
// (2) If ANY reached block is LiveWithin, the value was consumed in that
// reached block, preventing the value from being available at `parent`,
// so `parent` is NOT within the availability boundary.
BasicBlockWorklist worklist(parent->getFunction());
worklist.push(parent);
while (auto *block = worklist.pop()) {
auto isLive = liveBlocks.getBlockLiveness(block);
switch (isLive) {
case PrunedLiveBlocks::Dead: {
// Availability is unchanged; continue the backwards walk.
for (auto *predecessor : block->getPredecessorBlocks()) {
worklist.pushIfNotVisited(predecessor);
}
break;
}
case PrunedLiveBlocks::LiveWithin:
// Availability ended in this block. Some path to `parent` consumed the
// value. Case (2) above.
return false;
case PrunedLiveBlocks::LiveOut:
// Availability continued out of this block. Case (1) above.
continue;
}
}
return true;
}

template <typename LivenessWithDefs>
bool PrunedLiveRange<LivenessWithDefs>::areUsesWithinBoundary(
ArrayRef<Operand *> uses, DeadEndBlocks *deadEndBlocks) const {
assert(asImpl().isInitialized());

auto checkDeadEnd = [deadEndBlocks](SILInstruction *inst) {
return deadEndBlocks && deadEndBlocks->isDeadEnd(inst->getParent());
};
for (auto *use : uses) {
auto *user = use->getUser();
if (!asImpl().isWithinBoundary(user) && !checkDeadEnd(user))
if (!asImpl().isWithinBoundary(user) &&
!checkDeadEnd(user->getParent(), deadEndBlocks, liveBlocks))
return false;
}
return true;
Expand All @@ -510,12 +555,10 @@ bool PrunedLiveRange<LivenessWithDefs>::areUsesOutsideBoundary(
ArrayRef<Operand *> uses, DeadEndBlocks *deadEndBlocks) const {
assert(asImpl().isInitialized());

auto checkDeadEnd = [deadEndBlocks](SILInstruction *inst) {
return deadEndBlocks && deadEndBlocks->isDeadEnd(inst->getParent());
};
for (auto *use : uses) {
auto *user = use->getUser();
if (asImpl().isWithinBoundary(user) || checkDeadEnd(user))
if (asImpl().isWithinBoundary(user) ||
checkDeadEnd(user->getParent(), deadEndBlocks, liveBlocks))
return false;
}
return true;
Expand Down
23 changes: 23 additions & 0 deletions test/SILOptimizer/cse_ossa_nontrivial.sil
Original file line number Diff line number Diff line change
Expand Up @@ -723,6 +723,29 @@ bb0(%0 : @guaranteed $WrapperKlass):
return %res : $()
}

// CHECK-LABEL: sil [ossa] @test_refelementaddr_loop : {{.*}} {
// CHECK: ref_element_addr
// CHECK: ref_element_addr
// CHECK-LABEL: } // end sil function 'test_refelementaddr_loop'
sil [ossa] @test_refelementaddr_loop : $@convention(thin) () -> () {
bb0:
%2 = apply undef() : $@convention(thin) () -> @owned WrapperKlass
%3 = move_value [lexical] [var_decl] %2 : $WrapperKlass
br bb1

bb1:
%func = function_ref @use_object : $@convention(thin) (@inout Builtin.NativeObject) -> ()
%borrow1 = begin_borrow %3 : $WrapperKlass
%ele1 = ref_element_addr %borrow1 : $WrapperKlass, #WrapperKlass.val
apply %func(%ele1) : $@convention(thin) (@inout Builtin.NativeObject) -> ()
end_borrow %borrow1 : $WrapperKlass
%borrow2 = begin_borrow %3 : $WrapperKlass
%ele2 = ref_element_addr %borrow2 : $WrapperKlass, #WrapperKlass.val
apply %func(%ele2) : $@convention(thin) (@inout Builtin.NativeObject) -> ()
end_borrow %borrow2 : $WrapperKlass
br bb1
}

sil [ossa] @use_word2 : $@convention(thin) (@inout Builtin.Word) -> ()

/*
Expand Down
50 changes: 50 additions & 0 deletions test/SILOptimizer/dead_end_blocks.sil
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// RUN: %target-sil-opt -test-runner %s -o /dev/null 2>&1 | %FileCheck %s

// REQUIRES: asserts

// CHECK-LABEL: begin running test {{.*}} on with_trap: dead_end_blocks
// CHECK-LABEL: sil @with_trap : {{.*}} {
// CHECK: cond_br undef, [[DIE:bb[0-9]+]], [[EXIT:bb[0-9]+]]
// CHECK: [[DIE]]:
// CHECK: unreachable
// CHECK-LABEL: } // end sil function 'with_trap'
// CHECK: [[DIE]]
// CHECK-LABEL: end running test {{.*}} on with_trap: dead_end_blocks
sil @with_trap : $@convention(thin) () -> () {
entry:
specify_test "dead_end_blocks"
cond_br undef, die, exit

die:
unreachable

exit:
%retval = tuple ()
return %retval : $()
}

// CHECK-LABEL: begin running test {{.*}} on with_infinite_loop: dead_end_blocks
// CHECK-LABEL: sil @with_infinite_loop : $@convention(thin) () -> () {
// CHECK: cond_br undef, [[EXIT:bb[0-9]+]], [[HEADER:bb[0-9]+]]
// CHECK: [[HEADER]]:
// CHECK: br [[LOOP:bb[0-9]+]]
// CHECK: [[LOOP]]:
// CHECK: br [[LOOP]]
// CHECK-LABEL: } // end sil function 'with_infinite_loop'
// CHECK: [[HEADER]]
// CHECK: [[LOOP]]
// CHECK-LABEL: end running test {{.*}} on with_infinite_loop: dead_end_blocks
sil @with_infinite_loop : $@convention(thin) () -> () {
entry:
specify_test "dead_end_blocks"
cond_br undef, exit, header
header:
br loop
loop:
br loop
exit:
%retval = tuple ()
return %retval : $()
}


12 changes: 12 additions & 0 deletions test/embedded/lit.local.cfg
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# Make a local copy of the substitutions.
config.substitutions = list(config.substitutions)

if config.target_sdk_name == 'macosx':
def do_fixup(key, value):
if isinstance(value, str):
value = value.replace("-apple-macosx10.13", "-apple-macos14")
elif isinstance(value, SubstituteCaptures):
value.substitution = value.substitution.replace("-apple-macosx10.13", "-apple-macos14")
return (key, value)

config.substitutions = [do_fixup(a, b) for (a, b) in config.substitutions]
9 changes: 9 additions & 0 deletions validation-test/embedded/lit.local.cfg
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import sys
import platform

# Load the config at test/embedded/lit.local.cfg
lit_config.load_config(config,
os.path.join(
os.path.dirname(os.path.dirname(os.path.dirname(__file__))),
'test', 'embedded', 'lit.local.cfg'))

25 changes: 25 additions & 0 deletions validation-test/embedded/rdar126965232.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// RUN: %target-swiftc_driver \
// RUN: -c \
// RUN: %s \
// RUN: -Xfrontend -sil-verify-all \
// RUN: -enable-experimental-feature Embedded \
// RUN: -wmo \
// RUN: -Osize \
// RUN: -o %t/bin

// REQUIRES: swift_in_compiler
// REQUIRES: optimized_stdlib
// REQUIRES: OS=macosx || OS=linux-gnu

class MyClass {
var x: Int? = nil
var enabled: Bool = true
}

public func app_main() {
let object = MyClass()
while true {
let enabled = object.enabled
object.enabled = !enabled
}
}