Skip to content

[AddressLowering] Handle store_borrow. #62486

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 6 commits into from
Dec 12, 2022
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
7 changes: 5 additions & 2 deletions docs/SIL.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4070,11 +4070,14 @@ The stack location must not be modified by other instructions than
``store_borrow``.
All uses of the store_borrow destination ```%1`` should be via the store_borrow
return address ``%2`` except dealloc_stack.
The stored value is alive until the ``end_borrow``. During the its lifetime,the
The stored value is alive until the ``end_borrow``. During its lifetime, the
stored value must not be modified or destroyed.
The source value ``%0`` is borrowed (i.e. not copied) and it's borrow scope
The source value ``%0`` is borrowed (i.e. not copied) and its borrow scope
must outlive the lifetime of the stored value.

Notionally, the outer borrow scope ensures that there's something to be
addressed. The inner borrow scope provides the address to work with.

begin_borrow
````````````

Expand Down
9 changes: 5 additions & 4 deletions lib/SIL/Verifier/MemoryLifetimeVerifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -562,15 +562,16 @@ void MemoryLifetimeVerifier::checkFunction(BitDataflow &dataFlow) {
require(expectedReturnBits & ~bs.data.exitSet,
"indirect argument is not alive at function return", term);
require(bs.data.exitSet & ~expectedReturnBits & nonTrivialLocations,
"memory is initialized at function return but shouldn't", term,
/*excludeTrivialEnums*/ true);
"memory is initialized at function return but shouldn't be",
term,
/*excludeTrivialEnums*/ true);
break;
case SILInstructionKind::ThrowInst:
require(expectedThrowBits & ~bs.data.exitSet,
"indirect argument is not alive at throw", term);
require(bs.data.exitSet & ~expectedThrowBits & nonTrivialLocations,
"memory is initialized at throw but shouldn't", term,
/*excludeTrivialEnums*/ true);
"memory is initialized at throw but shouldn't be", term,
/*excludeTrivialEnums*/ true);
break;
default:
break;
Expand Down
13 changes: 8 additions & 5 deletions lib/SIL/Verifier/SILVerifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2243,6 +2243,10 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
require(
F.hasOwnership(),
"Inst with qualified ownership in a function that is not qualified");
if (EBI->getOperand()->getType().isAddress()) {
require(isa<StoreBorrowInst>(EBI->getOperand()),
"end_borrow of an address not produced by store_borrow");
}
}

void checkEndLifetimeInst(EndLifetimeInst *I) {
Expand Down Expand Up @@ -2446,23 +2450,22 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
}

void checkStoreBorrowInst(StoreBorrowInst *SI) {
// A store_borrow must be to an alloc_stack. That alloc_stack can only be
// used by store_borrows (and dealloc_stacks).
require(SI->getSrc()->getType().isObject(),
"Can't store from an address source");
require(!fnConv.useLoweredAddresses()
|| SI->getSrc()->getType().isLoadable(*SI->getFunction()),
"Can't store a non loadable type");
require(SI->getDest()->getType().isAddress(),
"Must store to an address dest");
// Note: This is the current implementation and the design is not final.
require(isa<AllocStackInst>(SI->getDest()),
"store_borrow destination should be alloc_stack");
"store_borrow destination can only be an alloc_stack");
requireSameType(SI->getDest()->getType().getObjectType(),
SI->getSrc()->getType(),
"Store operand type and dest type mismatch");

// Note: This is the current implementation and the design is not final.
require(isa<AllocStackInst>(SI->getDest()),
"store_borrow destination can only be an alloc_stack");

SSAPrunedLiveness scopedAddressLiveness;
ScopedAddressValue scopedAddress(SI);
// FIXME: Reenable @test_load_borrow_store_borrow_nested in
Expand Down
16 changes: 14 additions & 2 deletions lib/SILOptimizer/Mandatory/AddressLowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3101,8 +3101,17 @@ class UseRewriter : SILInstructionVisitor<UseRewriter> {
llvm::report_fatal_error("Unimplemented SelectValue use.");
}

// Opaque enum operand to a switch_enum.
void visitSwitchEnumInst(SwitchEnumInst *SEI);
void visitStoreBorrowInst(StoreBorrowInst *sbi) {
auto addr = addrMat.materializeAddress(use->get());
SmallVector<Operand *, 4> uses(sbi->getUses());
for (auto *use : uses) {
if (auto *ebi = dyn_cast<EndBorrowInst>(use->getUser())) {
pass.deleter.forceDelete(ebi);
}
}
sbi->replaceAllUsesWith(addr);
pass.deleter.forceDelete(sbi);
}

void rewriteStore(SILValue srcVal, SILValue destAddr,
IsInitialization_t isInit);
Expand Down Expand Up @@ -3134,6 +3143,9 @@ class UseRewriter : SILInstructionVisitor<UseRewriter> {
// member implies an address-only Struct.
void visitStructInst(StructInst *structInst) {}

// Opaque enum operand to a switch_enum.
void visitSwitchEnumInst(SwitchEnumInst *SEI);

// Opaque call argument.
void visitTryApplyInst(TryApplyInst *tryApplyInst) {
CallArgRewriter(tryApplyInst, pass).rewriteIndirectArgument(use);
Expand Down
2 changes: 1 addition & 1 deletion test/SIL/store_borrow_verify_errors.sil
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ bb1:
}

// CHECK: Begin Error in function test_store_borrow_dest
// CHECK: SIL verification failed: store_borrow destination should be alloc_stack: isa<AllocStackInst>(SI->getDest())
// CHECK: SIL verification failed: store_borrow destination can only be an alloc_stack: isa<AllocStackInst>(SI->getDest())
// CHECK: Verifying instruction:
// CHECK: %0 = argument of bb0 : $Klass // user: %3
// CHECK: %2 = struct_element_addr %1 : $*NonTrivialStruct, #NonTrivialStruct.val // user: %3
Expand Down
20 changes: 20 additions & 0 deletions test/SIL/verifier_failures.sil
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// RUN: %target-sil-opt -emit-sorted-sil -verify-continue-on-failure -o /dev/null %s 2>&1 | %FileCheck %s
// REQUIRES: asserts

sil_stage canonical

import Builtin
import Swift
import SwiftShims

class C {}

// CHECK: Begin Error in function end_borrow_1_addr_alloc_stack
// CHECK-NEXT: SIL verification failed: end_borrow of an address not produced by store_borrow
sil [ossa] @end_borrow_1_addr_alloc_stack : $@convention(thin) () -> () {
%addr = alloc_stack $C
end_borrow %addr : $*C
dealloc_stack %addr : $*C
%retval = tuple ()
return %retval : $()
}
30 changes: 30 additions & 0 deletions test/SILOptimizer/address_lowering.sil
Original file line number Diff line number Diff line change
Expand Up @@ -2149,6 +2149,36 @@ bb0:
return %retval : $()
}

// CHECK-LABEL: sil [ossa] @test_store_borrow_1_copy_addr : {{.*}} {
// CHECK: [[ADDR:%[^,]+]] = alloc_stack
// CHECK: apply undef<T>([[ADDR]])
// CHECK: [[COPY:%[^,]+]] = alloc_stack
// CHECK: copy_addr [[ADDR]] to [init] [[COPY]]
// CHECK: apply undef<T>([[COPY]])
// CHECK: destroy_addr [[COPY]]
// CHECK: dealloc_stack [[COPY]]
// CHECK-NOT: end_borrow [[ADDR]]
// CHECK: destroy_addr [[ADDR]]
// CHECK-LABEL: } // end sil function 'test_store_borrow_1_copy_addr'
sil [ossa] @test_store_borrow_1_copy_addr : $@convention(thin) <T> () -> () {
entry:
%addr = alloc_stack $T
%instance = apply undef<T>() : $@convention(thin) <τ> () -> (@out τ)
%lifetime = begin_borrow %instance : $T
%borrow = store_borrow %lifetime to %addr : $*T
%copy = alloc_stack $T
copy_addr %borrow to [init] %copy : $*T
apply undef<T>(%copy) : $@convention(thin) <τ> (@inout τ) -> ()
destroy_addr %copy : $*T
dealloc_stack %copy : $*T
end_borrow %borrow : $*T
end_borrow %lifetime : $T
dealloc_stack %addr : $*T
destroy_value %instance : $T
%retval = tuple ()
return %retval : $()
}

// CHECK-LABEL: sil hidden [ossa] @test_unchecked_bitwise_cast :
// CHECK: bb0(%0 : $*U, %1 : $*T, %2 : $@thick U.Type):
// CHECK: [[STK:%.*]] = alloc_stack $T
Expand Down