From fefd027eb2a136adc44b324c3dec77fa9611b594 Mon Sep 17 00:00:00 2001 From: Michael Gottesman Date: Sun, 2 Feb 2020 17:21:29 -0800 Subject: [PATCH] [semantic-arc-opts] Convert @owned -> @guaranteed args of transforming terminators when fed by load [copy], copy_value. This extends the (copy (guaranteed x)) -> (guaranteed x) optimization to handle transforming terminator arguments. This will begin to enable us to eliminate RC ops around optionals or casts. I still need to add support for eliminating copies that forward through br args and phis. --- .../Mandatory/SemanticARCOpts.cpp | 71 ++++++-- .../semantic-arc-opts-canonical.sil | 59 ++++++- test/SILOptimizer/semantic-arc-opts.sil | 153 +++++++++++++++++- 3 files changed, 263 insertions(+), 20 deletions(-) diff --git a/lib/SILOptimizer/Mandatory/SemanticARCOpts.cpp b/lib/SILOptimizer/Mandatory/SemanticARCOpts.cpp index 97165ff25256a..db1f9744a69f7 100644 --- a/lib/SILOptimizer/Mandatory/SemanticARCOpts.cpp +++ b/lib/SILOptimizer/Mandatory/SemanticARCOpts.cpp @@ -113,11 +113,10 @@ LiveRange::LiveRange(SILValue value) continue; } - // Otherwise, we have some form of consuming use that either forwards or - // that we do not understand. See if we have a forwarding value that has a - // single non-trivial operand that can accept a guaranteed value. If not, we - // can not recursively process it, so be conservative and assume that we - // /may consume/ the value, so the live range must not be eliminated. + // Otherwise, see if we have a forwarding value that has a single + // non-trivial operand that can accept a guaranteed value. If not, we can + // not recursively process it, so be conservative and assume that we /may + // consume/ the value, so the live range must not be eliminated. // // DISCUSSION: For now we do not support forwarding instructions with // multiple non-trivial arguments since we would need to optimize all of @@ -125,7 +124,9 @@ LiveRange::LiveRange(SILValue value) // // NOTE: Today we do not support TermInsts for simplicity... we /could/ // support it though if we need to. - if (isa(user) || !isGuaranteedForwardingInst(user) || + auto *ti = dyn_cast(user); + if ((ti && !ti->isTransformationTerminator()) || + !isGuaranteedForwardingInst(user) || 1 != count_if(user->getOperandValues( true /*ignore type dependent operands*/), [&](SILValue v) { @@ -137,13 +138,39 @@ LiveRange::LiveRange(SILValue value) } // Ok, this is a forwarding instruction whose ownership we can flip from - // owned -> guaranteed. Visit its users recursively to see if the the - // users force the live range to be alive. + // owned -> guaranteed. generalForwardingInsts.push_back(user); - for (SILValue v : user->getResults()) { - if (v.getOwnershipKind() != ValueOwnershipKind::Owned) + + // If we have a non-terminator, just visit its users recursively to see if + // the the users force the live range to be alive. + if (!ti) { + for (SILValue v : user->getResults()) { + if (v.getOwnershipKind() != ValueOwnershipKind::Owned) + continue; + llvm::copy(v->getUses(), std::back_inserter(worklist)); + } + continue; + } + + // Otherwise, we know that we have no only a terminator, but a + // transformation terminator, so we should add the users of its results to + // the worklist. + for (auto &succ : ti->getSuccessors()) { + auto *succBlock = succ.getBB(); + + // If we do not have any arguments, then continue. + if (succBlock->args_empty()) continue; - llvm::copy(v->getUses(), std::back_inserter(worklist)); + + for (auto *succArg : succBlock->getSILPhiArguments()) { + // If we have an any value, just continue. + if (succArg->getOwnershipKind() == ValueOwnershipKind::None) + continue; + + // Otherwise add all users of this BBArg to the worklist to visit + // recursively. + llvm::copy(succArg->getUses(), std::back_inserter(worklist)); + } } } } @@ -519,8 +546,28 @@ static void convertForwardingInstsFromOwnedToGuaranteed( while (!guaranteedForwardingInsts.empty()) { auto *i = guaranteedForwardingInsts.back(); guaranteedForwardingInsts = guaranteedForwardingInsts.drop_back(); - assert(i->hasResults()); + // If this is a term inst, just convert all of its incoming values that are + // owned to be guaranteed. + if (auto *ti = dyn_cast(i)) { + for (auto &succ : ti->getSuccessors()) { + auto *succBlock = succ.getBB(); + + // If we do not have any arguments, then continue. + if (succBlock->args_empty()) + continue; + + for (auto *succArg : succBlock->getSILPhiArguments()) { + // If we have an any value, just continue. + if (succArg->getOwnershipKind() == ValueOwnershipKind::Owned) { + succArg->setOwnershipKind(ValueOwnershipKind::Guaranteed); + } + } + } + continue; + } + + assert(i->hasResults()); for (SILValue result : i->getResults()) { if (auto *svi = dyn_cast(result)) { if (svi->getOwnershipKind() == ValueOwnershipKind::Owned) { diff --git a/test/SILOptimizer/semantic-arc-opts-canonical.sil b/test/SILOptimizer/semantic-arc-opts-canonical.sil index 4a2c6b4723f18..7890b29e740ff 100644 --- a/test/SILOptimizer/semantic-arc-opts-canonical.sil +++ b/test/SILOptimizer/semantic-arc-opts-canonical.sil @@ -342,10 +342,8 @@ bb0(%0 : @guaranteed $Builtin.NativeObject, %1 : @guaranteed $Builtin.NativeObje return %9999 : $() } -// TODO: We should be able to optimize these switch_enum. Make sure that today -// we do not do so though. // CHECK-LABEL: sil [ossa] @switch_enum_test_no_default : $@convention(thin) (@guaranteed FakeOptional) -> () { -// CHECK: copy_value +// CHECK-NOT: copy_value // CHECK: } // end sil function 'switch_enum_test_no_default' sil [ossa] @switch_enum_test_no_default : $@convention(thin) (@guaranteed FakeOptional) -> () { bb0(%0 : @guaranteed $FakeOptional): @@ -365,7 +363,7 @@ bb3: } // CHECK-LABEL: sil [ossa] @switch_enum_test_with_default : $@convention(thin) (@guaranteed FakeOptional) -> () { -// CHECK: copy_value +// CHECK-NOT: copy_value // CHECK: } // end sil function 'switch_enum_test_with_default' sil [ossa] @switch_enum_test_with_default : $@convention(thin) (@guaranteed FakeOptional) -> () { bb0(%0 : @guaranteed $FakeOptional): @@ -434,3 +432,56 @@ bb3(%6 : $*FakeOptional): %9999 = tuple() return %9999 : $() } + +// CHECK-LABEL: sil [ossa] @switch_enum_test_loadcopy_no_default : $@convention(thin) (@owned FakeOptional) -> () { +// CHECK-NOT: load [copy] +// CHECK: load_borrow +// CHECK-NOT: load [copy] +// CHECK: } // end sil function 'switch_enum_test_loadcopy_no_default' +sil [ossa] @switch_enum_test_loadcopy_no_default : $@convention(thin) (@owned FakeOptional) -> () { +bb0(%0 : @owned $FakeOptional): + %0a = alloc_stack $FakeOptional + store %0 to [init] %0a : $*FakeOptional + %1 = load [copy] %0a : $*FakeOptional + switch_enum %1 : $FakeOptional, case #FakeOptional.some!enumelt.1: bb1, case #FakeOptional.none!enumelt: bb2 + +bb1(%2 : @owned $Builtin.NativeObject): + destroy_value %2 : $Builtin.NativeObject + br bb3 + +bb2: + br bb3 + +bb3: + destroy_addr %0a : $*FakeOptional + dealloc_stack %0a : $*FakeOptional + %9999 = tuple() + return %9999 : $() +} + +// CHECK-LABEL: sil [ossa] @switch_enum_test_loadcopy_with_default : $@convention(thin) (@owned FakeOptional) -> () { +// CHECK-NOT: load [copy] +// CHECK: load_borrow +// CHECK-NOT: load [copy] +// CHECK: } // end sil function 'switch_enum_test_loadcopy_with_default' +sil [ossa] @switch_enum_test_loadcopy_with_default : $@convention(thin) (@owned FakeOptional) -> () { +bb0(%0 : @owned $FakeOptional): + %0a = alloc_stack $FakeOptional + store %0 to [init] %0a : $*FakeOptional + %1 = load [copy] %0a : $*FakeOptional + switch_enum %1 : $FakeOptional, case #FakeOptional.some!enumelt.1: bb1, default bb2 + +bb1(%2 : @owned $Builtin.NativeObject): + destroy_value %2 : $Builtin.NativeObject + br bb3 + +bb2(%3 : @owned $FakeOptional): + destroy_value %3 : $FakeOptional + br bb3 + +bb3: + destroy_addr %0a : $*FakeOptional + dealloc_stack %0a : $*FakeOptional + %9999 = tuple() + return %9999 : $() +} diff --git a/test/SILOptimizer/semantic-arc-opts.sil b/test/SILOptimizer/semantic-arc-opts.sil index bb6824f988d66..fccab319fef04 100644 --- a/test/SILOptimizer/semantic-arc-opts.sil +++ b/test/SILOptimizer/semantic-arc-opts.sil @@ -360,10 +360,8 @@ bb0(%0 : @guaranteed $Builtin.NativeObject, %1 : @guaranteed $Builtin.NativeObje return %9999 : $() } -// TODO: We should be able to optimize these switch_enum. Make sure that today -// we do not do so though. // CHECK-LABEL: sil [ossa] @switch_enum_test_no_default : $@convention(thin) (@guaranteed FakeOptional) -> () { -// CHECK: copy_value +// CHECK-NOT: copy_value // CHECK: } // end sil function 'switch_enum_test_no_default' sil [ossa] @switch_enum_test_no_default : $@convention(thin) (@guaranteed FakeOptional) -> () { bb0(%0 : @guaranteed $FakeOptional): @@ -383,7 +381,7 @@ bb3: } // CHECK-LABEL: sil [ossa] @switch_enum_test_with_default : $@convention(thin) (@guaranteed FakeOptional) -> () { -// CHECK: copy_value +// CHECK-NOT: copy_value // CHECK: } // end sil function 'switch_enum_test_with_default' sil [ossa] @switch_enum_test_with_default : $@convention(thin) (@guaranteed FakeOptional) -> () { bb0(%0 : @guaranteed $FakeOptional): @@ -1140,3 +1138,150 @@ bb0(%0 : $*NativeObjectPair, %1 : @owned $Builtin.NativeObject): %9999 = tuple() return %9999 : $() } + +// CHECK-LABEL: sil [ossa] @switch_enum_test_loadcopy_no_default : $@convention(thin) (@owned FakeOptional) -> () { +// CHECK-NOT: load [copy] +// CHECK: load_borrow +// CHECK-NOT: load [copy] +// CHECK: } // end sil function 'switch_enum_test_loadcopy_no_default' +sil [ossa] @switch_enum_test_loadcopy_no_default : $@convention(thin) (@owned FakeOptional) -> () { +bb0(%0 : @owned $FakeOptional): + %0a = alloc_stack $FakeOptional + store %0 to [init] %0a : $*FakeOptional + %1 = load [copy] %0a : $*FakeOptional + switch_enum %1 : $FakeOptional, case #FakeOptional.some!enumelt.1: bb1, case #FakeOptional.none!enumelt: bb2 + +bb1(%2 : @owned $Builtin.NativeObject): + destroy_value %2 : $Builtin.NativeObject + br bb3 + +bb2: + br bb3 + +bb3: + destroy_addr %0a : $*FakeOptional + dealloc_stack %0a : $*FakeOptional + %9999 = tuple() + return %9999 : $() +} + +// CHECK-LABEL: sil [ossa] @switch_enum_test_loadcopy_with_default : $@convention(thin) (@owned FakeOptional) -> () { +// CHECK-NOT: load [copy] +// CHECK: load_borrow +// CHECK-NOT: load [copy] +// CHECK: } // end sil function 'switch_enum_test_loadcopy_with_default' +sil [ossa] @switch_enum_test_loadcopy_with_default : $@convention(thin) (@owned FakeOptional) -> () { +bb0(%0 : @owned $FakeOptional): + %0a = alloc_stack $FakeOptional + store %0 to [init] %0a : $*FakeOptional + %1 = load [copy] %0a : $*FakeOptional + switch_enum %1 : $FakeOptional, case #FakeOptional.some!enumelt.1: bb1, default bb2 + +bb1(%2 : @owned $Builtin.NativeObject): + destroy_value %2 : $Builtin.NativeObject + br bb3 + +bb2(%3 : @owned $FakeOptional): + destroy_value %3 : $FakeOptional + br bb3 + +bb3: + destroy_addr %0a : $*FakeOptional + dealloc_stack %0a : $*FakeOptional + %9999 = tuple() + return %9999 : $() +} + +// CHECK-LABEL: sil [ossa] @switch_enum_test_copyvalue_no_default : $@convention(thin) (@guaranteed FakeOptional) -> () { +// CHECK-NOT: copy_value +// CHECK: } // end sil function 'switch_enum_test_copyvalue_no_default' +sil [ossa] @switch_enum_test_copyvalue_no_default : $@convention(thin) (@guaranteed FakeOptional) -> () { +bb0(%0 : @guaranteed $FakeOptional): + %1 = copy_value %0 : $FakeOptional + switch_enum %1 : $FakeOptional, case #FakeOptional.some!enumelt.1: bb1, case #FakeOptional.none!enumelt: bb2 + +bb1(%2 : @owned $Builtin.NativeObject): + destroy_value %2 : $Builtin.NativeObject + br bb3 + +bb2: + br bb3 + +bb3: + %9999 = tuple() + return %9999 : $() +} + +// CHECK-LABEL: sil [ossa] @switch_enum_test_copyvalue_with_default : $@convention(thin) (@guaranteed FakeOptional) -> () { +// CHECK-NOT: copy_value +// CHECK: } // end sil function 'switch_enum_test_copyvalue_with_default' +sil [ossa] @switch_enum_test_copyvalue_with_default : $@convention(thin) (@guaranteed FakeOptional) -> () { +bb0(%0 : @guaranteed $FakeOptional): + %1 = copy_value %0 : $FakeOptional + switch_enum %1 : $FakeOptional, case #FakeOptional.some!enumelt.1: bb1, default bb2 + +bb1(%2 : @owned $Builtin.NativeObject): + destroy_value %2 : $Builtin.NativeObject + br bb3 + +bb2(%3 : @owned $FakeOptional): + destroy_value %3 : $FakeOptional + br bb3 + +bb3: + %9999 = tuple() + return %9999 : $() +} + +// CHECK-LABEL: sil [ossa] @switch_enum_test_copyvalue_with_default_and_extract : $@convention(thin) (@guaranteed FakeOptional) -> () { +// CHECK-NOT: copy_value +// CHECK: } // end sil function 'switch_enum_test_copyvalue_with_default_and_extract' +sil [ossa] @switch_enum_test_copyvalue_with_default_and_extract : $@convention(thin) (@guaranteed FakeOptional) -> () { +bb0(%0 : @guaranteed $FakeOptional): + %f = function_ref @guaranteed_user : $@convention(thin) (@guaranteed Builtin.NativeObject) -> () + %1 = copy_value %0 : $FakeOptional + switch_enum %1 : $FakeOptional, case #FakeOptional.some!enumelt.1: bb1, default bb2 + +bb1(%2 : @owned $Builtin.NativeObject): + destroy_value %2 : $Builtin.NativeObject + br bb3 + +bb2(%3 : @owned $FakeOptional): + %3a = unchecked_enum_data %3 : $FakeOptional, #FakeOptional.some!enumelt.1 + apply %f(%3a) : $@convention(thin) (@guaranteed Builtin.NativeObject) -> () + destroy_value %3a : $Builtin.NativeObject + br bb3 + +bb3: + %9999 = tuple() + return %9999 : $() +} + +// TODO: We currently are unable to get rid of the begin_borrow. We should be +// able to with appropriate analysis. +// CHECK-LABEL: sil [ossa] @switch_enum_test_copyvalue_with_borrow : $@convention(thin) (@owned FakeOptional) -> () { +// CHECK-NOT: copy_value +// CHECK: } // end sil function 'switch_enum_test_copyvalue_with_borrow' +sil [ossa] @switch_enum_test_copyvalue_with_borrow : $@convention(thin) (@owned FakeOptional) -> () { +bb0(%0 : @owned $FakeOptional): + %f = function_ref @guaranteed_user : $@convention(thin) (@guaranteed Builtin.NativeObject) -> () + %0a = begin_borrow %0 : $FakeOptional + %1 = copy_value %0a : $FakeOptional + switch_enum %1 : $FakeOptional, case #FakeOptional.some!enumelt.1: bb1, default bb2 + +bb1(%2 : @owned $Builtin.NativeObject): + destroy_value %2 : $Builtin.NativeObject + br bb3 + +bb2(%3 : @owned $FakeOptional): + %3a = unchecked_enum_data %3 : $FakeOptional, #FakeOptional.some!enumelt.1 + apply %f(%3a) : $@convention(thin) (@guaranteed Builtin.NativeObject) -> () + destroy_value %3a : $Builtin.NativeObject + br bb3 + +bb3: + end_borrow %0a : $FakeOptional + destroy_value %0 : $FakeOptional + %9999 = tuple() + return %9999 : $() +}