Skip to content

[6.2] MoveOnlyChecker: Treat trivial stores as reinitializations rather than initializations. #80854

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
2 changes: 2 additions & 0 deletions lib/SIL/Utils/FieldSensitivePrunedLiveness.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1562,6 +1562,8 @@ void FieldSensitiveMultiDefPrunedLiveRange::findBoundariesInBlock(
return getBlockLiveness(predBlock, bitNo) ==
FieldSensitivePrunedLiveBlocks::IsLive::LiveOut;
})) {
PRUNED_LIVENESS_LOG(llvm::dbgs() << "Marking ourself as boundary edge: bb"
<< block->getDebugID() << '\n');
boundary.getBoundaryEdgeBits(block).set(bitNo);
}
}
Expand Down
9 changes: 6 additions & 3 deletions lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,9 @@ static void convertMemoryReinitToInitForm(SILInstruction *memInst,
}
case SILInstructionKind::StoreInst: {
auto *si = cast<StoreInst>(memInst);
si->setOwnershipQualifier(StoreOwnershipQualifier::Init);
if (si->getOwnershipQualifier() != StoreOwnershipQualifier::Trivial) {
si->setOwnershipQualifier(StoreOwnershipQualifier::Init);
}
dest = si->getDest();
break;
}
Expand All @@ -365,7 +367,8 @@ static bool isReinitToInitConvertibleInst(SILInstruction *memInst) {
}
case SILInstructionKind::StoreInst: {
auto *si = cast<StoreInst>(memInst);
return si->getOwnershipQualifier() == StoreOwnershipQualifier::Assign;
return si->getOwnershipQualifier() == StoreOwnershipQualifier::Assign
|| si->getOwnershipQualifier() == StoreOwnershipQualifier::Trivial;
}
}
}
Expand Down Expand Up @@ -2159,7 +2162,7 @@ bool GatherUsesVisitor::visitUse(Operand *op) {
}
return true;
}

// Then handle destroy_addr specially. We want to as part of our dataflow to
// ignore destroy_addr, so we need to track it separately from other uses.
if (auto *dvi = dyn_cast<DestroyAddrInst>(user)) {
Expand Down
14 changes: 11 additions & 3 deletions lib/SILOptimizer/Mandatory/MoveOnlyUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -213,8 +213,7 @@ bool noncopyable::memInstMustInitialize(Operand *memOper) {
}
case SILInstructionKind::StoreInst: {
auto qual = cast<StoreInst>(memInst)->getOwnershipQualifier();
return qual == StoreOwnershipQualifier::Init ||
qual == StoreOwnershipQualifier::Trivial;
return qual == StoreOwnershipQualifier::Init;
}
case SILInstructionKind::BuiltinInst: {
auto bi = cast<BuiltinInst>(memInst);
Expand Down Expand Up @@ -264,7 +263,9 @@ bool noncopyable::memInstMustReinitialize(Operand *memOper) {
}
case SILInstructionKind::StoreInst:
return cast<StoreInst>(memInst)->getOwnershipQualifier() ==
StoreOwnershipQualifier::Assign;
StoreOwnershipQualifier::Assign
|| cast<StoreInst>(memInst)->getOwnershipQualifier() ==
StoreOwnershipQualifier::Trivial;

#define NEVER_OR_SOMETIMES_LOADABLE_CHECKED_REF_STORAGE(Name, ...) \
case SILInstructionKind::Store##Name##Inst: \
Expand Down Expand Up @@ -507,6 +508,13 @@ struct SimpleTemporaryAllocStackElimVisitor
// tell these projections apart from projections from earlier allocations.
return state.setFinalUser(op);
}

if (auto *si = dyn_cast<StoreInst>(user);
si && si->getOwnershipQualifier() == StoreOwnershipQualifier::Trivial) {
// Bail on trivial stores.
LLVM_DEBUG(llvm::dbgs() << "Found trivial store: " << *user);
return false;
}

if (auto *cai = dyn_cast<CopyAddrInst>(user)) {
// If we already found a copy, bail. We always only visit one of these
Expand Down
5 changes: 5 additions & 0 deletions test/SILGen/inlinearray_reabstraction.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// RUN: %target-swift-emit-silgen -disable-availability-checking -verify %s


let a: InlineArray<_, (Int)->Int> = [{$0*2}]
print(a[0](3))
2 changes: 1 addition & 1 deletion test/SILOptimizer/moveonly_addresschecker.sil
Original file line number Diff line number Diff line change
Expand Up @@ -367,9 +367,9 @@ bb0(%0 : @owned $Klass, %1 : @owned $Klass):
// CHECK: bb0([[ARG0:%.*]] : ${{.*}}, [[ARG1:%.*]] :
// CHECK: debug_value [[ARG0]]
// CHECK: debug_value [[ARG1]]
// CHECK: destroy_addr [[ARG1]]
// CHECK: [[ACCESS:%.*]] = begin_access [modify] [static] [[ARG1]]
// CHECK: [[GEP:%.*]] = struct_element_addr [[ACCESS]]
// CHECK: destroy_addr [[ARG1]]
// CHECK: store [[ARG0]] to [trivial] [[GEP]]
// CHECK: end_access [[ACCESS]]
// CHECK: } // end sil function 'myBufferViewSetter'
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// RUN: %target-swift-frontend -emit-sil -verify %s

final class Bar {
func update() {
foo?.baz = Foo2(baz: 5)
}
var foo: Foo? = Foo()
}

struct Foo: ~Copyable {
var baz: Foo2 = Foo2(baz: 0)
}

struct Foo2: ~Copyable {
var baz: Int = 0
}