Skip to content

[SILOptimizer] Eliminate dead alloc_pack #66638

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

[SILOptimizer] Eliminate dead alloc_pack #66638

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1ebbc22
Eliminate dead alloc_pack
Azoy Jun 14, 2023
ea62c9b
Add dead pack sil test
Azoy Jun 21, 2023
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
44 changes: 44 additions & 0 deletions lib/SILOptimizer/Transforms/DeadObjectElimination.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ STATISTIC(DeadKeyPathEliminated,
STATISTIC(DeadAllocApplyEliminated,
"number of allocating Apply instructions removed");

STATISTIC(DeadAllocPackEliminated,
"number of AllocPack instructions removed");

using UserList = llvm::SmallSetVector<SILInstruction *, 16>;

namespace {
Expand Down Expand Up @@ -283,6 +286,14 @@ static bool canZapInstruction(SILInstruction *Inst, bool acceptRefCountInsts,
if (isa<BeginAccessInst>(Inst) || isa<EndAccessInst>(Inst))
return true;

// Setting a value within a pack does not prevent eliminating an alloc_pack.
if (auto set = dyn_cast<PackElementSetInst>(Inst)) {
// TODO: when we have OSSA, we can also accept stores of non trivial values:
// just replace the store with a destroy_value.
return !onlyAcceptTrivialStores ||
set->getElementType().isTrivial(*set->getFunction());
}

// If Inst does not read or write to memory, have side effects, and is not a
// terminator, we can zap it.
if (!Inst->mayHaveSideEffects() && !Inst->mayReadFromMemory() &&
Expand Down Expand Up @@ -738,6 +749,7 @@ class DeadObjectElimination : public SILFunctionTransform {
bool processKeyPath(KeyPathInst *KPI);
bool processAllocBox(AllocBoxInst *ABI){ return false;}
bool processAllocApply(ApplyInst *AI, DeadEndBlocks &DEBlocks);
bool processAllocPack(AllocPackInst *API);

bool insertCompensatingReleases(SILInstruction *before,
const UserList &users);
Expand Down Expand Up @@ -766,6 +778,8 @@ class DeadObjectElimination : public SILFunctionTransform {
Changed |= processAllocBox(A);
else if (auto *A = dyn_cast<ApplyInst>(&inst))
Changed |= processAllocApply(A, DEBlocks);
else if (auto *A = dyn_cast<AllocPackInst>(&inst))
Changed |= processAllocPack(A);
}
deleter.cleanupDeadInstructions();
}
Expand Down Expand Up @@ -899,6 +913,36 @@ bool DeadObjectElimination::processKeyPath(KeyPathInst *KPI) {
return true;
}

bool DeadObjectElimination::processAllocPack(AllocPackInst *API) {
bool isTrivialType = true;

// If all the elements within this pack are trivial, then this pack is trivial.
// Otherwise, a single non-trivial element makes this entire pack non-trivial.
for (auto i : indices(API->getPackType()->getElementTypes())) {
auto eltTy = API->getPackType()->getSILElementType(i);

if (!eltTy.isTrivial(*API->getFunction())) {
isTrivialType = false;
break;
}
}

UserList UsersToRemove;
if (hasUnremovableUsers(API, &UsersToRemove, /*acceptRefCountInsts=*/ true,
/*onlyAcceptTrivialStores*/ !isTrivialType)) {
LLVM_DEBUG(llvm::dbgs() << " Found a use that cannot be zapped...\n");
return false;
}

// Remove the AllocPack and all of its users.
removeInstructions(
ArrayRef<SILInstruction*>(UsersToRemove.begin(), UsersToRemove.end()));
LLVM_DEBUG(llvm::dbgs() << " Success! Eliminating alloc_pack.\n");

++DeadAllocPackEliminated;
return true;
}

/// If AI is the version of an initializer where we pass in either an apply or
/// an alloc_ref to initialize in place, validate that we are able to continue
/// optimizing and return To
Expand Down
101 changes: 101 additions & 0 deletions test/SILOptimizer/dead_alloc_pack.sil
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
// RUN: %target-sil-opt -enable-sil-verify-all -deadobject-elim %s | %FileCheck %s

import Swift
import Builtin
import SwiftShims

sil @makeTuple : $@convention(thin) <each T> (@pack_guaranteed Pack{repeat each T}) -> @pack_out Pack{repeat each T} {

}

// CHECK-LABEL: sil @makeEmptyPack : $@convention(thin) () -> () {
// CHECK-NEXT: bb0:
// CHECK-NEXT: %0 = tuple ()
// CHECK-NEXT: return %0 : $()
sil @makeEmptyPack : $@convention(thin) () -> () {
bb0:
%0 = alloc_pack $Pack{}
debug_value %0 : $*Pack{}
dealloc_pack %0 : $*Pack{}
%1 = tuple ()
return %1 : $()
}

// CHECK-LABEL: sil @makeConcretePack : $@convention(thin) () -> () {
// CHECK-NEXT: bb0:
// CHECK-NEXT: %0 = tuple ()
// CHECK-NEXT: return %0 : $()
sil @makeConcretePack : $@convention(thin) () -> () {
%0 = alloc_pack $Pack{Bool, Bool}
%1 = scalar_pack_index 0 of $Pack{Bool, Bool}
%2 = scalar_pack_index 1 of $Pack{Bool, Bool}

%3 = alloc_stack $Bool
%4 = integer_literal $Builtin.Int1, 1
%5 = struct $Bool (%4 : $Builtin.Int1)
store %5 to %3 : $*Bool

%6 = alloc_stack $Bool
%7 = integer_literal $Builtin.Int1, 0
%8 = struct $Bool (%7 : $Builtin.Int1)
store %8 to %6 : $*Bool

pack_element_set %3 : $*Bool into %1 of %0 : $*Pack{Bool, Bool}
pack_element_set %6 : $*Bool into %2 of %0 : $*Pack{Bool, Bool}

dealloc_stack %6 : $*Bool
dealloc_stack %3 : $*Bool
dealloc_pack %0 : $*Pack{Bool, Bool}

%9 = tuple ()
return %9 : $()
}

// FIXME: This should get eliminated, but the load (pack_element_get) prevents
// deletion. Add logic in DeadStore/LSLocation to potentially remove these.
//
// CHECK-LABEL: sil @makePackGet : $@convention(thin) () -> () {
// CHECK-NEXT: bb0:
// CHECK-NEXT: %0 = alloc_pack $Pack{Bool}
sil @makePackGet : $@convention(thin) () -> () {
%0 = alloc_pack $Pack{Bool}
%1 = scalar_pack_index 0 of $Pack{Bool}

%2 = alloc_stack $Bool
%3 = integer_literal $Builtin.Int1, 1
%4 = struct $Bool (%3 : $Builtin.Int1)
store %4 to %2 : $*Bool

pack_element_set %2 : $*Bool into %1 of %0 : $*Pack{Bool}

%5 = pack_element_get %1 of %0 : $*Pack{Bool} as $*Bool
%6 = load %5 : $*Bool

dealloc_stack %2 : $*Bool
dealloc_pack %0 : $*Pack{Bool}

%tup = tuple ()
return %tup : $()
}

// FIXME: This should get eliminated as well
//
// CHECK-LABEL: sil @makeNonTrivialPack : $@convention(thin) () -> () {
// CHECK-NEXT: bb0:
// CHECK-NEXT: %0 = alloc_pack $Pack{Builtin.BridgeObject}
sil @makeNonTrivialPack : $@convention(thin) () -> () {
%0 = alloc_pack $Pack{Builtin.BridgeObject}

%1 = integer_literal $Builtin.IntLiteral, 123
%2 = value_to_bridge_object %1 : $Builtin.IntLiteral
%3 = scalar_pack_index 0 of $Pack{Builtin.BridgeObject}
%4 = alloc_stack $Builtin.BridgeObject
store %2 to %4 : $*Builtin.BridgeObject
pack_element_set %4 : $*Builtin.BridgeObject into %3 of %0 : $*Pack{Builtin.BridgeObject}

dealloc_stack %4 : $*Builtin.BridgeObject
dealloc_pack %0 : $*Pack{Builtin.BridgeObject}

%tup = tuple ()
return %tup : $()
}
58 changes: 58 additions & 0 deletions test/SILOptimizer/dead_alloc_pack.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
// RUN: %target-swift-frontend -parse-as-library -O -emit-sil -enable-builtin-module -module-name dap %s | %FileCheck %s

import Builtin

class TrivialDestructor {
var int : Int
var int2 : Int
init() {
self.int = 0
self.int2 = 1
}
//deinit {}
}

class Kl {}

class NontrivialDestructor {
var p : Kl
var i : Int
init() {
self.p = Kl()
self.i = 123
}
}

public func makeTuple<each T>(_ t: repeat each T) -> (repeat each T) {
(repeat each t)
}

// CHECK-LABEL: sil @$s3dap14makeEmptyTupleyyF : $@convention(thin) () -> () {
// CHECK: bb0:
// CHECK-NEXT: [[RET:%.*]] = tuple ()
// CHECK-NEXT: return [[RET]] : $()
public func makeEmptyTuple() {
makeTuple()
}

// CHECK-LABEL: sil @$s3dap16makeTrivialTupleyyF : $@convention(thin) () -> () {
// CHECK: bb0
// CHECK-NEXT: [[RET:%.*]] = tuple ()
// CHECK-NEXT: return [[RET]] : $()
public func makeTrivialTuple() {
makeTuple(123, 321, 456, 654)
}

// CHECK-LABEL: sil @$s3dap26makeTrivialDestructorTupleyyF : $@convention(thin) () -> () {
// CHECK: bb0:
// CHECK-NEXT: %0 = alloc_pack $Pack{TrivialDestructor, TrivialDestructor}
public func makeTrivialDestructorTuple() {
makeTuple(TrivialDestructor(), TrivialDestructor())
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this also a case that should be eliminated? It seems we make 2 packs to pass to makeTuple, but we also consider this pack to be non-trivial.. is that correct?

}

// CHECK-LABEL: sil @$s3dap19makeNonTrivialTupleyyF : $@convention(thin) () -> () {
// CHECK: bb0:
// CHECK-NEXT: %0 = alloc_pack $Pack{NontrivialDestructor, NontrivialDestructor}
public func makeNonTrivialTuple() {
makeTuple(NontrivialDestructor(), NontrivialDestructor())
}
6 changes: 0 additions & 6 deletions test/SILOptimizer/tuples_from_packs.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,8 @@
return (repeat each t)
}

// FIXME: Useless alloc_pack/dealloc_pack

// CHECK-LABEL: sil @$s17tuples_from_packs14makeEmptyTupleyyF : $@convention(thin) () -> () {
// CHECK: bb0:
// CHECK-NEXT: %0 = alloc_pack $Pack{}
// CHECK-NEXT: %1 = alloc_pack $Pack{}
// CHECK-NEXT: dealloc_pack %1 : $*Pack{}
// CHECK-NEXT: dealloc_pack %0 : $*Pack{}
// CHECK-NEXT: [[RET:%.*]] = tuple ()
// CHECK-NEXT: return [[RET]] : $()
public func makeEmptyTuple() {
Expand Down