Skip to content

Add and improve some "mandatory" peephole optimization to fix a performance diagnostic #62247

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
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
8 changes: 5 additions & 3 deletions lib/SILOptimizer/Transforms/AllocBoxToStack.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ static bool useCaptured(Operand *UI) {
// These instructions do not cause the address to escape.
if (isa<DebugValueInst>(User)
|| isa<StrongReleaseInst>(User) || isa<StrongRetainInst>(User)
|| isa<DestroyValueInst>(User))
|| isa<DestroyValueInst>(User)
|| isa<EndBorrowInst>(User))
return false;

if (auto *Store = dyn_cast<StoreInst>(User)) {
Expand Down Expand Up @@ -254,8 +255,9 @@ static bool partialApplyEscapes(SILValue V, bool examineApply) {
// If we have a copy_value, the copy value does not cause an escape, but its
// uses might do so... so add the copy_value's uses to the worklist and
// continue.
if (auto CVI = dyn_cast<CopyValueInst>(User)) {
llvm::copy(CVI->getUses(), std::back_inserter(Worklist));
if (isa<CopyValueInst>(User) || isa<BeginBorrowInst>(User)) {
llvm::copy(cast<SingleValueInstruction>(User)->getUses(),
std::back_inserter(Worklist));
continue;
}

Expand Down
43 changes: 33 additions & 10 deletions lib/SILOptimizer/Transforms/GenericSpecializer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include "swift/SILOptimizer/Utils/InstructionDeleter.h"
#include "swift/SILOptimizer/Utils/SILOptFunctionBuilder.h"
#include "swift/SILOptimizer/Utils/SILInliner.h"
#include "swift/SILOptimizer/Utils/StackNesting.h"
#include "llvm/ADT/SmallVector.h"

using namespace swift;
Expand Down Expand Up @@ -195,10 +196,12 @@ class MandatoryGenericSpecializer : public SILModuleTransform {

void run() override;

bool optimize(SILFunction *func, ClassHierarchyAnalysis *cha);
bool optimize(SILFunction *func, ClassHierarchyAnalysis *cha,
bool &invalidatedStackNesting);

bool optimizeInst(SILInstruction *inst, SILOptFunctionBuilder &funcBuilder,
InstructionDeleter &deleter, ClassHierarchyAnalysis *cha);
InstructionDeleter &deleter, ClassHierarchyAnalysis *cha,
bool &invalidatedStackNesting);
};


Expand All @@ -220,7 +223,7 @@ void MandatoryGenericSpecializer::run() {
visited.insert(&function);
}
}

while (!workList.empty()) {
SILFunction *func = workList.pop_back_val();
module->linkFunction(func, SILModule::LinkingMode::LinkAll);
Expand All @@ -229,20 +232,26 @@ void MandatoryGenericSpecializer::run() {

// Perform generic specialization and other related optimization.

bool invalidatedStackNesting = false;

// To avoid phase ordering problems of the involved optimizations, iterate
// until we reach a fixed point.
// This should always happen, but to be on the safe side, limit the number
// of iterations to 10 (which is more than enough - usually the loop runs
// 1 to 3 times).
for (int i = 0; i < 10; i++) {
bool changed = optimize(func, cha);
bool changed = optimize(func, cha, invalidatedStackNesting);
if (changed) {
invalidateAnalysis(func, SILAnalysis::InvalidationKind::FunctionBody);
} else {
break;
}
}

if (invalidatedStackNesting) {
StackNesting::fixNesting(func);
}

// Continue specializing called functions.
for (SILBasicBlock &block : *func) {
for (SILInstruction &inst : block) {
Expand All @@ -260,7 +269,8 @@ void MandatoryGenericSpecializer::run() {
/// Specialize generic calls in \p func and do some other related optimizations:
/// devirtualization and constant-folding of the Builtin.canBeClass.
bool MandatoryGenericSpecializer::optimize(SILFunction *func,
ClassHierarchyAnalysis *cha) {
ClassHierarchyAnalysis *cha,
bool &invalidatedStackNesting) {
bool changed = false;
SILOptFunctionBuilder funcBuilder(*this);
InstructionDeleter deleter;
Expand All @@ -282,7 +292,7 @@ bool MandatoryGenericSpecializer::optimize(SILFunction *func,
continue;

for (SILInstruction *inst : deleter.updatingReverseRange(&block)) {
changed |= optimizeInst(inst, funcBuilder, deleter, cha);
changed |= optimizeInst(inst, funcBuilder, deleter, cha, invalidatedStackNesting);
}
}
deleter.cleanupDeadInstructions();
Expand All @@ -295,7 +305,8 @@ bool MandatoryGenericSpecializer::optimize(SILFunction *func,

bool MandatoryGenericSpecializer::
optimizeInst(SILInstruction *inst, SILOptFunctionBuilder &funcBuilder,
InstructionDeleter &deleter, ClassHierarchyAnalysis *cha) {
InstructionDeleter &deleter, ClassHierarchyAnalysis *cha,
bool &invalidatedStackNesting) {
if (auto as = ApplySite::isa(inst)) {

bool changed = false;
Expand All @@ -307,10 +318,22 @@ optimizeInst(SILInstruction *inst, SILOptFunctionBuilder &funcBuilder,
as = newAS;
}

auto fas = FullApplySite::isa(as.getInstruction());
if (!fas)
if (auto *pai = dyn_cast<PartialApplyInst>(as)) {
SILBuilderContext builderCtxt(funcBuilder.getModule());
if (tryOptimizeApplyOfPartialApply(pai, builderCtxt, deleter.getCallbacks())) {
// Try to delete the partial_apply.
// We don't need to copy all arguments again (to extend their lifetimes),
// because it was already done in tryOptimizeApplyOfPartialApply.
tryDeleteDeadClosure(pai, deleter.getCallbacks(), /*needKeepArgsAlive=*/ false);
invalidatedStackNesting = true;
return true;
}
return changed;

}

auto fas = FullApplySite::isa(as.getInstruction());
assert(fas);

SILFunction *callee = fas.getReferencedFunctionOrNull();
if (!callee)
return changed;
Expand Down
4 changes: 2 additions & 2 deletions lib/SILOptimizer/Utils/PartialApplyCombiner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -240,8 +240,8 @@ bool PartialApplyCombiner::combine() {
auto *user = use->getUser();

// Recurse through copy_value
if (auto *cvi = dyn_cast<CopyValueInst>(user)) {
for (auto *copyUse : cvi->getUses())
if (isa<CopyValueInst>(user) || isa<BeginBorrowInst>(user)) {
for (auto *copyUse : cast<SingleValueInstruction>(user)->getUses())
worklist.push_back(copyUse);
continue;
}
Expand Down
8 changes: 6 additions & 2 deletions test/IRGen/alloc_box.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,17 @@

func f() -> Bool? { return nil }

var gb = false
var gc: () -> () = {}

({
guard var b = f() else { return }
let c = { b = true }
_ = (b, c)
gb = b
gc = c
})()

// CHECK-LABEL: @"$s9alloc_boxyyXEfU_"
// CHECK-LABEL: @"$s9alloc_boxyyXEfU0_"
// CHECK-NOT: call void @swift_setDeallocating
// CHECK: call void @swift_deallocUninitializedObject
// CHECK-NOT: call void @swift_setDeallocating
Expand Down
32 changes: 32 additions & 0 deletions test/SILOptimizer/allocbox_to_stack_ownership.sil
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,38 @@ bb0(%0 : $Int):
unreachable
}

// CHECK-LABEL: sil [ossa] @test_copy_and_borrow_of_closure
// CHECK-NOT: alloc_box
// CHECK: alloc_stack
// CHECK-NOT: alloc_box
// CHECK: } // end sil function 'test_copy_and_borrow_of_closure'
sil [ossa] @test_copy_and_borrow_of_closure : $@convention(thin) (Int) -> () {
bb0(%0 : $Int):
// This is not destroyed, but the unreachable makes the verifier not trip.
%1 = alloc_box $<τ_0_0> { var τ_0_0 } <Int>
%1a = project_box %1 : $<τ_0_0> { var τ_0_0 } <Int>, 0
store %0 to [trivial] %1a : $*Int
%3 = function_ref @closure3 : $@convention(thin) (@owned <τ_0_0> { var τ_0_0 } <Int>) -> ()
%4 = copy_value %1 : $<τ_0_0> { var τ_0_0 } <Int>
%5 = partial_apply %3(%4) : $@convention(thin) (@owned <τ_0_0> { var τ_0_0 } <Int>) -> ()
%6 = begin_borrow %5 : $@callee_owned () -> ()
%7 = copy_value %6 : $@callee_owned () -> ()
apply %7() : $@callee_owned () -> ()
end_borrow %6 : $@callee_owned () -> ()
destroy_value %5 : $@callee_owned () -> ()
destroy_value %1 : $<τ_0_0> { var τ_0_0 } <Int>
%r = tuple ()
return %r : $()
}

sil [ossa] @closure3 : $@convention(thin) (@owned <τ_0_0> { var τ_0_0 } <Int>) -> () {
bb0(%0 : @owned $<τ_0_0> { var τ_0_0 } <Int>):
%1 = project_box %0 : $<τ_0_0> { var τ_0_0 } <Int>, 0
destroy_value %0 : $<τ_0_0> { var τ_0_0 } <Int> // id: %7
%r = tuple ()
return %r : $()
}

sil [ossa] @closureWithBoxArg : $@convention(thin) (@guaranteed { var SomeClass }) -> () {
bb0(%0 : @guaranteed ${ var SomeClass }):
%r = tuple ()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -574,15 +574,25 @@ public func castAccess2<T : P>(_ x : __owned T) {
// Partial Apply Tests //
/////////////////////////

public func nonEscapingpartialApplyTest<T : P>(_ x: __owned T) {
var x2 = x // expected-error {{'x2' used after being moved}}
x2 = x
let _ = _move x2 // expected-note {{move here}}
let f = { // expected-note {{use here}}
print(x2)
}
f()
}

// This makes sure we always fail if we are asked to check in a partial apply.
public func partialApplyTest<T : P>(_ x: __owned T) {
public func partialApplyTest<T : P>(_ x: __owned T) -> () -> () {
var x2 = x
x2 = x
let _ = _move x2 // expected-error {{move applied to value that the compiler does not support checking}}
let f = {
print(x2)
}
f()
return f
}

////////////////////////
Expand Down
10 changes: 10 additions & 0 deletions test/SILOptimizer/performance-annotations.swift
Original file line number Diff line number Diff line change
Expand Up @@ -213,3 +213,13 @@ func goodClosure2() {
})
}

@_noAllocation
func closueWhichModifiesLocalVar() -> Int {
var x = 42
let localNonEscapingClosure = {
x += 1
}
localNonEscapingClosure()
return x
}

19 changes: 19 additions & 0 deletions test/SILOptimizer/sil_combine_apply_ossa.sil
Original file line number Diff line number Diff line change
Expand Up @@ -1152,3 +1152,22 @@ bb3:
%rv = tuple()
return %rv : $()
}

// CHECK-LABEL: sil [ossa] @test_copy_and_borrow_of_closure
// CHECK-NOT: partial_apply
// CHECK-LABEL: } // end sil function 'test_copy_and_borrow_of_closure'
sil [ossa] @test_copy_and_borrow_of_closure : $@convention(thin) (Int) -> () {
bb0(%0 : $Int):
%3 = function_ref @closure2 : $@convention(thin) (Int) -> ()
%5 = partial_apply %3(%0) : $@convention(thin) (Int) -> ()
%6 = begin_borrow [lexical] %5 : $@callee_owned () -> ()
%7 = copy_value %6 : $@callee_owned () -> ()
apply %7() : $@callee_owned () -> ()
end_borrow %6 : $@callee_owned () -> ()
destroy_value %5 : $@callee_owned () -> ()
%r = tuple ()
return %r : $()
}

sil [ossa] @closure2 : $@convention(thin) (Int) -> ()