Skip to content

[semantic-arc-opts] Teach semantic-arc-opts how to handle tuples with multiple non-trivial operands. #32179

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
8 changes: 8 additions & 0 deletions include/swift/SIL/OwnershipUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,10 @@ struct OwnedValueIntroducerKind {
/// owned.
Struct,

/// An owned value that is from a tuple that has multiple operands that are
/// owned.
Tuple,

/// An owned value that is a function argument.
FunctionArgument,

Expand All @@ -528,6 +532,8 @@ struct OwnedValueIntroducerKind {
return OwnedValueIntroducerKind(BeginApply);
case ValueKind::StructInst:
return OwnedValueIntroducerKind(Struct);
case ValueKind::TupleInst:
return OwnedValueIntroducerKind(Tuple);
case ValueKind::SILPhiArgument: {
auto *phiArg = cast<SILPhiArgument>(value);
if (dyn_cast_or_null<TryApplyInst>(phiArg->getSingleTerminator())) {
Expand Down Expand Up @@ -621,6 +627,7 @@ struct OwnedValueIntroducer {
case OwnedValueIntroducerKind::LoadTake:
case OwnedValueIntroducerKind::Phi:
case OwnedValueIntroducerKind::Struct:
case OwnedValueIntroducerKind::Tuple:
case OwnedValueIntroducerKind::FunctionArgument:
case OwnedValueIntroducerKind::PartialApplyInit:
case OwnedValueIntroducerKind::AllocBoxInit:
Expand All @@ -639,6 +646,7 @@ struct OwnedValueIntroducer {
case OwnedValueIntroducerKind::Phi:
return true;
case OwnedValueIntroducerKind::Struct:
case OwnedValueIntroducerKind::Tuple:
case OwnedValueIntroducerKind::Copy:
case OwnedValueIntroducerKind::LoadCopy:
case OwnedValueIntroducerKind::Apply:
Expand Down
3 changes: 3 additions & 0 deletions lib/SIL/Utils/OwnershipUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,9 @@ void OwnedValueIntroducerKind::print(llvm::raw_ostream &os) const {
case OwnedValueIntroducerKind::Struct:
os << "Struct";
return;
case OwnedValueIntroducerKind::Tuple:
os << "Tuple";
return;
case OwnedValueIntroducerKind::FunctionArgument:
os << "FunctionArgument";
return;
Expand Down
12 changes: 12 additions & 0 deletions lib/SILOptimizer/Transforms/SemanticARCOpts.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ class OwnershipPhiOperand {
enum Kind {
Branch,
Struct,
Tuple,
};

private:
Expand All @@ -67,6 +68,7 @@ class OwnershipPhiOperand {
switch (op->getUser()->getKind()) {
case SILInstructionKind::BranchInst:
case SILInstructionKind::StructInst:
case SILInstructionKind::TupleInst:
return {{const_cast<Operand *>(op)}};
default:
return None;
Expand All @@ -79,6 +81,8 @@ class OwnershipPhiOperand {
return Kind::Branch;
case SILInstructionKind::StructInst:
return Kind::Struct;
case SILInstructionKind::TupleInst:
return Kind::Tuple;
default:
llvm_unreachable("unhandled case?!");
}
Expand All @@ -104,6 +108,7 @@ class OwnershipPhiOperand {
switch (getKind()) {
case Kind::Branch:
return true;
case Kind::Tuple:
case Kind::Struct:
return false;
}
Expand All @@ -121,6 +126,8 @@ class OwnershipPhiOperand {
switch (getKind()) {
case Kind::Struct:
return visitor(cast<StructInst>(getInst()));
case Kind::Tuple:
return visitor(cast<TupleInst>(getInst()));
case Kind::Branch: {
auto *br = cast<BranchInst>(getInst());
unsigned opNum = getOperandNumber();
Expand Down Expand Up @@ -589,6 +596,11 @@ static SILValue convertIntroducerToGuaranteed(OwnedValueIntroducer introducer) {
si->setOwnershipKind(ValueOwnershipKind::Guaranteed);
return si;
}
case OwnedValueIntroducerKind::Tuple: {
auto *ti = cast<TupleInst>(introducer.value);
ti->setOwnershipKind(ValueOwnershipKind::Guaranteed);
return ti;
}
case OwnedValueIntroducerKind::Copy:
case OwnedValueIntroducerKind::LoadCopy:
case OwnedValueIntroducerKind::Apply:
Expand Down
2 changes: 0 additions & 2 deletions test/SILOptimizer/semantic-arc-opts-canonical.sil
Original file line number Diff line number Diff line change
Expand Up @@ -323,8 +323,6 @@ bb0(%0 : @guaranteed $StructMemberTest):
// multiple trivial arguments), we can.
//
// CHECK-LABEL: sil [ossa] @multiple_arg_forwarding_inst_test : $@convention(thin) (@guaranteed Builtin.NativeObject, @guaranteed Builtin.NativeObject, Builtin.Int32) -> () {
// CHECK: copy_value
// CHECK: copy_value
// CHECK-NOT: copy_value
// CHECK: } // end sil function 'multiple_arg_forwarding_inst_test'
sil [ossa] @multiple_arg_forwarding_inst_test : $@convention(thin) (@guaranteed Builtin.NativeObject, @guaranteed Builtin.NativeObject, Builtin.Int32) -> () {
Expand Down
21 changes: 14 additions & 7 deletions test/SILOptimizer/semantic-arc-opts.sil
Original file line number Diff line number Diff line change
Expand Up @@ -347,13 +347,7 @@ bb0(%0 : @guaranteed $StructMemberTest):
return %7 : $Builtin.Int32
}

// Make sure that in a case where we have multiple non-trivial values passed to
// a forwarding instruction we do not optimize... but if we only have one (and
// multiple trivial arguments), we can.
//
// CHECK-LABEL: sil [ossa] @multiple_arg_forwarding_inst_test : $@convention(thin) (@guaranteed Builtin.NativeObject, @guaranteed Builtin.NativeObject, Builtin.Int32) -> () {
// CHECK: copy_value
// CHECK: copy_value
// CHECK-NOT: copy_value
// CHECK: } // end sil function 'multiple_arg_forwarding_inst_test'
sil [ossa] @multiple_arg_forwarding_inst_test : $@convention(thin) (@guaranteed Builtin.NativeObject, @guaranteed Builtin.NativeObject, Builtin.Int32) -> () {
Expand Down Expand Up @@ -2560,4 +2554,17 @@ bb0(%0 : @guaranteed $Builtin.NativeObject, %1 : @guaranteed $Builtin.NativeObje
destroy_value %2 : $NativeObjectPair
%9999 = tuple()
return %9999 : $()
}
}

// CHECK-LABEL: sil [ossa] @tuple_with_multiple_nontrivial_operands : $@convention(thin) (@guaranteed Builtin.NativeObject, @guaranteed Builtin.NativeObject) -> () {
// CHECK-NOT: copy_value
// CHECK: } // end sil function 'tuple_with_multiple_nontrivial_operands'
sil [ossa] @tuple_with_multiple_nontrivial_operands : $@convention(thin) (@guaranteed Builtin.NativeObject, @guaranteed Builtin.NativeObject) -> () {
bb0(%0 : @guaranteed $Builtin.NativeObject, %1 : @guaranteed $Builtin.NativeObject):
%0a = copy_value %0 : $Builtin.NativeObject
%1a = copy_value %1 : $Builtin.NativeObject
%2 = tuple (%0a : $Builtin.NativeObject, %1a : $Builtin.NativeObject)
destroy_value %2 : $(Builtin.NativeObject, Builtin.NativeObject)
%9999 = tuple()
return %9999 : $()
}