Skip to content

Handle store_borrow in SILMem2Reg and some other fixes #59541

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

Merged
merged 8 commits into from
Aug 26, 2022
Merged

Handle store_borrow in SILMem2Reg and some other fixes #59541

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
f2d9bc7
Add dealloc_stack_ref as a non-write in LoadBorrowImmutabilityAnalysis
meg-gupta Aug 18, 2022
8769cbe
Simplify SILMem2Reg by using computeDominatedBoundaryBlocks api inste…
meg-gupta Jun 16, 2022
7f6fd0b
Reorganize if to reduce indentation
meg-gupta Jun 16, 2022
0c2fd6b
Remove unnecessary edge case
meg-gupta Jun 16, 2022
13223f6
Remove fake use of lexical copy
meg-gupta Jun 17, 2022
d3b0219
Update test
meg-gupta Jun 24, 2022
8e9117d
Handle store_borrow in SILMem2Reg
meg-gupta Jun 9, 2022
ed7788f
Get rid of isStorageValid flag on lastStoreInst
meg-gupta Aug 25, 2022
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
5 changes: 3 additions & 2 deletions include/swift/SILOptimizer/Utils/CFGOptUtils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,14 @@ TermInst *changeEdgeValue(TermInst *branch, SILBasicBlock *dest, size_t idx,
/// specified index. Asserts internally that the argument along the edge does
/// not have uses.
TermInst *deleteEdgeValue(TermInst *branch, SILBasicBlock *destBlock,
size_t argIndex);
size_t argIndex, bool cleanupDeadPhiOp = true);

/// Erase the \p argIndex phi argument from \p block. Asserts that the argument
/// is a /real/ phi argument. Removes all incoming values for the argument from
/// predecessor terminators. Asserts internally that it only ever is given
/// "true" phi argument.
void erasePhiArgument(SILBasicBlock *block, unsigned argIndex);
void erasePhiArgument(SILBasicBlock *block, unsigned argIndex,
bool cleanupDeadPhiOp = true);

/// Replace a branch target.
///
Expand Down
1 change: 1 addition & 0 deletions lib/SIL/Verifier/LoadBorrowImmutabilityChecker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ bool GatherWritesVisitor::visitUse(Operand *op, AccessUseType useTy) {
case SILInstructionKind::SelectEnumAddrInst:
case SILInstructionKind::SwitchEnumAddrInst:
case SILInstructionKind::DeallocStackInst:
case SILInstructionKind::DeallocStackRefInst:
case SILInstructionKind::DeallocBoxInst:
case SILInstructionKind::WitnessMethodInst:
case SILInstructionKind::ExistentialMetatypeInst:
Expand Down
563 changes: 352 additions & 211 deletions lib/SILOptimizer/Transforms/SILMem2Reg.cpp
View file
Open in desktop

Large diffs are not rendered by default.

24 changes: 15 additions & 9 deletions lib/SILOptimizer/Utils/CFGOptUtils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ deleteTriviallyDeadOperandsOfDeadArgument(MutableArrayRef<Operand> termOperands,
// Our implementation assumes that our caller is attempting to remove a dead
// SILPhiArgument from a SILBasicBlock and has already RAUWed the argument.
TermInst *swift::deleteEdgeValue(TermInst *branch, SILBasicBlock *destBlock,
size_t argIndex) {
size_t argIndex, bool cleanupDeadPhiOps) {
if (auto *cbi = dyn_cast<CondBranchInst>(branch)) {
SmallVector<SILValue, 8> trueArgs;
SmallVector<SILValue, 8> falseArgs;
Expand All @@ -97,14 +97,18 @@ TermInst *swift::deleteEdgeValue(TermInst *branch, SILBasicBlock *destBlock,
llvm::copy(cbi->getFalseArgs(), std::back_inserter(falseArgs));

if (destBlock == cbi->getTrueBB()) {
deleteTriviallyDeadOperandsOfDeadArgument(cbi->getTrueOperands(),
argIndex);
if (cleanupDeadPhiOps) {
deleteTriviallyDeadOperandsOfDeadArgument(cbi->getTrueOperands(),
argIndex);
}
trueArgs.erase(trueArgs.begin() + argIndex);
}

if (destBlock == cbi->getFalseBB()) {
deleteTriviallyDeadOperandsOfDeadArgument(cbi->getFalseOperands(),
argIndex);
if (cleanupDeadPhiOps) {
deleteTriviallyDeadOperandsOfDeadArgument(cbi->getFalseOperands(),
argIndex);
}
falseArgs.erase(falseArgs.begin() + argIndex);
}

Expand All @@ -120,8 +124,9 @@ TermInst *swift::deleteEdgeValue(TermInst *branch, SILBasicBlock *destBlock,
if (auto *bi = dyn_cast<BranchInst>(branch)) {
SmallVector<SILValue, 8> args;
llvm::copy(bi->getArgs(), std::back_inserter(args));

deleteTriviallyDeadOperandsOfDeadArgument(bi->getAllOperands(), argIndex);
if (cleanupDeadPhiOps) {
deleteTriviallyDeadOperandsOfDeadArgument(bi->getAllOperands(), argIndex);
}
args.erase(args.begin() + argIndex);
auto *result = SILBuilderWithScope(bi).createBranch(bi->getLoc(),
bi->getDestBB(), args);
Expand All @@ -132,7 +137,8 @@ TermInst *swift::deleteEdgeValue(TermInst *branch, SILBasicBlock *destBlock,
llvm_unreachable("unsupported terminator");
}

void swift::erasePhiArgument(SILBasicBlock *block, unsigned argIndex) {
void swift::erasePhiArgument(SILBasicBlock *block, unsigned argIndex,
bool cleanupDeadPhiOps) {
assert(block->getArgument(argIndex)->isPhi()
&& "Only should be used on phi arguments");
block->eraseArgument(argIndex);
Expand All @@ -149,7 +155,7 @@ void swift::erasePhiArgument(SILBasicBlock *block, unsigned argIndex) {
predBlocks.insert(pred);

for (auto *pred : predBlocks)
deleteEdgeValue(pred->getTerminator(), block, argIndex);
deleteEdgeValue(pred->getTerminator(), block, argIndex, cleanupDeadPhiOps);
}

/// Changes the edge value between a branch and destination basic block
Expand Down
3 changes: 3 additions & 0 deletions test/SILOptimizer/OSLogFullOptTest.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,13 +131,15 @@ func testNSObjectInterpolation(nsArray: NSArray) {
// TODO: check why the ARC optimizer cannot eliminate the many retain/release pairs here.
// CHECK: entry:
// CHECK-NEXT: bitcast %TSo7NSArrayC* %0 to i8*
// CHECK-NEXT: [[COPY:%.+]] = tail call i8* @llvm.objc.retain
// CHECK-NEXT: [[NSARRAY_ARG:%.+]] = tail call i8* @llvm.objc.retain
// CHECK: tail call swiftcc i1 @"${{.*}}isLoggingEnabled{{.*}}"()
// CHECK-NEXT: br i1 {{%.*}}, label %[[ENABLED:[0-9]+]], label %[[NOT_ENABLED:[0-9]+]]

// CHECK: [[NOT_ENABLED]]:
// CHECK-NEXT: tail call void @swift_release
// CHECK-NEXT: tail call void @llvm.objc.release
// CHECK-NEXT: tail call void @llvm.objc.release
// CHECK: br label %[[EXIT:[0-9]+]]

// CHECK: [[ENABLED]]:
Expand Down Expand Up @@ -167,6 +169,7 @@ func testNSObjectInterpolation(nsArray: NSArray) {
// CHECK-NEXT: [[BITCASTED_SRC2:%.+]] = bitcast i8* {{.*}} to %TSo7NSArrayC*
// CHECK-64-NEXT: store %TSo7NSArrayC* [[BITCASTED_SRC2]], %TSo7NSArrayC** [[BITCASTED_DEST2]], align 8
// CHECK-32-NEXT: store %TSo7NSArrayC* [[BITCASTED_SRC2]], %TSo7NSArrayC** [[BITCASTED_DEST2]], align 4
// CHECK-NEXT: tail call void @llvm.objc.release(i8* [[NSARRAY_ARG]])
// CHECK-64: tail call swiftcc void @"${{.*}}_os_log_impl_test{{.*}}"({{.*}}, {{.*}}, {{.*}}, {{.*}}, i8* getelementptr inbounds ([20 x i8], [20 x i8]* @{{.*}}, i64 0, i64 0), i8* {{(nonnull )?}}[[BUFFER]], i32 12)
// CHECK-32: tail call swiftcc void @"${{.*}}_os_log_impl_test{{.*}}"({{.*}}, {{.*}}, {{.*}}, {{.*}}, i8* getelementptr inbounds ([20 x i8], [20 x i8]* @{{.*}}, i32 0, i32 0), i8* {{(nonnull )?}}[[BUFFER]], i32 8)
// CHECK: [[BITCASTED_OBJ_STORAGE:%.+]] = bitcast i8* [[OBJ_STORAGE]] to %swift.opaque*
Expand Down
50 changes: 10 additions & 40 deletions test/SILOptimizer/mem2reg_borrows.sil
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,23 +79,6 @@ bb0(%0 : @owned $Klass):
return %6 : $()
}

// CHECK-LABEL: sil [ossa] @test_no_storeborrow4 :
// CHECK-NOT: alloc_stack
// CHECK-LABEL: } // end sil function 'test_no_storeborrow4'
sil [ossa] @test_no_storeborrow4 : $@convention(thin) (@owned Klass) -> () {
bb0(%0 : @owned $Klass):
%1 = alloc_stack [lexical] $Klass
store %0 to [init] %1 : $*Klass
%2 = load_borrow %1 : $*Klass
%3 = function_ref @use_guaranteed : $@convention(thin) (@guaranteed Klass) -> ()
%4 = apply %3(%2) : $@convention(thin) (@guaranteed Klass) -> ()
end_borrow %2 : $Klass
destroy_addr %1 : $*Klass
dealloc_stack %1 : $*Klass
%6 = tuple ()
return %6 : $()
}

// load_borrow of projections are not optimized
// CHECK-LABEL: sil [ossa] @test_with_structs_and_borrows1 :
// CHECK: alloc_stack
Expand All @@ -114,10 +97,10 @@ bb0(%0 : @guaranteed $WrapperStruct):
%r = tuple ()
return %r : $()
}
// CHECK-LABEL: sil [ossa] @store_only_allocas :
// CHECK: alloc_stack
// CHECK-LABEL: } // end sil function 'store_only_allocas'
sil [ossa] @store_only_allocas : $@convention(thin) (@guaranteed Klass) -> () {
// CHECK-LABEL: sil [ossa] @storeborrow_only_allocas :
// CHECK-NOT: alloc_stack
// CHECK-LABEL: } // end sil function 'storeborrow_only_allocas'
sil [ossa] @storeborrow_only_allocas : $@convention(thin) (@guaranteed Klass) -> () {
bb0(%0 : @guaranteed $Klass):
%1 = alloc_stack $Klass
%2 = store_borrow %0 to %1 : $*Klass
Expand All @@ -127,21 +110,8 @@ bb0(%0 : @guaranteed $Klass):
return %6 : $()
}

// CHECK-LABEL: sil [ossa] @store_only_lexicalallocas :
// CHECK: alloc_stack
// CHECK-LABEL: } // end sil function 'store_only_lexicalallocas'
sil [ossa] @store_only_lexicalallocas : $@convention(thin) (@guaranteed Klass) -> () {
bb0(%0 : @guaranteed $Klass):
%1 = alloc_stack [lexical] $Klass
%2 = store_borrow %0 to %1 : $*Klass
end_borrow %2 : $*Klass
dealloc_stack %1 : $*Klass
%6 = tuple ()
return %6 : $()
}

// CHECK-LABEL: sil [ossa] @test1 :
// CHECK: alloc_stack
// CHECK-NOT: alloc_stack
// CHECK-LABEL: } // end sil function 'test1'
sil [ossa] @test1 : $@convention(thin) (@guaranteed Klass) -> () {
bb0(%0 : @guaranteed $Klass):
Expand All @@ -158,7 +128,7 @@ bb0(%0 : @guaranteed $Klass):
}

// CHECK-LABEL: sil [ossa] @test1_lexical :
// CHECK: alloc_stack
// CHECK-NOT: alloc_stack
// CHECK-LABEL: } // end sil function 'test1_lexical'
sil [ossa] @test1_lexical : $@convention(thin) (@guaranteed Klass) -> () {
bb0(%0 : @guaranteed $Klass):
Expand All @@ -175,7 +145,7 @@ bb0(%0 : @guaranteed $Klass):
}

// CHECK-LABEL: sil [ossa] @test2 :
// CHECK: alloc_stack
// CHECK-NOT: alloc_stack
// CHECK-LABEL: } // end sil function 'test2'
sil [ossa] @test2 : $@convention(thin) (@guaranteed Klass) -> () {
bb0(%0 : @guaranteed $Klass):
Expand All @@ -196,7 +166,7 @@ bb0(%0 : @guaranteed $Klass):
}

// CHECK-LABEL: sil [ossa] @test3 :
// CHECK: alloc_stack
// CHECK-NOT: alloc_stack
// CHECK-LABEL: } // end sil function 'test3'
sil [ossa] @test3 : $@convention(thin) (@guaranteed Klass, @guaranteed Klass) -> () {
bb0(%0 : @guaranteed $Klass, %1 : @guaranteed $Klass):
Expand All @@ -218,7 +188,7 @@ bb0(%0 : @guaranteed $Klass, %1 : @guaranteed $Klass):
}

// CHECK-LABEL: sil [ossa] @test4 :
// CHECK: alloc_stack
// CHECK-NOT: alloc_stack
// CHECK-LABEL: } // end sil function 'test4'
sil [ossa] @test4 : $@convention(thin) (@guaranteed Klass) -> () {
bb0(%0 : @guaranteed $Klass):
Expand All @@ -238,7 +208,7 @@ bb0(%0 : @guaranteed $Klass):
}

// CHECK-LABEL: sil [ossa] @test6 :
// CHECK: alloc_stack
// CHECK-NOT: alloc_stack
// CHECK-LABEL: } // end sil function 'test6'
sil [ossa] @test6 : $@convention(thin) (@owned Klass) -> () {
bb0(%0 : @owned $Klass):
Expand Down
Loading