Skip to content

Fix ExistentialSpecializer bugs for swiftpm and enable existential-specializer for sil-opt #20617

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 1 commit into from
Dec 3, 2018
Merged

Fix ExistentialSpecializer bugs for swiftpm and enable existential-specializer for sil-opt #20617

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 8 additions & 1 deletion lib/SILOptimizer/FunctionSignatureTransforms/ExistentialSpecializer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,8 @@ static bool findConcreteType(ApplySite AI, int ArgIdx, CanType &ConcreteType) {
SILValue InitExistential =
findInitExistentialFromGlobalAddrAndApply(GAI, AI, ArgIdx);
/// If the Arg is already init_existential, return the concrete type.
if (findConcreteTypeFromInitExistential(InitExistential, ConcreteType)) {
if (InitExistential &&
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes the case where global_addr analysis can not find an init_existential. See "global_addr_init" test case below.

findConcreteTypeFromInitExistential(InitExistential, ConcreteType)) {
return true;
}
}
Expand Down Expand Up @@ -257,6 +258,12 @@ bool ExistentialSpecializer::canSpecializeCalleeFunction(ApplySite &Apply) {
if (Callee->getInlineStrategy() == Inline_t::AlwaysInline)
return false;

/// Ignore externally linked functions with public_external or higher
/// linkage.
if (isAvailableExternally(Callee->getLinkage())) {
Copy link
Contributor Author

@rajbarik rajbarik Nov 26, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes the swiftpm bug related to external linkage functions causing name mangling issues. See test case: s7dealloc20wrap_foo_ncp_another1aSiAA1P_pz_tF below.

return false;
}

/// Only choose a select few function representations for specialization.
switch (Callee->getRepresentation()) {
case SILFunctionTypeRepresentation::ObjCMethod:
Expand Down
36 changes: 10 additions & 26 deletions lib/SILOptimizer/FunctionSignatureTransforms/ExistentialTransform.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,6 @@ void ExistentialSpecializerCloner::cloneAndPopulateFunction() {
SILModule &M = OrigF->getModule();
auto &Ctx = M.getASTContext();
llvm::SmallDenseMap<int, AllocStackInst *> ArgToAllocStackMap;
bool MissingDestroyUse = false;

NewFBuilder.setInsertionPoint(ClonedEntryBB);

Expand Down Expand Up @@ -153,8 +152,6 @@ void ExistentialSpecializerCloner::cloneAndPopulateFunction() {
IsInitialization_t::IsInitialization);
if (ExistentialArgDescriptor[ArgDesc.Index].DestroyAddrUse) {
NewFBuilder.createDestroyAddr(InsertLoc, NewArg);
} else {
MissingDestroyUse = true;
}
entryArgs.push_back(ASI);
break;
Expand Down Expand Up @@ -192,13 +189,11 @@ void ExistentialSpecializerCloner::cloneAndPopulateFunction() {
/// If there is an argument with no DestroyUse, insert DeallocStack
/// before return Instruction.
llvm::SmallPtrSet<ReturnInst *, 4> ReturnInsts;
if (MissingDestroyUse) {
/// Find the set of return instructions in a function.
for (auto &BB : NewF) {
TermInst *TI = BB.getTerminator();
if (auto *RI = dyn_cast<ReturnInst>(TI)) {
ReturnInsts.insert(RI);
}
/// Find the set of return instructions in a function.
for (auto &BB : NewF) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes the old code that was inserting dealloc_stack after destroy_addr which is wrong. dealloc_stack are now inserted just before return instructions. See test case: $s7dealloc12wrap_foo_ncp1a1bSiAA1P_pz_AaE_pztFTf4ee_n below.

TermInst *TI = BB.getTerminator();
if (auto *RI = dyn_cast<ReturnInst>(TI)) {
ReturnInsts.insert(RI);
}
}

Expand All @@ -207,22 +202,11 @@ void ExistentialSpecializerCloner::cloneAndPopulateFunction() {
int ArgIndex = ArgDesc.Index;
auto iter = ArgToAllocStackMap.find(ArgIndex);
if (iter != ArgToAllocStackMap.end()) {
auto it = ExistentialArgDescriptor.find(ArgIndex);
if (it != ExistentialArgDescriptor.end() && it->second.DestroyAddrUse) {
for (Operand *ASIUse : iter->second->getUses()) {
auto *ASIUser = ASIUse->getUser();
if (auto *DAI = dyn_cast<DestroyAddrInst>(ASIUser)) {
SILBuilder Builder(ASIUser);
Builder.setInsertionPoint(&*std::next(ASIUser->getIterator()));
Builder.createDeallocStack(DAI->getLoc(), iter->second);
}
}
} else { // Need to insert DeallocStack before return.
for (auto *I : ReturnInsts) {
SILBuilder Builder(I->getParent());
Builder.setInsertionPoint(I);
Builder.createDeallocStack(iter->second->getLoc(), iter->second);
}
// Need to insert DeallocStack before return.
for (auto *I : ReturnInsts) {
SILBuilder Builder(I->getParent());
Builder.setInsertionPoint(I);
Builder.createDeallocStack(iter->second->getLoc(), iter->second);
}
}
}
Expand Down
217 changes: 217 additions & 0 deletions test/SILOptimizer/existential_transform_extras.sil
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,217 @@
// RUN: %target-sil-opt -assume-parsing-unqualified-ownership-sil -enable-sil-verify-all %s -enable-sil-existential-specializer -existential-specializer | %FileCheck %s

// Additional tests for existential_specializer

import Builtin
import Swift
import SwiftShims

internal protocol P {
func foo() -> Int32
}

internal class Klass1 : P {
@inline(never) func foo() -> Int32
init()
}

internal class Klass2 : P {
@inline(never) func foo() -> Int32
init()
}

@inline(never) internal func wrap_foo_ncp(a: inout P, b: inout P) -> Int32

@inline(never) func ncp()

sil hidden [noinline] @$s7dealloc3ncpyyF : $@convention(thin) () -> Int32 {
bb0:
%0 = alloc_stack $P, var, name "magic2"
%1 = alloc_ref $Klass1
%4 = init_existential_addr %0 : $*P, $Klass1
store %1 to %4 : $*Klass1
%6 = alloc_stack $P, var, name "magic3"
%7 = alloc_ref $Klass1
%10 = init_existential_addr %6 : $*P, $Klass1
store %7 to %10 : $*Klass1
%12 = function_ref @$s7dealloc12wrap_foo_ncp1a1bSiAA1P_pz_AaE_pztF : $@convention(thin) (@in P, @in P) -> Int32
%13 = apply %12(%0, %6) : $@convention(thin) (@in P, @in P) -> Int32
debug_value %13 : $Int32, let, name "x"
%14 = alloc_stack $P, var, name "magic4"
%15 = alloc_ref $Klass1
%16 = init_existential_addr %14 : $*P, $Klass1
store %15 to %16 : $*Klass1
%17 = function_ref @$s7dealloc20wrap_foo_ncp_another1aSiAA1P_pz_tF : $@convention(thin) (@inout P) -> Int32
%18 = apply %17(%14) : $@convention(thin) (@inout P) -> Int32
%24 = struct_extract %13 : $Int32, #Int32._value
%25 = struct_extract %18 : $Int32, #Int32._value
%26 = integer_literal $Builtin.Int1, -1
%27 = builtin "sadd_with_overflow_Int32"(%24 : $Builtin.Int32, %25 : $Builtin.Int32, %26 : $Builtin.Int1) : $(Builtin.Int32, Builtin.Int1)
%28 = tuple_extract %27 : $(Builtin.Int32, Builtin.Int1), 0
%29 = tuple_extract %27 : $(Builtin.Int32, Builtin.Int1), 1
cond_fail %29 : $Builtin.Int1
%31 = struct $Int32 (%28 : $Builtin.Int32)
destroy_addr %14 : $*P
dealloc_stack %14 : $*P
dealloc_stack %6 : $*P
dealloc_stack %0 : $*P
return %31 : $Int32
}

// CHECK-LABEL: sil public_external [serialized] @$s7dealloc20wrap_foo_ncp_another1aSiAA1P_pz_tF : $@convention(thin) (@inout P) -> Int32 {
// CHECK: bb0(%0 : $*P):
// CHECK: debug_value_addr
// CHECK: alloc_stack
// CHECK: copy_addr
// CHECK: open_existential_addr
// CHECK: witness_method
// CHECK: apply
// CHECK: destroy_addr
// CHECK: dealloc_stack
// CHECK: return
// CHECK-LABEL :} // end sil function '$s7dealloc20wrap_foo_ncp_another1aSiAA1P_pz_tF'
sil public_external [serialized] @$s7dealloc20wrap_foo_ncp_another1aSiAA1P_pz_tF : $@convention(thin) (@inout P) -> Int32 {
bb0(%0 : $*P):
debug_value_addr %0 : $*P, var, name "a", argno 1
%2 = alloc_stack $P
copy_addr %0 to [initialization] %2 : $*P
%4 = open_existential_addr immutable_access %2 : $*P to $*@opened("EE9F89E4-ECF4-11E8-8DDF-D0817AD4059B") P
%5 = witness_method $@opened("EE9F89E4-ECF4-11E8-8DDF-D0817AD4059B") P, #P.foo!1 : <Self where Self : P> (Self) -> () -> Int32, %4 : $*@opened("EE9F89E4-ECF4-11E8-8DDF-D0817AD4059B") P : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> Int32
%6 = apply %5<@opened("EE9F89E4-ECF4-11E8-8DDF-D0817AD4059B") P>(%4) : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> Int32
destroy_addr %2 : $*P
dealloc_stack %2 : $*P
return %6 : $Int32
} // end sil function '$s7dealloc20wrap_foo_ncp_another1aSiAA1P_pz_tF'

sil shared [noinline] @$s7dealloc6Klass1C3fooSiyFTf4d_n : $@convention(thin) () -> Int32 {
bb0:
%0 = integer_literal $Builtin.Int32, 10
%1 = struct $Int32 (%0 : $Builtin.Int32)
return %1 : $Int32
}

sil_global hidden [let] @$global_var : $P

// CHECK-LABEL: sil hidden [noinline] @$helper : $@convention(thin) (@in P) -> Int32 {
// CHECK: bb0(%0 : $*P):
// CHECK: debug_value_addr
// CHECK: alloc_stack
// CHECK: copy_addr
// CHECK: destroy_addr
// CHECK: open_existential_addr
// CHECK: witness_method
// CHECK: apply
// CHECK: dealloc_stack
// CHECK: return
// CHECK-LABEL: } // end sil function '$helper'
sil hidden [noinline] @$helper : $@convention(thin) (@in P) -> Int32 {
bb0(%0 : $*P):
debug_value_addr %0 : $*P, var, name "a", argno 1
%4 = alloc_stack $P
copy_addr %0 to [initialization] %4 : $*P
destroy_addr %0 : $*P
%6 = open_existential_addr immutable_access %4 : $*P to $*@opened("3CB58EC4-ECED-11E8-9798-D0817AD4059B") P
%7 = witness_method $@opened("3CB58EC4-ECED-11E8-9798-D0817AD4059B") P, #P.foo!1 : <Self where Self : P> (Self) -> () -> Int32, %6 : $*@opened("3CB58EC4-ECED-11E8-9798-D0817AD4059B") P : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> Int32
%8 = apply %7<@opened("3CB58EC4-ECED-11E8-9798-D0817AD4059B") P>(%6) : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> Int32
dealloc_stack %4 : $*P
return %8 : $Int32
}

sil @global_addr_init: $@convention(thin) (Builtin.Int1) -> Int32 {
bb0(%0 : $Builtin.Int1):
alloc_global @$global_var
%1 = global_addr @$global_var : $*P
cond_br %0, bb1, bb2

bb1:
%2 = init_existential_addr %1 : $*P, $Klass1
%3 = alloc_ref $Klass1
store %3 to %2 : $*Klass1
br bb3

bb2:
%5 = init_existential_addr %1 : $*P, $Klass2
%6 = alloc_ref $Klass2
store %6 to %5 : $*Klass2
br bb3

bb3:
%12 = function_ref @$helper : $@convention(thin) (@in P) -> Int32
%13 = apply %12(%1) : $@convention(thin) (@in P) -> Int32
return %13 : $Int32
}

// CHECK-LABEL: sil shared [noinline] @$s7dealloc12wrap_foo_ncp1a1bSiAA1P_pz_AaE_pztFTf4ee_n : $@convention(thin) <τ_0_0, τ_0_1 where τ_0_0 : P, τ_0_1 : P> (@in τ_0_0, @in τ_0_1) -> Int32 {
// CHECK: bb0(%0 : $*τ_0_0, %1 : $*τ_0_1):
// CHECK: alloc_stack
// CHECK: init_existential_addr
// CHECK: copy_addr
// CHECK: destroy_addr
// CHECK: alloc_stack
// CHECK: init_existential_addr
// CHECK: copy_addr
// CHECK: destroy_addr
// CHECK: debug_value_addr
// CHECK: debug_value_addr
// CHECK: alloc_stack
// CHECK: copy_addr
// CHECK: destroy_addr
// CHECK: open_existential_addr
// CHECK: witness_method
// CHECK: apply
// CHECK: alloc_stack
// CHECK: copy_addr
// CHECK: destroy_addr
// CHECK: open_existential_addr
// CHECK: witness_method
// CHECK: apply
// CHECK: struct_extract
// CHECK: struct_extract
// CHECK: integer_literal
// CHECK: builtin
// CHECK: tuple_extract
// CHECK: tuple_extract
// CHECK: cond_fail
// CHECK: struct
// CHECK: dealloc_stack
// CHECK: dealloc_stack
// CHECK: dealloc_stack
// CHECK: dealloc_stack
// CHECK: return
// CHECK-LABEL: } // end sil function '$s7dealloc12wrap_foo_ncp1a1bSiAA1P_pz_AaE_pztFTf4ee_n'
sil hidden [noinline] @$s7dealloc12wrap_foo_ncp1a1bSiAA1P_pz_AaE_pztF : $@convention(thin) (@in P, @in P) -> Int32 {
bb0(%0 : $*P, %1 : $*P):
debug_value_addr %0 : $*P, var, name "a", argno 1
debug_value_addr %1 : $*P, var, name "b", argno 2
%4 = alloc_stack $P
copy_addr %0 to [initialization] %4 : $*P
destroy_addr %0 : $*P
%6 = open_existential_addr immutable_access %4 : $*P to $*@opened("3CB58EC4-ECED-11E8-9798-D0817AD4059B") P
%7 = witness_method $@opened("3CB58EC4-ECED-11E8-9798-D0817AD4059B") P, #P.foo!1 : <Self where Self : P> (Self) -> () -> Int32, %6 : $*@opened("3CB58EC4-ECED-11E8-9798-D0817AD4059B") P : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> Int32
%8 = apply %7<@opened("3CB58EC4-ECED-11E8-9798-D0817AD4059B") P>(%6) : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> Int32
%9 = alloc_stack $P
copy_addr %1 to [initialization] %9 : $*P
destroy_addr %1 : $*P
%11 = open_existential_addr immutable_access %9 : $*P to $*@opened("3CB58FAA-ECED-11E8-9798-D0817AD4059B") P
%12 = witness_method $@opened("3CB58FAA-ECED-11E8-9798-D0817AD4059B") P, #P.foo!1 : <Self where Self : P> (Self) -> () -> Int32, %11 : $*@opened("3CB58FAA-ECED-11E8-9798-D0817AD4059B") P : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> Int32
%13 = apply %12<@opened("3CB58FAA-ECED-11E8-9798-D0817AD4059B") P>(%11) : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> Int32
%14 = struct_extract %8 : $Int32, #Int32._value
%15 = struct_extract %13 : $Int32, #Int32._value
%16 = integer_literal $Builtin.Int1, -1
%17 = builtin "sadd_with_overflow_Int32"(%14 : $Builtin.Int32, %15 : $Builtin.Int32, %16 : $Builtin.Int1) : $(Builtin.Int32, Builtin.Int1)
%18 = tuple_extract %17 : $(Builtin.Int32, Builtin.Int1), 0
%19 = tuple_extract %17 : $(Builtin.Int32, Builtin.Int1), 1
cond_fail %19 : $Builtin.Int1
%21 = struct $Int32 (%18 : $Builtin.Int32)
dealloc_stack %9 : $*P
dealloc_stack %4 : $*P
return %21 : $Int32
}

sil_witness_table hidden Klass1: P module dealloc {
method #P.foo!1: <Self where Self : P> (Self) -> () -> Int32 : nil
}

sil_witness_table hidden Klass2: P module dealloc {
method #P.foo!1: <Self where Self : P> (Self) -> () -> Int32 : nil
}
6 changes: 6 additions & 0 deletions tools/sil-opt/SILOpt.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,11 @@ static llvm::cl::opt<int>
SILInlineThreshold("sil-inline-threshold", llvm::cl::Hidden,
llvm::cl::init(-1));

static llvm::cl::opt<bool>
SILExistentialSpecializer("enable-sil-existential-specializer",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a flag to enable and disable existential-specializer from sil-opt.

llvm::cl::Hidden,
llvm::cl::init(false));

static llvm::cl::opt<bool>
EnableSILVerifyAll("enable-sil-verify-all",
llvm::cl::Hidden,
Expand Down Expand Up @@ -328,6 +333,7 @@ int main(int argc, char **argv) {
// Setup the SIL Options.
SILOptions &SILOpts = Invocation.getSILOptions();
SILOpts.InlineThreshold = SILInlineThreshold;
SILOpts.ExistentialSpecializer = SILExistentialSpecializer;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Store it in the module's Options.

SILOpts.VerifyAll = EnableSILVerifyAll;
SILOpts.RemoveRuntimeAsserts = RemoveRuntimeAsserts;
SILOpts.AssertConfig = AssertConfId;
Expand Down