From d9b95a026f115c2981a80dee547e8cd4d871d3d9 Mon Sep 17 00:00:00 2001 From: zoecarver Date: Sun, 3 Nov 2019 11:56:11 -0800 Subject: [PATCH 01/36] Implement devirtualization calls to composition type protocols --- lib/AST/SubstitutionMap.cpp | 2 ++ .../SILCombiner/SILCombinerApplyVisitors.cpp | 16 ++++++++++++++++ .../SILCombiner/SILCombinerCastVisitors.cpp | 3 ++- .../SILCombiner/SILCombinerMiscVisitors.cpp | 5 ++++- lib/SILOptimizer/Utils/Existential.cpp | 9 +++++++++ lib/SILOptimizer/Utils/InstOptUtils.cpp | 6 ++++-- 6 files changed, 37 insertions(+), 4 deletions(-) diff --git a/lib/AST/SubstitutionMap.cpp b/lib/AST/SubstitutionMap.cpp index 682ea0cc29e1d..52a9e5f472340 100644 --- a/lib/AST/SubstitutionMap.cpp +++ b/lib/AST/SubstitutionMap.cpp @@ -33,6 +33,8 @@ #include "swift/AST/Types.h" #include "llvm/Support/Debug.h" +#include + using namespace swift; SubstitutionMap::Storage::Storage( diff --git a/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp b/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp index 42bd42c43c75d..68e4e9d248383 100644 --- a/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp +++ b/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp @@ -33,6 +33,8 @@ #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/Statistic.h" +#include + using namespace swift; using namespace swift::PatternMatch; @@ -985,6 +987,7 @@ void SILCombiner::buildConcreteOpenedExistentialInfos( SILOpenedArchetypesTracker &OpenedArchetypesTracker) { for (unsigned ArgIdx = 0; ArgIdx < Apply.getNumArguments(); ArgIdx++) { auto ArgASTType = Apply.getArgument(ArgIdx)->getType().getASTType(); + ArgASTType->dump(); if (!ArgASTType->hasArchetype()) continue; @@ -1332,10 +1335,13 @@ SILInstruction *SILCombiner::createApplyWithConcreteType( SILInstruction * SILCombiner::propagateConcreteTypeOfInitExistential(FullApplySite Apply, WitnessMethodInst *WMI) { + WMI->getConformance().dump(); + // Check if it is legal to perform the propagation. if (WMI->getConformance().isConcrete()) return nullptr; + WMI->getLookupType()->dump(); // If the lookup type is not an opened existential type, // it cannot be made more concrete. if (!WMI->getLookupType()->isOpenedExistential()) @@ -1358,6 +1364,8 @@ SILCombiner::propagateConcreteTypeOfInitExistential(FullApplySite Apply, // Bail, if no argument has a concrete existential to propagate. if (COEIs.empty()) return nullptr; + + std::cout << "here" << std::endl; auto SelfCOEIIt = COEIs.find(Apply.getCalleeArgIndex(Apply.getSelfArgumentOperand())); @@ -1396,6 +1404,12 @@ SILCombiner::propagateConcreteTypeOfInitExistential(FullApplySite Apply, replaceWitnessMethodInst(WMI, BuilderCtx, SelfCEI.ConcreteType, SelfConformance); } + + std::cout << "done" << std::endl; + + std::cout << std::endl << "FULL FN:" << std::endl; + Apply.getFunction()->dump(); + std::cout << std::endl; /// Create the new apply instruction using concrete types for arguments. return createApplyWithConcreteType(Apply, COEIs, BuilderCtx); @@ -1671,6 +1685,8 @@ SILInstruction *SILCombiner::visitApplyInst(ApplyInst *AI) { // (apply (witness_method)) -> propagate information about // a concrete type from init_existential_addr or init_existential_ref. if (auto *WMI = dyn_cast(AI->getCallee())) { + AI->dump(); + WMI->dump(); if (propagateConcreteTypeOfInitExistential(AI, WMI)) { return nullptr; } diff --git a/lib/SILOptimizer/SILCombiner/SILCombinerCastVisitors.cpp b/lib/SILOptimizer/SILCombiner/SILCombinerCastVisitors.cpp index eee7bf3a22fe2..be18eb13a6f85 100644 --- a/lib/SILOptimizer/SILCombiner/SILCombinerCastVisitors.cpp +++ b/lib/SILOptimizer/SILCombiner/SILCombinerCastVisitors.cpp @@ -49,9 +49,10 @@ SILCombiner::visitRefToRawPointerInst(RefToRawPointerInst *RRPI) { // (ref_to_raw_pointer (open_existential_ref (init_existential_ref x))) -> // (ref_to_raw_pointer x) if (auto *OER = dyn_cast(RRPI->getOperand())) - if (auto *IER = dyn_cast(OER->getOperand())) + if (auto *IER = dyn_cast(OER->getOperand())) { return Builder.createRefToRawPointer(RRPI->getLoc(), IER->getOperand(), RRPI->getType()); + } return nullptr; } diff --git a/lib/SILOptimizer/SILCombiner/SILCombinerMiscVisitors.cpp b/lib/SILOptimizer/SILCombiner/SILCombinerMiscVisitors.cpp index aa41f2c8d55b2..fd464ea927e6c 100644 --- a/lib/SILOptimizer/SILCombiner/SILCombinerMiscVisitors.cpp +++ b/lib/SILOptimizer/SILCombiner/SILCombinerMiscVisitors.cpp @@ -1255,12 +1255,15 @@ SILInstruction *SILCombiner::visitStrongReleaseInst(StrongReleaseInst *SRI) { // Release of a classbound existential converted from a class is just a // release of the class, squish the conversion. - if (auto ier = dyn_cast(SRI->getOperand())) + if (auto ier = dyn_cast(SRI->getOperand())) { +// ier->dump(); +// ier->getOperand()->dump(); if (ier->hasOneUse()) { SRI->setOperand(ier->getOperand()); eraseInstFromFunction(*ier); return SRI; } + } return nullptr; } diff --git a/lib/SILOptimizer/Utils/Existential.cpp b/lib/SILOptimizer/Utils/Existential.cpp index 012f8aa3f9e32..8c6004eb7501a 100644 --- a/lib/SILOptimizer/Utils/Existential.cpp +++ b/lib/SILOptimizer/Utils/Existential.cpp @@ -19,6 +19,8 @@ #include "swift/SILOptimizer/Utils/InstOptUtils.h" #include "llvm/ADT/SmallPtrSet.h" +#include + using namespace swift; /// Determine InitExistential from global_addr. @@ -203,6 +205,8 @@ static SILValue getAddressOfStackInit(SILValue allocStackAddr, OpenedArchetypeInfo::OpenedArchetypeInfo(Operand &use) { SILValue openedVal = use.get(); SILInstruction *user = use.getUser(); + openedVal->dump(); + user->dump(); if (auto *instance = dyn_cast(openedVal)) { // Handle: // %opened = open_existential_addr @@ -213,6 +217,11 @@ OpenedArchetypeInfo::OpenedArchetypeInfo(Operand &use) { getAddressOfStackInit(instance, user, isOpenedValueCopied)) { openedVal = stackInitVal; } + + auto firstUse = *instance->getUses().begin(); + if (auto *store = dyn_cast(firstUse->getUser())) { + openedVal = store->getSrc(); + } } if (auto *Open = dyn_cast(openedVal)) { OpenedArchetype = Open->getType().castTo(); diff --git a/lib/SILOptimizer/Utils/InstOptUtils.cpp b/lib/SILOptimizer/Utils/InstOptUtils.cpp index e952b85f57dbc..6877fc26a9308 100644 --- a/lib/SILOptimizer/Utils/InstOptUtils.cpp +++ b/lib/SILOptimizer/Utils/InstOptUtils.cpp @@ -472,8 +472,10 @@ SILValue swift::castValueToABICompatibleType(SILBuilder *builder, if (srcTy == destTy) return value; - assert(srcTy.isAddress() == destTy.isAddress() - && "Addresses aren't compatible with values"); +// assert(srcTy.isAddress() == destTy.isAddress() +// && "Addresses aren't compatible with values"); + + if (srcTy.isAddress() == destTy.isAddress()) return nullptr; if (srcTy.isAddress() && destTy.isAddress()) { // Cast between two addresses and that's it. From bfb3dd72d779a13afd49a28b365c66e1b90f49af Mon Sep 17 00:00:00 2001 From: zoecarver Date: Sun, 3 Nov 2019 12:50:26 -0800 Subject: [PATCH 02/36] Clean up implementation and remove debug --- lib/AST/SubstitutionMap.cpp | 2 -- .../SILCombiner/SILCombinerApplyVisitors.cpp | 17 ----------------- .../SILCombiner/SILCombinerCastVisitors.cpp | 3 +-- .../SILCombiner/SILCombinerMiscVisitors.cpp | 5 +---- lib/SILOptimizer/Utils/Existential.cpp | 8 ++++---- lib/SILOptimizer/Utils/InstOptUtils.cpp | 3 --- 6 files changed, 6 insertions(+), 32 deletions(-) diff --git a/lib/AST/SubstitutionMap.cpp b/lib/AST/SubstitutionMap.cpp index 52a9e5f472340..682ea0cc29e1d 100644 --- a/lib/AST/SubstitutionMap.cpp +++ b/lib/AST/SubstitutionMap.cpp @@ -33,8 +33,6 @@ #include "swift/AST/Types.h" #include "llvm/Support/Debug.h" -#include - using namespace swift; SubstitutionMap::Storage::Storage( diff --git a/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp b/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp index 68e4e9d248383..bae584a21071c 100644 --- a/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp +++ b/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp @@ -33,8 +33,6 @@ #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/Statistic.h" -#include - using namespace swift; using namespace swift::PatternMatch; @@ -987,7 +985,6 @@ void SILCombiner::buildConcreteOpenedExistentialInfos( SILOpenedArchetypesTracker &OpenedArchetypesTracker) { for (unsigned ArgIdx = 0; ArgIdx < Apply.getNumArguments(); ArgIdx++) { auto ArgASTType = Apply.getArgument(ArgIdx)->getType().getASTType(); - ArgASTType->dump(); if (!ArgASTType->hasArchetype()) continue; @@ -1335,13 +1332,9 @@ SILInstruction *SILCombiner::createApplyWithConcreteType( SILInstruction * SILCombiner::propagateConcreteTypeOfInitExistential(FullApplySite Apply, WitnessMethodInst *WMI) { - WMI->getConformance().dump(); - // Check if it is legal to perform the propagation. if (WMI->getConformance().isConcrete()) return nullptr; - - WMI->getLookupType()->dump(); // If the lookup type is not an opened existential type, // it cannot be made more concrete. if (!WMI->getLookupType()->isOpenedExistential()) @@ -1364,8 +1357,6 @@ SILCombiner::propagateConcreteTypeOfInitExistential(FullApplySite Apply, // Bail, if no argument has a concrete existential to propagate. if (COEIs.empty()) return nullptr; - - std::cout << "here" << std::endl; auto SelfCOEIIt = COEIs.find(Apply.getCalleeArgIndex(Apply.getSelfArgumentOperand())); @@ -1404,12 +1395,6 @@ SILCombiner::propagateConcreteTypeOfInitExistential(FullApplySite Apply, replaceWitnessMethodInst(WMI, BuilderCtx, SelfCEI.ConcreteType, SelfConformance); } - - std::cout << "done" << std::endl; - - std::cout << std::endl << "FULL FN:" << std::endl; - Apply.getFunction()->dump(); - std::cout << std::endl; /// Create the new apply instruction using concrete types for arguments. return createApplyWithConcreteType(Apply, COEIs, BuilderCtx); @@ -1685,8 +1670,6 @@ SILInstruction *SILCombiner::visitApplyInst(ApplyInst *AI) { // (apply (witness_method)) -> propagate information about // a concrete type from init_existential_addr or init_existential_ref. if (auto *WMI = dyn_cast(AI->getCallee())) { - AI->dump(); - WMI->dump(); if (propagateConcreteTypeOfInitExistential(AI, WMI)) { return nullptr; } diff --git a/lib/SILOptimizer/SILCombiner/SILCombinerCastVisitors.cpp b/lib/SILOptimizer/SILCombiner/SILCombinerCastVisitors.cpp index be18eb13a6f85..eee7bf3a22fe2 100644 --- a/lib/SILOptimizer/SILCombiner/SILCombinerCastVisitors.cpp +++ b/lib/SILOptimizer/SILCombiner/SILCombinerCastVisitors.cpp @@ -49,10 +49,9 @@ SILCombiner::visitRefToRawPointerInst(RefToRawPointerInst *RRPI) { // (ref_to_raw_pointer (open_existential_ref (init_existential_ref x))) -> // (ref_to_raw_pointer x) if (auto *OER = dyn_cast(RRPI->getOperand())) - if (auto *IER = dyn_cast(OER->getOperand())) { + if (auto *IER = dyn_cast(OER->getOperand())) return Builder.createRefToRawPointer(RRPI->getLoc(), IER->getOperand(), RRPI->getType()); - } return nullptr; } diff --git a/lib/SILOptimizer/SILCombiner/SILCombinerMiscVisitors.cpp b/lib/SILOptimizer/SILCombiner/SILCombinerMiscVisitors.cpp index fd464ea927e6c..aa41f2c8d55b2 100644 --- a/lib/SILOptimizer/SILCombiner/SILCombinerMiscVisitors.cpp +++ b/lib/SILOptimizer/SILCombiner/SILCombinerMiscVisitors.cpp @@ -1255,15 +1255,12 @@ SILInstruction *SILCombiner::visitStrongReleaseInst(StrongReleaseInst *SRI) { // Release of a classbound existential converted from a class is just a // release of the class, squish the conversion. - if (auto ier = dyn_cast(SRI->getOperand())) { -// ier->dump(); -// ier->getOperand()->dump(); + if (auto ier = dyn_cast(SRI->getOperand())) if (ier->hasOneUse()) { SRI->setOperand(ier->getOperand()); eraseInstFromFunction(*ier); return SRI; } - } return nullptr; } diff --git a/lib/SILOptimizer/Utils/Existential.cpp b/lib/SILOptimizer/Utils/Existential.cpp index 8c6004eb7501a..77fd94461208c 100644 --- a/lib/SILOptimizer/Utils/Existential.cpp +++ b/lib/SILOptimizer/Utils/Existential.cpp @@ -19,8 +19,6 @@ #include "swift/SILOptimizer/Utils/InstOptUtils.h" #include "llvm/ADT/SmallPtrSet.h" -#include - using namespace swift; /// Determine InitExistential from global_addr. @@ -205,8 +203,6 @@ static SILValue getAddressOfStackInit(SILValue allocStackAddr, OpenedArchetypeInfo::OpenedArchetypeInfo(Operand &use) { SILValue openedVal = use.get(); SILInstruction *user = use.getUser(); - openedVal->dump(); - user->dump(); if (auto *instance = dyn_cast(openedVal)) { // Handle: // %opened = open_existential_addr @@ -218,6 +214,10 @@ OpenedArchetypeInfo::OpenedArchetypeInfo(Operand &use) { openedVal = stackInitVal; } + // Handle: + // %4 = open_existential_ref + // %5 = alloc_stack + // store %4 to %5 auto firstUse = *instance->getUses().begin(); if (auto *store = dyn_cast(firstUse->getUser())) { openedVal = store->getSrc(); diff --git a/lib/SILOptimizer/Utils/InstOptUtils.cpp b/lib/SILOptimizer/Utils/InstOptUtils.cpp index 6877fc26a9308..a841fff5b5207 100644 --- a/lib/SILOptimizer/Utils/InstOptUtils.cpp +++ b/lib/SILOptimizer/Utils/InstOptUtils.cpp @@ -471,9 +471,6 @@ SILValue swift::castValueToABICompatibleType(SILBuilder *builder, // No cast is required if types are the same. if (srcTy == destTy) return value; - -// assert(srcTy.isAddress() == destTy.isAddress() -// && "Addresses aren't compatible with values"); if (srcTy.isAddress() == destTy.isAddress()) return nullptr; From 287bbd4df40cdf96549894b4c378a55ac9c5e26e Mon Sep 17 00:00:00 2001 From: zoecarver Date: Sun, 3 Nov 2019 13:01:52 -0800 Subject: [PATCH 03/36] Add a test --- .../devirtualize_protocol_composition.swift | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 test/SILOptimizer/devirtualize_protocol_composition.swift diff --git a/test/SILOptimizer/devirtualize_protocol_composition.swift b/test/SILOptimizer/devirtualize_protocol_composition.swift new file mode 100644 index 0000000000000..b4f939ec44264 --- /dev/null +++ b/test/SILOptimizer/devirtualize_protocol_composition.swift @@ -0,0 +1,31 @@ +// RUN: %target-swift-frontend -parse-as-library -O -wmo -emit-sil %s | %FileCheck %s +// RUN: %target-swift-frontend -parse-as-library -Osize -wmo -emit-sil %s | %FileCheck %s + +// This is an end-to-end test to ensure that the optimizer devertualizes +// calls to a protocol composition type. + +public class ClassA { } + +protocol ProtocolA { + func foo() -> Int +} + +func shouldOptimizeWitness(_ x: ClassA & ProtocolA) -> Int { + return x.foo() +} + +public class ClassB: ClassA { + func foo() -> Int { + return 10 + } +} +extension ClassB: ProtocolA { } + +//CHECK: witnessEntryPoint +//CHECK-NOT: init_existential_ref +//CHECK-NOT: open_existential_ref +//CHECK-NOT: witness_method +//CHECK: return +public func witnessEntryPoint(c: ClassB) -> Int { + return shouldOptimizeWitness(c) +} From f3ac3bd08b6bbece20fd20567963590ef63b5432 Mon Sep 17 00:00:00 2001 From: zoecarver Date: Sun, 3 Nov 2019 13:03:05 -0800 Subject: [PATCH 04/36] Cleanup and fix spacing (again) --- lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp | 1 + lib/SILOptimizer/Utils/Existential.cpp | 2 +- lib/SILOptimizer/Utils/InstOptUtils.cpp | 5 +++-- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp b/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp index bae584a21071c..42bd42c43c75d 100644 --- a/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp +++ b/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp @@ -1335,6 +1335,7 @@ SILCombiner::propagateConcreteTypeOfInitExistential(FullApplySite Apply, // Check if it is legal to perform the propagation. if (WMI->getConformance().isConcrete()) return nullptr; + // If the lookup type is not an opened existential type, // it cannot be made more concrete. if (!WMI->getLookupType()->isOpenedExistential()) diff --git a/lib/SILOptimizer/Utils/Existential.cpp b/lib/SILOptimizer/Utils/Existential.cpp index 77fd94461208c..44ecbafbac618 100644 --- a/lib/SILOptimizer/Utils/Existential.cpp +++ b/lib/SILOptimizer/Utils/Existential.cpp @@ -213,7 +213,7 @@ OpenedArchetypeInfo::OpenedArchetypeInfo(Operand &use) { getAddressOfStackInit(instance, user, isOpenedValueCopied)) { openedVal = stackInitVal; } - + // Handle: // %4 = open_existential_ref // %5 = alloc_stack diff --git a/lib/SILOptimizer/Utils/InstOptUtils.cpp b/lib/SILOptimizer/Utils/InstOptUtils.cpp index a841fff5b5207..7803a27d60fd6 100644 --- a/lib/SILOptimizer/Utils/InstOptUtils.cpp +++ b/lib/SILOptimizer/Utils/InstOptUtils.cpp @@ -471,8 +471,9 @@ SILValue swift::castValueToABICompatibleType(SILBuilder *builder, // No cast is required if types are the same. if (srcTy == destTy) return value; - - if (srcTy.isAddress() == destTy.isAddress()) return nullptr; + + if (srcTy.isAddress() == destTy.isAddress()) + return nullptr; if (srcTy.isAddress() && destTy.isAddress()) { // Cast between two addresses and that's it. From f1c75f6d628fa47996cf1bafba7ed51162bd626f Mon Sep 17 00:00:00 2001 From: zoecarver Date: Mon, 4 Nov 2019 11:01:39 -0800 Subject: [PATCH 05/36] Remove assertion all together and use getAddressOfStackInit instead of dyn_cast to StoreInst --- lib/SILOptimizer/Utils/Existential.cpp | 5 +++-- lib/SILOptimizer/Utils/InstOptUtils.cpp | 3 --- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/SILOptimizer/Utils/Existential.cpp b/lib/SILOptimizer/Utils/Existential.cpp index 44ecbafbac618..10f03cbe368f1 100644 --- a/lib/SILOptimizer/Utils/Existential.cpp +++ b/lib/SILOptimizer/Utils/Existential.cpp @@ -219,8 +219,9 @@ OpenedArchetypeInfo::OpenedArchetypeInfo(Operand &use) { // %5 = alloc_stack // store %4 to %5 auto firstUse = *instance->getUses().begin(); - if (auto *store = dyn_cast(firstUse->getUser())) { - openedVal = store->getSrc(); + if (auto stackInitVal = + getAddressOfStackInit(instance, firstUse->getUser(), isOpenedValueCopied)) { + openedVal = stackInitVal; } } if (auto *Open = dyn_cast(openedVal)) { diff --git a/lib/SILOptimizer/Utils/InstOptUtils.cpp b/lib/SILOptimizer/Utils/InstOptUtils.cpp index 7803a27d60fd6..ab7a1d013c025 100644 --- a/lib/SILOptimizer/Utils/InstOptUtils.cpp +++ b/lib/SILOptimizer/Utils/InstOptUtils.cpp @@ -472,9 +472,6 @@ SILValue swift::castValueToABICompatibleType(SILBuilder *builder, if (srcTy == destTy) return value; - if (srcTy.isAddress() == destTy.isAddress()) - return nullptr; - if (srcTy.isAddress() && destTy.isAddress()) { // Cast between two addresses and that's it. return builder->createUncheckedAddrCast(loc, value, destTy); From 4b36f3fd69d10fcc2e1ba405847f0da8747359c6 Mon Sep 17 00:00:00 2001 From: zoecarver Date: Mon, 4 Nov 2019 12:27:35 -0800 Subject: [PATCH 06/36] Fix based on review and add more tests --- .../SILCombiner/SILCombinerApplyVisitors.cpp | 2 +- .../devirtualize_protocol_composition.swift | 113 +++++++++++++++++- 2 files changed, 109 insertions(+), 6 deletions(-) diff --git a/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp b/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp index 42bd42c43c75d..6d68082542653 100644 --- a/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp +++ b/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp @@ -907,7 +907,7 @@ SILCombiner::buildConcreteOpenedExistentialInfoFromSoleConformingType( (archetypeTy->getConformsTo().size() == 1)) { PD = archetypeTy->getConformsTo()[0]; } else if (ArgType.isExistentialType() && !ArgType.isAnyObject() && - !SwiftArgType->isAny()) { + !SwiftArgType->isAny() && SwiftArgType->getAnyNominal()) { PD = dyn_cast(SwiftArgType->getAnyNominal()); } } diff --git a/test/SILOptimizer/devirtualize_protocol_composition.swift b/test/SILOptimizer/devirtualize_protocol_composition.swift index b4f939ec44264..f4b444b758a6d 100644 --- a/test/SILOptimizer/devirtualize_protocol_composition.swift +++ b/test/SILOptimizer/devirtualize_protocol_composition.swift @@ -10,8 +10,8 @@ protocol ProtocolA { func foo() -> Int } -func shouldOptimizeWitness(_ x: ClassA & ProtocolA) -> Int { - return x.foo() +protocol ProtocolB { + func bar() -> Int } public class ClassB: ClassA { @@ -19,13 +19,116 @@ public class ClassB: ClassA { return 10 } } + extension ClassB: ProtocolA { } -//CHECK: witnessEntryPoint +public class ClassC: ClassA { + func foo() -> Int { + return 10 + } +} + +extension ClassC: ProtocolA { } + +public class ClassD { } +public class ClassE : ClassD { + func foo() -> Int { + return 10 + } +} + +extension ClassE: ProtocolA { } + +public class ClassF { + func foo() -> Int { + return 10 + } + + func bar() -> Int { + return 10 + } +} + +extension ClassF: ProtocolA, ProtocolB { } + +public class ClassG { + func foo() -> Int { + return 10 + } + + func bar() -> Int { + return 10 + } +} + +extension ClassG: ProtocolA, ProtocolB { } + +public class ClassH { + typealias type = ClassD +} + +func shouldOptimize1(_ x: ClassA & ProtocolA) -> Int { + return x.foo() +} + +func shouldOptimize2(_ x: ClassD & ProtocolA) -> Int { + return x.foo() +} + +func shouldOptimize3(_ x: ProtocolA & ProtocolB) -> Int { + return x.foo() + x.bar() +} + +func shouldOptimize4(_ x: ClassH.type & ProtocolA) -> Int { + return x.foo() +} + +//CHECK: entryPoint1 +//CHECK-NOT: init_existential_ref +//CHECK-NOT: open_existential_ref +//CHECK-NOT: witness_method +//CHECK: return +public func entryPoint1(c: ClassB) -> Int { + return shouldOptimize1(c) +} + +// TODO: is this broken on master too? +//public func entryPoint2(c: ClassC) -> Int { +// return shouldOptimize1(c) +//} + +//CHECK: entryPoint3 +//CHECK-NOT: init_existential_ref +//CHECK-NOT: open_existential_ref +//CHECK-NOT: witness_method +//CHECK: return +public func entryPoint3(c: ClassE) -> Int { + return shouldOptimize2(c) +} + +//CHECK: entryPoint4 +//CHECK-NOT: init_existential_ref +//CHECK-NOT: open_existential_ref +//CHECK-NOT: witness_method +//CHECK: return +public func entryPoint4(c: ClassF) -> Int { + return shouldOptimize3(c) +} + +//CHECK: entryPoint5 +//CHECK-NOT: init_existential_ref +//CHECK-NOT: open_existential_ref +//CHECK-NOT: witness_method +//CHECK: return +public func entryPoint5(c: ClassG) -> Int { + return shouldOptimize3(c) +} + +//CHECK: entryPoint6 //CHECK-NOT: init_existential_ref //CHECK-NOT: open_existential_ref //CHECK-NOT: witness_method //CHECK: return -public func witnessEntryPoint(c: ClassB) -> Int { - return shouldOptimizeWitness(c) +public func entryPoint6(c: ClassE) -> Int { + return shouldOptimize4(c) } From f1c15d26f30ca5abfffe633daf0f97ab0bda2d8c Mon Sep 17 00:00:00 2001 From: zoecarver Date: Mon, 4 Nov 2019 13:06:38 -0800 Subject: [PATCH 07/36] Fix tests --- lib/SILOptimizer/Utils/Existential.cpp | 9 ++-- .../devirtualize_protocol_composition.swift | 46 +++++++++++++++---- 2 files changed, 43 insertions(+), 12 deletions(-) diff --git a/lib/SILOptimizer/Utils/Existential.cpp b/lib/SILOptimizer/Utils/Existential.cpp index 10f03cbe368f1..75310c4fc4d74 100644 --- a/lib/SILOptimizer/Utils/Existential.cpp +++ b/lib/SILOptimizer/Utils/Existential.cpp @@ -219,10 +219,13 @@ OpenedArchetypeInfo::OpenedArchetypeInfo(Operand &use) { // %5 = alloc_stack // store %4 to %5 auto firstUse = *instance->getUses().begin(); - if (auto stackInitVal = - getAddressOfStackInit(instance, firstUse->getUser(), isOpenedValueCopied)) { - openedVal = stackInitVal; + if (auto *store = dyn_cast(firstUse->getUser())) { + openedVal = store->getSrc(); } +// if (auto stackInitVal = +// getAddressOfStackInit(instance, firstUse->getUser(), isOpenedValueCopied)) { +// openedVal = stackInitVal; +// } } if (auto *Open = dyn_cast(openedVal)) { OpenedArchetype = Open->getType().castTo(); diff --git a/test/SILOptimizer/devirtualize_protocol_composition.swift b/test/SILOptimizer/devirtualize_protocol_composition.swift index f4b444b758a6d..73c4bcc8be0f7 100644 --- a/test/SILOptimizer/devirtualize_protocol_composition.swift +++ b/test/SILOptimizer/devirtualize_protocol_composition.swift @@ -14,6 +14,30 @@ protocol ProtocolB { func bar() -> Int } +protocol ProtocolC { + func foo() -> Int +} + +protocol ProtocolD { + func foo() -> Int +} + +protocol ProtocolE { + func foo() -> Int +} + +protocol ProtocolF { + func foo() -> Int +} + +protocol ProtocolG { + func bar() -> Int +} + +protocol ProtocolH { + func bar() -> Int +} + public class ClassB: ClassA { func foo() -> Int { return 10 @@ -28,7 +52,7 @@ public class ClassC: ClassA { } } -extension ClassC: ProtocolA { } +extension ClassC: ProtocolC { } public class ClassD { } public class ClassE : ClassD { @@ -37,7 +61,7 @@ public class ClassE : ClassD { } } -extension ClassE: ProtocolA { } +extension ClassE: ProtocolD { } public class ClassF { func foo() -> Int { @@ -49,7 +73,7 @@ public class ClassF { } } -extension ClassF: ProtocolA, ProtocolB { } +extension ClassF: ProtocolE, ProtocolG { } public class ClassG { func foo() -> Int { @@ -61,7 +85,7 @@ public class ClassG { } } -extension ClassG: ProtocolA, ProtocolB { } +extension ClassG: ProtocolF, ProtocolH { } public class ClassH { typealias type = ClassD @@ -71,15 +95,19 @@ func shouldOptimize1(_ x: ClassA & ProtocolA) -> Int { return x.foo() } -func shouldOptimize2(_ x: ClassD & ProtocolA) -> Int { +func shouldOptimize2(_ x: ClassD & ProtocolD) -> Int { return x.foo() } -func shouldOptimize3(_ x: ProtocolA & ProtocolB) -> Int { +func shouldOptimize3(_ x: ProtocolE & ProtocolG) -> Int { return x.foo() + x.bar() } -func shouldOptimize4(_ x: ClassH.type & ProtocolA) -> Int { +func shouldOptimize4(_ x: ProtocolF & ProtocolH) -> Int { + return x.foo() + x.bar() +} + +func shouldOptimize5(_ x: ClassH.type & ProtocolD) -> Int { return x.foo() } @@ -121,7 +149,7 @@ public func entryPoint4(c: ClassF) -> Int { //CHECK-NOT: witness_method //CHECK: return public func entryPoint5(c: ClassG) -> Int { - return shouldOptimize3(c) + return shouldOptimize4(c) } //CHECK: entryPoint6 @@ -130,5 +158,5 @@ public func entryPoint5(c: ClassG) -> Int { //CHECK-NOT: witness_method //CHECK: return public func entryPoint6(c: ClassE) -> Int { - return shouldOptimize4(c) + return shouldOptimize5(c) } From 6c48a8d35fd5a821408ce29475db38b1fe72267a Mon Sep 17 00:00:00 2001 From: zoecarver Date: Mon, 4 Nov 2019 20:01:27 -0800 Subject: [PATCH 08/36] Ensure that only correct patterns are optimized --- lib/SILOptimizer/Utils/Existential.cpp | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/lib/SILOptimizer/Utils/Existential.cpp b/lib/SILOptimizer/Utils/Existential.cpp index 75310c4fc4d74..e29966d5507db 100644 --- a/lib/SILOptimizer/Utils/Existential.cpp +++ b/lib/SILOptimizer/Utils/Existential.cpp @@ -218,14 +218,27 @@ OpenedArchetypeInfo::OpenedArchetypeInfo(Operand &use) { // %4 = open_existential_ref // %5 = alloc_stack // store %4 to %5 - auto firstUse = *instance->getUses().begin(); - if (auto *store = dyn_cast(firstUse->getUser())) { + // apply (%5) + // It's important that the only uses of %5 (instance) are in + // a store and an apply. + StoreInst *store; + ApplyInst *apply; + bool shouldOptimize = true; + + for (auto *use : instance->getUses()) { + if (auto *foundStore = dyn_cast(use->getUser())) { store = foundStore; } + else if (auto *foundApply = dyn_cast(use->getUser())) { apply = foundApply; } + else if (auto *dealloc = dyn_cast(use->getUser())) { + shouldOptimize = store && apply; + } else { + // TODO: this may be too harsh + shouldOptimize = false; + } + } + + if (shouldOptimize) { openedVal = store->getSrc(); } -// if (auto stackInitVal = -// getAddressOfStackInit(instance, firstUse->getUser(), isOpenedValueCopied)) { -// openedVal = stackInitVal; -// } } if (auto *Open = dyn_cast(openedVal)) { OpenedArchetype = Open->getType().castTo(); From 01a452f0cb72a42a84f2af25c92e9d4cc0b4130c Mon Sep 17 00:00:00 2001 From: zoecarver Date: Mon, 4 Nov 2019 20:14:33 -0800 Subject: [PATCH 09/36] Update comment --- test/SILOptimizer/devirtualize_protocol_composition.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/SILOptimizer/devirtualize_protocol_composition.swift b/test/SILOptimizer/devirtualize_protocol_composition.swift index 73c4bcc8be0f7..14f9d51d458a6 100644 --- a/test/SILOptimizer/devirtualize_protocol_composition.swift +++ b/test/SILOptimizer/devirtualize_protocol_composition.swift @@ -120,7 +120,7 @@ public func entryPoint1(c: ClassB) -> Int { return shouldOptimize1(c) } -// TODO: is this broken on master too? +// TODO: create SR -- this causes a crash on master too //public func entryPoint2(c: ClassC) -> Int { // return shouldOptimize1(c) //} From 17894bb6eb8f85161bd3d6369693b3718ca70035 Mon Sep 17 00:00:00 2001 From: zoecarver Date: Mon, 4 Nov 2019 20:23:00 -0800 Subject: [PATCH 10/36] Remove redundant protocols --- .../devirtualize_protocol_composition.swift | 40 ++++--------------- 1 file changed, 8 insertions(+), 32 deletions(-) diff --git a/test/SILOptimizer/devirtualize_protocol_composition.swift b/test/SILOptimizer/devirtualize_protocol_composition.swift index 14f9d51d458a6..75d6d7bf5dbb0 100644 --- a/test/SILOptimizer/devirtualize_protocol_composition.swift +++ b/test/SILOptimizer/devirtualize_protocol_composition.swift @@ -14,30 +14,6 @@ protocol ProtocolB { func bar() -> Int } -protocol ProtocolC { - func foo() -> Int -} - -protocol ProtocolD { - func foo() -> Int -} - -protocol ProtocolE { - func foo() -> Int -} - -protocol ProtocolF { - func foo() -> Int -} - -protocol ProtocolG { - func bar() -> Int -} - -protocol ProtocolH { - func bar() -> Int -} - public class ClassB: ClassA { func foo() -> Int { return 10 @@ -52,7 +28,7 @@ public class ClassC: ClassA { } } -extension ClassC: ProtocolC { } +extension ClassC: ProtocolA { } public class ClassD { } public class ClassE : ClassD { @@ -61,7 +37,7 @@ public class ClassE : ClassD { } } -extension ClassE: ProtocolD { } +extension ClassE: ProtocolA { } public class ClassF { func foo() -> Int { @@ -73,7 +49,7 @@ public class ClassF { } } -extension ClassF: ProtocolE, ProtocolG { } +extension ClassF: ProtocolA, ProtocolB { } public class ClassG { func foo() -> Int { @@ -85,7 +61,7 @@ public class ClassG { } } -extension ClassG: ProtocolF, ProtocolH { } +extension ClassG: ProtocolA, ProtocolB { } public class ClassH { typealias type = ClassD @@ -95,19 +71,19 @@ func shouldOptimize1(_ x: ClassA & ProtocolA) -> Int { return x.foo() } -func shouldOptimize2(_ x: ClassD & ProtocolD) -> Int { +func shouldOptimize2(_ x: ClassD & ProtocolA) -> Int { return x.foo() } -func shouldOptimize3(_ x: ProtocolE & ProtocolG) -> Int { +func shouldOptimize3(_ x: ProtocolA & ProtocolB) -> Int { return x.foo() + x.bar() } -func shouldOptimize4(_ x: ProtocolF & ProtocolH) -> Int { +func shouldOptimize4(_ x: ProtocolA & ProtocolB) -> Int { return x.foo() + x.bar() } -func shouldOptimize5(_ x: ClassH.type & ProtocolD) -> Int { +func shouldOptimize5(_ x: ClassH.type & ProtocolA) -> Int { return x.foo() } From 2913dbc430579b9fba575204473f660d847b2604 Mon Sep 17 00:00:00 2001 From: zoecarver Date: Sun, 1 Dec 2019 10:50:18 -0800 Subject: [PATCH 11/36] Check dominance order and add .sil test --- .../SILCombiner/SILCombinerApplyVisitors.cpp | 1 + lib/SILOptimizer/Utils/Existential.cpp | 23 +- ...alize_protocol_composition_two_applies.sil | 301 ++++++++++++++++++ 3 files changed, 317 insertions(+), 8 deletions(-) create mode 100644 test/SILOptimizer/devirtualize_protocol_composition_two_applies.sil diff --git a/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp b/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp index 444e005e340a2..b36a4883d8616 100644 --- a/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp +++ b/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp @@ -1673,6 +1673,7 @@ SILInstruction *SILCombiner::visitApplyInst(ApplyInst *AI) { // a concrete type from init_existential_addr or init_existential_ref. if (auto *WMI = dyn_cast(AI->getCallee())) { if (propagateConcreteTypeOfInitExistential(AI, WMI)) { + Builder.getFunction().dump(); return nullptr; } } diff --git a/lib/SILOptimizer/Utils/Existential.cpp b/lib/SILOptimizer/Utils/Existential.cpp index 2a1dd48a110af..d270189f2da57 100644 --- a/lib/SILOptimizer/Utils/Existential.cpp +++ b/lib/SILOptimizer/Utils/Existential.cpp @@ -221,23 +221,30 @@ OpenedArchetypeInfo::OpenedArchetypeInfo(Operand &use) { // apply (%5) // It's important that the only uses of %5 (instance) are in // a store and an apply. - StoreInst *store; - ApplyInst *apply; + StoreInst *store = nullptr; + ApplyInst *apply = nullptr; + DeallocStackInst *dealloc = nullptr; bool shouldOptimize = true; for (auto *use : instance->getUses()) { - if (auto *foundStore = dyn_cast(use->getUser())) { store = foundStore; } - else if (auto *foundApply = dyn_cast(use->getUser())) { apply = foundApply; } - else if (auto *dealloc = dyn_cast(use->getUser())) { - shouldOptimize = store && apply; + if (auto *foundStore = dyn_cast(use->getUser())) { + store = foundStore; + } else if (auto *foundApply = dyn_cast(use->getUser())) { + apply = foundApply; + } else if (auto *foundDealloc = dyn_cast(use->getUser())) { + dealloc = foundDealloc; } else { // TODO: this may be too harsh shouldOptimize = false; } } - if (shouldOptimize) { - openedVal = store->getSrc(); + if (shouldOptimize && store && apply && dealloc) { + DominanceInfo domInfo(store->getFunction()); + bool correctOrder = domInfo.dominates(store, apply) && + domInfo.dominates(apply, dealloc); + if (correctOrder) + openedVal = store->getSrc(); } } if (auto *Open = dyn_cast(openedVal)) { diff --git a/test/SILOptimizer/devirtualize_protocol_composition_two_applies.sil b/test/SILOptimizer/devirtualize_protocol_composition_two_applies.sil new file mode 100644 index 0000000000000..f6a579c2c1c83 --- /dev/null +++ b/test/SILOptimizer/devirtualize_protocol_composition_two_applies.sil @@ -0,0 +1,301 @@ +// RUN: %target-build-swift %s -O -wmo -emit-sil | %FileCheck %s + +sil_stage canonical + +import Builtin +import Swift +import SwiftShims + +public class A { + @objc deinit + init() +} + +public protocol P { + func foo() -> Int +} + +public class B : A, P { + public func foo() -> Int + @objc deinit + override init() +} + +public class C : A, P { + public func foo() -> Int + @objc deinit + override init() +} + +func imp(x: A & P, y: A & P) -> Int + +public func test(x: B, y: C) -> Int + +// main +sil @main : $@convention(c) (Int32, UnsafeMutablePointer>>) -> Int32 { +bb0(%0 : $Int32, %1 : $UnsafeMutablePointer>>): + %2 = integer_literal $Builtin.Int32, 0 // user: %3 + %3 = struct $Int32 (%2 : $Builtin.Int32) // user: %4 + return %3 : $Int32 // id: %4 +} // end sil function 'main' + +// A.deinit +sil @$s3run1ACfd : $@convention(method) (@guaranteed A) -> @owned Builtin.NativeObject { +// %0 // users: %2, %1 +bb0(%0 : $A): + debug_value %0 : $A, let, name "self", argno 1 // id: %1 + %2 = unchecked_ref_cast %0 : $A to $Builtin.NativeObject // user: %3 + return %2 : $Builtin.NativeObject // id: %3 +} // end sil function '$s3run1ACfd' + +// A.__deallocating_deinit +sil @$s3run1ACfD : $@convention(method) (@owned A) -> () { +// %0 // users: %3, %1 +bb0(%0 : $A): + debug_value %0 : $A, let, name "self", argno 1 // id: %1 + // function_ref A.deinit + %2 = function_ref @$s3run1ACfd : $@convention(method) (@guaranteed A) -> @owned Builtin.NativeObject // user: %3 + %3 = apply %2(%0) : $@convention(method) (@guaranteed A) -> @owned Builtin.NativeObject // user: %4 + %4 = unchecked_ref_cast %3 : $Builtin.NativeObject to $A // user: %5 + dealloc_ref %4 : $A // id: %5 + %6 = tuple () // user: %7 + return %6 : $() // id: %7 +} // end sil function '$s3run1ACfD' + +// A.__allocating_init() +sil hidden [exact_self_class] @$s3run1ACACycfC : $@convention(method) (@thick A.Type) -> @owned A { +bb0(%0 : $@thick A.Type): + %1 = alloc_ref $A // user: %3 + // function_ref A.init() + %2 = function_ref @$s3run1ACACycfc : $@convention(method) (@owned A) -> @owned A // user: %3 + %3 = apply %2(%1) : $@convention(method) (@owned A) -> @owned A // user: %4 + return %3 : $A // id: %4 +} // end sil function '$s3run1ACACycfC' + +// A.init() +sil hidden @$s3run1ACACycfc : $@convention(method) (@owned A) -> @owned A { +// %0 // users: %2, %1 +bb0(%0 : $A): + debug_value %0 : $A, let, name "self", argno 1 // id: %1 + return %0 : $A // id: %2 +} // end sil function '$s3run1ACACycfc' + +// B.foo() +sil [noinline] @foo : $@convention(method) (@guaranteed B) -> Int { +// %0 // user: %1 +bb0(%0 : $B): + debug_value %0 : $B, let, name "self", argno 1 // id: %1 + %2 = integer_literal $Builtin.Int64, 1 // user: %3 + %3 = struct $Int (%2 : $Builtin.Int64) // user: %4 + return %3 : $Int // id: %4 +} // end sil function '$s3run1BC3fooSiyF' + +// Int.init(_builtinIntegerLiteral:) +sil public_external [transparent] [serialized] @$sSi22_builtinIntegerLiteralSiBI_tcfC : $@convention(method) (Builtin.IntLiteral, @thin Int.Type) -> Int { +// %0 // user: %2 +bb0(%0 : $Builtin.IntLiteral, %1 : $@thin Int.Type): + %2 = builtin "s_to_s_checked_trunc_IntLiteral_Int64"(%0 : $Builtin.IntLiteral) : $(Builtin.Int64, Builtin.Int1) // user: %3 + %3 = tuple_extract %2 : $(Builtin.Int64, Builtin.Int1), 0 // user: %4 + %4 = struct $Int (%3 : $Builtin.Int64) // user: %5 + return %4 : $Int // id: %5 +} // end sil function '$sSi22_builtinIntegerLiteralSiBI_tcfC' + +// B.deinit +sil @$s3run1BCfd : $@convention(method) (@guaranteed B) -> @owned Builtin.NativeObject { +// %0 // users: %2, %1 +bb0(%0 : $B): + debug_value %0 : $B, let, name "self", argno 1 // id: %1 + %2 = upcast %0 : $B to $A // user: %4 + // function_ref A.deinit + %3 = function_ref @$s3run1ACfd : $@convention(method) (@guaranteed A) -> @owned Builtin.NativeObject // user: %4 + %4 = apply %3(%2) : $@convention(method) (@guaranteed A) -> @owned Builtin.NativeObject // users: %5, %6 + %5 = unchecked_ref_cast %4 : $Builtin.NativeObject to $B + return %4 : $Builtin.NativeObject // id: %6 +} // end sil function '$s3run1BCfd' + +// B.__deallocating_deinit +sil @$s3run1BCfD : $@convention(method) (@owned B) -> () { +// %0 // users: %3, %1 +bb0(%0 : $B): + debug_value %0 : $B, let, name "self", argno 1 // id: %1 + // function_ref B.deinit + %2 = function_ref @$s3run1BCfd : $@convention(method) (@guaranteed B) -> @owned Builtin.NativeObject // user: %3 + %3 = apply %2(%0) : $@convention(method) (@guaranteed B) -> @owned Builtin.NativeObject // user: %4 + %4 = unchecked_ref_cast %3 : $Builtin.NativeObject to $B // user: %5 + dealloc_ref %4 : $B // id: %5 + %6 = tuple () // user: %7 + return %6 : $() // id: %7 +} // end sil function '$s3run1BCfD' + +// B.__allocating_init() +sil hidden [exact_self_class] @$s3run1BCACycfC : $@convention(method) (@thick B.Type) -> @owned B { +bb0(%0 : $@thick B.Type): + %1 = alloc_ref $B // user: %3 + // function_ref B.init() + %2 = function_ref @$s3run1BCACycfc : $@convention(method) (@owned B) -> @owned B // user: %3 + %3 = apply %2(%1) : $@convention(method) (@owned B) -> @owned B // user: %4 + return %3 : $B // id: %4 +} // end sil function '$s3run1BCACycfC' + +// B.init() +sil hidden @$s3run1BCACycfc : $@convention(method) (@owned B) -> @owned B { +// %0 // user: %2 +bb0(%0 : $B): + %1 = alloc_stack $B, let, name "self" // users: %9, %3, %2, %10, %11 + store %0 to %1 : $*B // id: %2 + %3 = load %1 : $*B // user: %4 + %4 = upcast %3 : $B to $A // user: %6 + // function_ref A.init() + %5 = function_ref @$s3run1ACACycfc : $@convention(method) (@owned A) -> @owned A // user: %6 + %6 = apply %5(%4) : $@convention(method) (@owned A) -> @owned A // user: %7 + %7 = unchecked_ref_cast %6 : $A to $B // users: %12, %9, %8 + strong_retain %7 : $B // id: %8 + store %7 to %1 : $*B // id: %9 + destroy_addr %1 : $*B // id: %10 + dealloc_stack %1 : $*B // id: %11 + return %7 : $B // id: %12 +} // end sil function '$s3run1BCACycfc' + +// C.foo() +sil @bar : $@convention(method) (@guaranteed C) -> Int { +// %0 // user: %1 +bb0(%0 : $C): + debug_value %0 : $C, let, name "self", argno 1 // id: %1 + %2 = integer_literal $Builtin.Int64, 0 // user: %3 + %3 = struct $Int (%2 : $Builtin.Int64) // user: %4 + return %3 : $Int // id: %4 +} // end sil function 'bar' + +// C.deinit +sil @$s3run1CCfd : $@convention(method) (@guaranteed C) -> @owned Builtin.NativeObject { +// %0 // users: %2, %1 +bb0(%0 : $C): + debug_value %0 : $C, let, name "self", argno 1 // id: %1 + %2 = upcast %0 : $C to $A // user: %4 + // function_ref A.deinit + %3 = function_ref @$s3run1ACfd : $@convention(method) (@guaranteed A) -> @owned Builtin.NativeObject // user: %4 + %4 = apply %3(%2) : $@convention(method) (@guaranteed A) -> @owned Builtin.NativeObject // users: %5, %6 + %5 = unchecked_ref_cast %4 : $Builtin.NativeObject to $C + return %4 : $Builtin.NativeObject // id: %6 +} // end sil function '$s3run1CCfd' + +// C.__deallocating_deinit +sil @$s3run1CCfD : $@convention(method) (@owned C) -> () { +// %0 // users: %3, %1 +bb0(%0 : $C): + debug_value %0 : $C, let, name "self", argno 1 // id: %1 + // function_ref C.deinit + %2 = function_ref @$s3run1CCfd : $@convention(method) (@guaranteed C) -> @owned Builtin.NativeObject // user: %3 + %3 = apply %2(%0) : $@convention(method) (@guaranteed C) -> @owned Builtin.NativeObject // user: %4 + %4 = unchecked_ref_cast %3 : $Builtin.NativeObject to $C // user: %5 + dealloc_ref %4 : $C // id: %5 + %6 = tuple () // user: %7 + return %6 : $() // id: %7 +} // end sil function '$s3run1CCfD' + +// C.__allocating_init() +sil hidden [exact_self_class] @$s3run1CCACycfC : $@convention(method) (@thick C.Type) -> @owned C { +bb0(%0 : $@thick C.Type): + %1 = alloc_ref $C // user: %3 + // function_ref C.init() + %2 = function_ref @$s3run1CCACycfc : $@convention(method) (@owned C) -> @owned C // user: %3 + %3 = apply %2(%1) : $@convention(method) (@owned C) -> @owned C // user: %4 + return %3 : $C // id: %4 +} // end sil function '$s3run1CCACycfC' + +// C.init() +sil hidden @$s3run1CCACycfc : $@convention(method) (@owned C) -> @owned C { +// %0 // user: %2 +bb0(%0 : $C): + %1 = alloc_stack $C, let, name "self" // users: %9, %3, %2, %10, %11 + store %0 to %1 : $*C // id: %2 + %3 = load %1 : $*C // user: %4 + %4 = upcast %3 : $C to $A // user: %6 + // function_ref A.init() + %5 = function_ref @$s3run1ACACycfc : $@convention(method) (@owned A) -> @owned A // user: %6 + %6 = apply %5(%4) : $@convention(method) (@owned A) -> @owned A // user: %7 + %7 = unchecked_ref_cast %6 : $A to $C // users: %12, %9, %8 + strong_retain %7 : $C // id: %8 + store %7 to %1 : $*C // id: %9 + destroy_addr %1 : $*C // id: %10 + dealloc_stack %1 : $*C // id: %11 + return %7 : $C // id: %12 +} // end sil function '$s3run1CCACycfc' + +// protocol witness for P.foo() in conformance B +sil shared [transparent] [serialized] [thunk] @$s3run1BCAA1PA2aDP3fooSiyFTW : $@convention(witness_method: P) (@in_guaranteed B) -> Int { +// %0 // user: %1 +bb0(%0 : $*B): + %1 = load %0 : $*B // users: %2, %3 + %2 = class_method %1 : $B, #B.foo!1 : (B) -> () -> Int, $@convention(method) (@guaranteed B) -> Int // user: %3 + %3 = apply %2(%1) : $@convention(method) (@guaranteed B) -> Int // user: %4 + return %3 : $Int // id: %4 +} // end sil function '$s3run1BCAA1PA2aDP3fooSiyFTW' + +// CHECK-LABEL: sil shared [noinline] @${{.*}}impl{{.*}} +// In the optimization pass we look for uses of alloc_stack (aka %5). +// This test makes sure that we look at the correct uses with the +// correct dominance order (store before apply before dealloc). +// This function will be passed arguments of type B and C for arguments +// %0 and %1 respectively. We want to make sure that we call B's foo method +// and not C's foo method. +sil hidden [noinline] @impl : $@convention(thin) (@guaranteed A & P, @guaranteed A & P) -> Int { +bb0(%0 : $A & P, %1 : $A & P): + %2 = open_existential_ref %0 : $A & P to $@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P + %3 = unchecked_ref_cast %1 : $A & P to $@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P + + %5 = alloc_stack $@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P + store %2 to %5 : $*@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P + + // We want to make sure that we picked up on the FIRST store and not the second one. + // class C's foo method is named "bar" whereas class B's foo method is named "foo". + // We want to make sure that we call a function named "foo" not "bar". + // CHECK: [[FN:%[0-9]+]] = function_ref @${{.*}}foo{{.*}} : $@convention(thin) () -> Int + %7 = witness_method $@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P, #P.foo!1 : (Self) -> () -> Int, %2 : $@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> Int + // CHECK: apply [[FN]] + %8 = apply %7<@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P>(%5) : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> Int + + store %3 to %5 : $*@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P + dealloc_stack %5 : $*@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P + return %8 : $Int +} // end sil function 'impl' + +// test(x:y:) +sil @test :$@convention(thin) (@guaranteed B, @guaranteed C) -> Int { +// %0 // users: %5, %4, %2 +// %1 // users: %7, %6, %3 +bb0(%0 : $B, %1 : $C): + debug_value %0 : $B, let, name "x", argno 1 // id: %2 + debug_value %1 : $C, let, name "y", argno 2 // id: %3 + strong_retain %0 : $B // id: %4 + %5 = init_existential_ref %0 : $B : $B, $A & P // users: %11, %9 + strong_retain %1 : $C // id: %6 + %7 = init_existential_ref %1 : $C : $C, $A & P // users: %10, %9 + // function_ref imp(x:y:) + %8 = function_ref @impl : $@convention(thin) (@guaranteed A & P, @guaranteed A & P) -> Int // user: %9 + %9 = apply %8(%5, %7) : $@convention(thin) (@guaranteed A & P, @guaranteed A & P) -> Int // user: %12 + strong_release %7 : $A & P // id: %10 + strong_release %5 : $A & P // id: %11 + return %9 : $Int // id: %12 +} // end sil function 'test' + +sil_vtable [serialized] A { + #A.init!allocator.1: (A.Type) -> () -> A : @$s3run1ACACycfC // A.__allocating_init() + #A.deinit!deallocator.1: @$s3run1ACfD // A.__deallocating_deinit +} + +sil_vtable [serialized] B { + #A.init!allocator.1: (A.Type) -> () -> A : @$s3run1BCACycfC [override] // B.__allocating_init() + #B.foo!1: (B) -> () -> Int : @foo // B.foo() + #B.deinit!deallocator.1: @$s3run1BCfD // B.__deallocating_deinit +} + +sil_vtable [serialized] C { + #A.init!allocator.1: (A.Type) -> () -> A : @$s3run1CCACycfC [override] // C.__allocating_init() + #C.foo!1: (C) -> () -> Int : @bar // C.foo() + #C.deinit!deallocator.1: @$s3run1CCfD // C.__deallocating_deinit +} + +sil_witness_table [serialized] B: P module run { + method #P.foo!1: (Self) -> () -> Int : @$s3run1BCAA1PA2aDP3fooSiyFTW // protocol witness for P.foo() in conformance B +} From 331a6af2a422e376a733f3800e06b9f355dfeeec Mon Sep 17 00:00:00 2001 From: zoecarver Date: Sun, 1 Dec 2019 10:51:55 -0800 Subject: [PATCH 12/36] Remove .dump in unrelated file --- lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp b/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp index b36a4883d8616..444e005e340a2 100644 --- a/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp +++ b/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp @@ -1673,7 +1673,6 @@ SILInstruction *SILCombiner::visitApplyInst(ApplyInst *AI) { // a concrete type from init_existential_addr or init_existential_ref. if (auto *WMI = dyn_cast(AI->getCallee())) { if (propagateConcreteTypeOfInitExistential(AI, WMI)) { - Builder.getFunction().dump(); return nullptr; } } From d032de63a7db6d558d85914b5e6a2b2fd0d290d6 Mon Sep 17 00:00:00 2001 From: zoecarver Date: Sun, 1 Dec 2019 10:55:59 -0800 Subject: [PATCH 13/36] Move relevant part of sil test to the top of the file --- ...alize_protocol_composition_two_applies.sil | 56 +++++++++---------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/test/SILOptimizer/devirtualize_protocol_composition_two_applies.sil b/test/SILOptimizer/devirtualize_protocol_composition_two_applies.sil index f6a579c2c1c83..e3639ed2f1224 100644 --- a/test/SILOptimizer/devirtualize_protocol_composition_two_applies.sil +++ b/test/SILOptimizer/devirtualize_protocol_composition_two_applies.sil @@ -31,6 +31,34 @@ func imp(x: A & P, y: A & P) -> Int public func test(x: B, y: C) -> Int +// CHECK-LABEL: sil shared [noinline] @${{.*}}impl{{.*}} +// In the optimization pass we look for uses of alloc_stack (aka %5). +// This test makes sure that we look at the correct uses with the +// correct dominance order (store before apply before dealloc). +// This function will be passed arguments of type B and C for arguments +// %0 and %1 respectively. We want to make sure that we call B's foo method +// and not C's foo method. +sil hidden [noinline] @impl : $@convention(thin) (@guaranteed A & P, @guaranteed A & P) -> Int { +bb0(%0 : $A & P, %1 : $A & P): + %2 = open_existential_ref %0 : $A & P to $@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P + %3 = unchecked_ref_cast %1 : $A & P to $@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P + + %5 = alloc_stack $@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P + store %2 to %5 : $*@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P + + // We want to make sure that we picked up on the FIRST store and not the second one. + // class C's foo method is named "bar" whereas class B's foo method is named "foo". + // We want to make sure that we call a function named "foo" not "bar". + // CHECK: [[FN:%[0-9]+]] = function_ref @${{.*}}foo{{.*}} : $@convention(thin) () -> Int + %7 = witness_method $@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P, #P.foo!1 : (Self) -> () -> Int, %2 : $@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> Int + // CHECK: apply [[FN]] + %8 = apply %7<@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P>(%5) : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> Int + + store %3 to %5 : $*@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P + dealloc_stack %5 : $*@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P + return %8 : $Int +} // end sil function 'impl' + // main sil @main : $@convention(c) (Int32, UnsafeMutablePointer>>) -> Int32 { bb0(%0 : $Int32, %1 : $UnsafeMutablePointer>>): @@ -232,34 +260,6 @@ bb0(%0 : $*B): return %3 : $Int // id: %4 } // end sil function '$s3run1BCAA1PA2aDP3fooSiyFTW' -// CHECK-LABEL: sil shared [noinline] @${{.*}}impl{{.*}} -// In the optimization pass we look for uses of alloc_stack (aka %5). -// This test makes sure that we look at the correct uses with the -// correct dominance order (store before apply before dealloc). -// This function will be passed arguments of type B and C for arguments -// %0 and %1 respectively. We want to make sure that we call B's foo method -// and not C's foo method. -sil hidden [noinline] @impl : $@convention(thin) (@guaranteed A & P, @guaranteed A & P) -> Int { -bb0(%0 : $A & P, %1 : $A & P): - %2 = open_existential_ref %0 : $A & P to $@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P - %3 = unchecked_ref_cast %1 : $A & P to $@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P - - %5 = alloc_stack $@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P - store %2 to %5 : $*@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P - - // We want to make sure that we picked up on the FIRST store and not the second one. - // class C's foo method is named "bar" whereas class B's foo method is named "foo". - // We want to make sure that we call a function named "foo" not "bar". - // CHECK: [[FN:%[0-9]+]] = function_ref @${{.*}}foo{{.*}} : $@convention(thin) () -> Int - %7 = witness_method $@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P, #P.foo!1 : (Self) -> () -> Int, %2 : $@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> Int - // CHECK: apply [[FN]] - %8 = apply %7<@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P>(%5) : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> Int - - store %3 to %5 : $*@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P - dealloc_stack %5 : $*@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P - return %8 : $Int -} // end sil function 'impl' - // test(x:y:) sil @test :$@convention(thin) (@guaranteed B, @guaranteed C) -> Int { // %0 // users: %5, %4, %2 From 644689dc12082942800327074c76d97851b07af8 Mon Sep 17 00:00:00 2001 From: zoecarver Date: Sun, 1 Dec 2019 13:57:12 -0800 Subject: [PATCH 14/36] Rename test --- ...plies.sil => devirtualize_protocol_composition_two_stores.sil} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename test/SILOptimizer/{devirtualize_protocol_composition_two_applies.sil => devirtualize_protocol_composition_two_stores.sil} (100%) diff --git a/test/SILOptimizer/devirtualize_protocol_composition_two_applies.sil b/test/SILOptimizer/devirtualize_protocol_composition_two_stores.sil similarity index 100% rename from test/SILOptimizer/devirtualize_protocol_composition_two_applies.sil rename to test/SILOptimizer/devirtualize_protocol_composition_two_stores.sil From 758f1f3208ca897ec2a8f53b93c8e323feb0b855 Mon Sep 17 00:00:00 2001 From: zoecarver Date: Sun, 22 Dec 2019 13:54:10 -0800 Subject: [PATCH 15/36] Update getAddressOfStackInit and getStackInitInst to support store instructions --- lib/SILOptimizer/Utils/Existential.cpp | 52 ++++++++++---------------- 1 file changed, 19 insertions(+), 33 deletions(-) diff --git a/lib/SILOptimizer/Utils/Existential.cpp b/lib/SILOptimizer/Utils/Existential.cpp index d270189f2da57..3c5881e39c008 100644 --- a/lib/SILOptimizer/Utils/Existential.cpp +++ b/lib/SILOptimizer/Utils/Existential.cpp @@ -111,6 +111,19 @@ static SILInstruction *getStackInitInst(SILValue allocStackAddr, } continue; } + if (auto *store = dyn_cast(User)) { + if (store->getDest() == allocStackAddr) { + if (SingleWrite) + return nullptr; + SingleWrite = store; + // When we support OSSA here, we need to insert a new copy of the value + // before `store` (and make sure that the copy is destroyed when + // replacing the apply operand). + assert(store->getOwnershipQualifier() == + StoreOwnershipQualifier::Unqualified); + } + continue; + } if (isa(User)) { if (SingleWrite) return nullptr; @@ -144,6 +157,9 @@ static SILInstruction *getStackInitInst(SILValue allocStackAddr, if (BB != allocStackAddr->getParentBlock() && BB != ASIUser->getParent()) return nullptr; + if(auto *store = dyn_cast(SingleWrite)) + return store; + if (auto *IE = dyn_cast(SingleWrite)) return IE; @@ -194,6 +210,9 @@ static SILValue getAddressOfStackInit(SILValue allocStackAddr, if (auto *CAI = dyn_cast(initI)) return CAI->getSrc(); + + if (auto *store = dyn_cast(initI)) + return store->getSrc(); return SILValue(); } @@ -213,39 +232,6 @@ OpenedArchetypeInfo::OpenedArchetypeInfo(Operand &use) { getAddressOfStackInit(instance, user, isOpenedValueCopied)) { openedVal = stackInitVal; } - - // Handle: - // %4 = open_existential_ref - // %5 = alloc_stack - // store %4 to %5 - // apply (%5) - // It's important that the only uses of %5 (instance) are in - // a store and an apply. - StoreInst *store = nullptr; - ApplyInst *apply = nullptr; - DeallocStackInst *dealloc = nullptr; - bool shouldOptimize = true; - - for (auto *use : instance->getUses()) { - if (auto *foundStore = dyn_cast(use->getUser())) { - store = foundStore; - } else if (auto *foundApply = dyn_cast(use->getUser())) { - apply = foundApply; - } else if (auto *foundDealloc = dyn_cast(use->getUser())) { - dealloc = foundDealloc; - } else { - // TODO: this may be too harsh - shouldOptimize = false; - } - } - - if (shouldOptimize && store && apply && dealloc) { - DominanceInfo domInfo(store->getFunction()); - bool correctOrder = domInfo.dominates(store, apply) && - domInfo.dominates(apply, dealloc); - if (correctOrder) - openedVal = store->getSrc(); - } } if (auto *Open = dyn_cast(openedVal)) { OpenedArchetype = Open->getType().castTo(); From f4e3ca9999a422b2c2d907778e6cb56c4ee63833 Mon Sep 17 00:00:00 2001 From: zoecarver Date: Sun, 22 Dec 2019 13:58:01 -0800 Subject: [PATCH 16/36] Formatting --- lib/SILOptimizer/Utils/Existential.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/SILOptimizer/Utils/Existential.cpp b/lib/SILOptimizer/Utils/Existential.cpp index 3c5881e39c008..8053e32082017 100644 --- a/lib/SILOptimizer/Utils/Existential.cpp +++ b/lib/SILOptimizer/Utils/Existential.cpp @@ -157,9 +157,9 @@ static SILInstruction *getStackInitInst(SILValue allocStackAddr, if (BB != allocStackAddr->getParentBlock() && BB != ASIUser->getParent()) return nullptr; - if(auto *store = dyn_cast(SingleWrite)) + if (auto *store = dyn_cast(SingleWrite)) return store; - + if (auto *IE = dyn_cast(SingleWrite)) return IE; @@ -210,7 +210,7 @@ static SILValue getAddressOfStackInit(SILValue allocStackAddr, if (auto *CAI = dyn_cast(initI)) return CAI->getSrc(); - + if (auto *store = dyn_cast(initI)) return store->getSrc(); From a52fe57256f275e5897ae2d9af3d456f2c51e218 Mon Sep 17 00:00:00 2001 From: zoecarver Date: Sun, 5 Jan 2020 11:51:39 -0800 Subject: [PATCH 17/36] Cleanup tests --- ...ualize_protocol_composition_two_stores.sil | 217 ++---------------- 1 file changed, 15 insertions(+), 202 deletions(-) diff --git a/test/SILOptimizer/devirtualize_protocol_composition_two_stores.sil b/test/SILOptimizer/devirtualize_protocol_composition_two_stores.sil index e3639ed2f1224..ac7115966f202 100644 --- a/test/SILOptimizer/devirtualize_protocol_composition_two_stores.sil +++ b/test/SILOptimizer/devirtualize_protocol_composition_two_stores.sil @@ -31,7 +31,7 @@ func imp(x: A & P, y: A & P) -> Int public func test(x: B, y: C) -> Int -// CHECK-LABEL: sil shared [noinline] @${{.*}}impl{{.*}} +// CHECK-LABEL: sil shared [noinline] @{{.*}}impl{{.*}} // In the optimization pass we look for uses of alloc_stack (aka %5). // This test makes sure that we look at the correct uses with the // correct dominance order (store before apply before dealloc). @@ -59,54 +59,15 @@ bb0(%0 : $A & P, %1 : $A & P): return %8 : $Int } // end sil function 'impl' -// main -sil @main : $@convention(c) (Int32, UnsafeMutablePointer>>) -> Int32 { -bb0(%0 : $Int32, %1 : $UnsafeMutablePointer>>): - %2 = integer_literal $Builtin.Int32, 0 // user: %3 - %3 = struct $Int32 (%2 : $Builtin.Int32) // user: %4 - return %3 : $Int32 // id: %4 -} // end sil function 'main' - -// A.deinit -sil @$s3run1ACfd : $@convention(method) (@guaranteed A) -> @owned Builtin.NativeObject { -// %0 // users: %2, %1 -bb0(%0 : $A): - debug_value %0 : $A, let, name "self", argno 1 // id: %1 - %2 = unchecked_ref_cast %0 : $A to $Builtin.NativeObject // user: %3 - return %2 : $Builtin.NativeObject // id: %3 -} // end sil function '$s3run1ACfd' - -// A.__deallocating_deinit -sil @$s3run1ACfD : $@convention(method) (@owned A) -> () { -// %0 // users: %3, %1 -bb0(%0 : $A): - debug_value %0 : $A, let, name "self", argno 1 // id: %1 - // function_ref A.deinit - %2 = function_ref @$s3run1ACfd : $@convention(method) (@guaranteed A) -> @owned Builtin.NativeObject // user: %3 - %3 = apply %2(%0) : $@convention(method) (@guaranteed A) -> @owned Builtin.NativeObject // user: %4 - %4 = unchecked_ref_cast %3 : $Builtin.NativeObject to $A // user: %5 - dealloc_ref %4 : $A // id: %5 - %6 = tuple () // user: %7 - return %6 : $() // id: %7 -} // end sil function '$s3run1ACfD' - -// A.__allocating_init() -sil hidden [exact_self_class] @$s3run1ACACycfC : $@convention(method) (@thick A.Type) -> @owned A { -bb0(%0 : $@thick A.Type): - %1 = alloc_ref $A // user: %3 - // function_ref A.init() - %2 = function_ref @$s3run1ACACycfc : $@convention(method) (@owned A) -> @owned A // user: %3 - %3 = apply %2(%1) : $@convention(method) (@owned A) -> @owned A // user: %4 - return %3 : $A // id: %4 -} // end sil function '$s3run1ACACycfC' - -// A.init() -sil hidden @$s3run1ACACycfc : $@convention(method) (@owned A) -> @owned A { -// %0 // users: %2, %1 -bb0(%0 : $A): - debug_value %0 : $A, let, name "self", argno 1 // id: %1 - return %0 : $A // id: %2 -} // end sil function '$s3run1ACACycfc' +// protocol witness for P.foo() in conformance B +sil shared [transparent] [serialized] [thunk] @Pfoo : $@convention(witness_method: P) (@in_guaranteed B) -> Int { +// %0 // user: %1 +bb0(%0 : $*B): + %1 = load %0 : $*B // users: %2, %3 + %2 = class_method %1 : $B, #B.foo!1 : (B) -> () -> Int, $@convention(method) (@guaranteed B) -> Int // user: %3 + %3 = apply %2(%1) : $@convention(method) (@guaranteed B) -> Int // user: %4 + return %3 : $Int // id: %4 +} // end sil function 'Pfoo' // B.foo() sil [noinline] @foo : $@convention(method) (@guaranteed B) -> Int { @@ -116,149 +77,7 @@ bb0(%0 : $B): %2 = integer_literal $Builtin.Int64, 1 // user: %3 %3 = struct $Int (%2 : $Builtin.Int64) // user: %4 return %3 : $Int // id: %4 -} // end sil function '$s3run1BC3fooSiyF' - -// Int.init(_builtinIntegerLiteral:) -sil public_external [transparent] [serialized] @$sSi22_builtinIntegerLiteralSiBI_tcfC : $@convention(method) (Builtin.IntLiteral, @thin Int.Type) -> Int { -// %0 // user: %2 -bb0(%0 : $Builtin.IntLiteral, %1 : $@thin Int.Type): - %2 = builtin "s_to_s_checked_trunc_IntLiteral_Int64"(%0 : $Builtin.IntLiteral) : $(Builtin.Int64, Builtin.Int1) // user: %3 - %3 = tuple_extract %2 : $(Builtin.Int64, Builtin.Int1), 0 // user: %4 - %4 = struct $Int (%3 : $Builtin.Int64) // user: %5 - return %4 : $Int // id: %5 -} // end sil function '$sSi22_builtinIntegerLiteralSiBI_tcfC' - -// B.deinit -sil @$s3run1BCfd : $@convention(method) (@guaranteed B) -> @owned Builtin.NativeObject { -// %0 // users: %2, %1 -bb0(%0 : $B): - debug_value %0 : $B, let, name "self", argno 1 // id: %1 - %2 = upcast %0 : $B to $A // user: %4 - // function_ref A.deinit - %3 = function_ref @$s3run1ACfd : $@convention(method) (@guaranteed A) -> @owned Builtin.NativeObject // user: %4 - %4 = apply %3(%2) : $@convention(method) (@guaranteed A) -> @owned Builtin.NativeObject // users: %5, %6 - %5 = unchecked_ref_cast %4 : $Builtin.NativeObject to $B - return %4 : $Builtin.NativeObject // id: %6 -} // end sil function '$s3run1BCfd' - -// B.__deallocating_deinit -sil @$s3run1BCfD : $@convention(method) (@owned B) -> () { -// %0 // users: %3, %1 -bb0(%0 : $B): - debug_value %0 : $B, let, name "self", argno 1 // id: %1 - // function_ref B.deinit - %2 = function_ref @$s3run1BCfd : $@convention(method) (@guaranteed B) -> @owned Builtin.NativeObject // user: %3 - %3 = apply %2(%0) : $@convention(method) (@guaranteed B) -> @owned Builtin.NativeObject // user: %4 - %4 = unchecked_ref_cast %3 : $Builtin.NativeObject to $B // user: %5 - dealloc_ref %4 : $B // id: %5 - %6 = tuple () // user: %7 - return %6 : $() // id: %7 -} // end sil function '$s3run1BCfD' - -// B.__allocating_init() -sil hidden [exact_self_class] @$s3run1BCACycfC : $@convention(method) (@thick B.Type) -> @owned B { -bb0(%0 : $@thick B.Type): - %1 = alloc_ref $B // user: %3 - // function_ref B.init() - %2 = function_ref @$s3run1BCACycfc : $@convention(method) (@owned B) -> @owned B // user: %3 - %3 = apply %2(%1) : $@convention(method) (@owned B) -> @owned B // user: %4 - return %3 : $B // id: %4 -} // end sil function '$s3run1BCACycfC' - -// B.init() -sil hidden @$s3run1BCACycfc : $@convention(method) (@owned B) -> @owned B { -// %0 // user: %2 -bb0(%0 : $B): - %1 = alloc_stack $B, let, name "self" // users: %9, %3, %2, %10, %11 - store %0 to %1 : $*B // id: %2 - %3 = load %1 : $*B // user: %4 - %4 = upcast %3 : $B to $A // user: %6 - // function_ref A.init() - %5 = function_ref @$s3run1ACACycfc : $@convention(method) (@owned A) -> @owned A // user: %6 - %6 = apply %5(%4) : $@convention(method) (@owned A) -> @owned A // user: %7 - %7 = unchecked_ref_cast %6 : $A to $B // users: %12, %9, %8 - strong_retain %7 : $B // id: %8 - store %7 to %1 : $*B // id: %9 - destroy_addr %1 : $*B // id: %10 - dealloc_stack %1 : $*B // id: %11 - return %7 : $B // id: %12 -} // end sil function '$s3run1BCACycfc' - -// C.foo() -sil @bar : $@convention(method) (@guaranteed C) -> Int { -// %0 // user: %1 -bb0(%0 : $C): - debug_value %0 : $C, let, name "self", argno 1 // id: %1 - %2 = integer_literal $Builtin.Int64, 0 // user: %3 - %3 = struct $Int (%2 : $Builtin.Int64) // user: %4 - return %3 : $Int // id: %4 -} // end sil function 'bar' - -// C.deinit -sil @$s3run1CCfd : $@convention(method) (@guaranteed C) -> @owned Builtin.NativeObject { -// %0 // users: %2, %1 -bb0(%0 : $C): - debug_value %0 : $C, let, name "self", argno 1 // id: %1 - %2 = upcast %0 : $C to $A // user: %4 - // function_ref A.deinit - %3 = function_ref @$s3run1ACfd : $@convention(method) (@guaranteed A) -> @owned Builtin.NativeObject // user: %4 - %4 = apply %3(%2) : $@convention(method) (@guaranteed A) -> @owned Builtin.NativeObject // users: %5, %6 - %5 = unchecked_ref_cast %4 : $Builtin.NativeObject to $C - return %4 : $Builtin.NativeObject // id: %6 -} // end sil function '$s3run1CCfd' - -// C.__deallocating_deinit -sil @$s3run1CCfD : $@convention(method) (@owned C) -> () { -// %0 // users: %3, %1 -bb0(%0 : $C): - debug_value %0 : $C, let, name "self", argno 1 // id: %1 - // function_ref C.deinit - %2 = function_ref @$s3run1CCfd : $@convention(method) (@guaranteed C) -> @owned Builtin.NativeObject // user: %3 - %3 = apply %2(%0) : $@convention(method) (@guaranteed C) -> @owned Builtin.NativeObject // user: %4 - %4 = unchecked_ref_cast %3 : $Builtin.NativeObject to $C // user: %5 - dealloc_ref %4 : $C // id: %5 - %6 = tuple () // user: %7 - return %6 : $() // id: %7 -} // end sil function '$s3run1CCfD' - -// C.__allocating_init() -sil hidden [exact_self_class] @$s3run1CCACycfC : $@convention(method) (@thick C.Type) -> @owned C { -bb0(%0 : $@thick C.Type): - %1 = alloc_ref $C // user: %3 - // function_ref C.init() - %2 = function_ref @$s3run1CCACycfc : $@convention(method) (@owned C) -> @owned C // user: %3 - %3 = apply %2(%1) : $@convention(method) (@owned C) -> @owned C // user: %4 - return %3 : $C // id: %4 -} // end sil function '$s3run1CCACycfC' - -// C.init() -sil hidden @$s3run1CCACycfc : $@convention(method) (@owned C) -> @owned C { -// %0 // user: %2 -bb0(%0 : $C): - %1 = alloc_stack $C, let, name "self" // users: %9, %3, %2, %10, %11 - store %0 to %1 : $*C // id: %2 - %3 = load %1 : $*C // user: %4 - %4 = upcast %3 : $C to $A // user: %6 - // function_ref A.init() - %5 = function_ref @$s3run1ACACycfc : $@convention(method) (@owned A) -> @owned A // user: %6 - %6 = apply %5(%4) : $@convention(method) (@owned A) -> @owned A // user: %7 - %7 = unchecked_ref_cast %6 : $A to $C // users: %12, %9, %8 - strong_retain %7 : $C // id: %8 - store %7 to %1 : $*C // id: %9 - destroy_addr %1 : $*C // id: %10 - dealloc_stack %1 : $*C // id: %11 - return %7 : $C // id: %12 -} // end sil function '$s3run1CCACycfc' - -// protocol witness for P.foo() in conformance B -sil shared [transparent] [serialized] [thunk] @$s3run1BCAA1PA2aDP3fooSiyFTW : $@convention(witness_method: P) (@in_guaranteed B) -> Int { -// %0 // user: %1 -bb0(%0 : $*B): - %1 = load %0 : $*B // users: %2, %3 - %2 = class_method %1 : $B, #B.foo!1 : (B) -> () -> Int, $@convention(method) (@guaranteed B) -> Int // user: %3 - %3 = apply %2(%1) : $@convention(method) (@guaranteed B) -> Int // user: %4 - return %3 : $Int // id: %4 -} // end sil function '$s3run1BCAA1PA2aDP3fooSiyFTW' +} // end sil function 'foo' // test(x:y:) sil @test :$@convention(thin) (@guaranteed B, @guaranteed C) -> Int { @@ -279,23 +98,17 @@ bb0(%0 : $B, %1 : $C): return %9 : $Int // id: %12 } // end sil function 'test' -sil_vtable [serialized] A { - #A.init!allocator.1: (A.Type) -> () -> A : @$s3run1ACACycfC // A.__allocating_init() - #A.deinit!deallocator.1: @$s3run1ACfD // A.__deallocating_deinit -} +// C.foo() +sil @bar : $@convention(method) (@guaranteed C) -> Int sil_vtable [serialized] B { - #A.init!allocator.1: (A.Type) -> () -> A : @$s3run1BCACycfC [override] // B.__allocating_init() #B.foo!1: (B) -> () -> Int : @foo // B.foo() - #B.deinit!deallocator.1: @$s3run1BCfD // B.__deallocating_deinit } sil_vtable [serialized] C { - #A.init!allocator.1: (A.Type) -> () -> A : @$s3run1CCACycfC [override] // C.__allocating_init() - #C.foo!1: (C) -> () -> Int : @bar // C.foo() - #C.deinit!deallocator.1: @$s3run1CCfD // C.__deallocating_deinit + #C.foo!1: (C) -> () -> Int : @bar } sil_witness_table [serialized] B: P module run { - method #P.foo!1: (Self) -> () -> Int : @$s3run1BCAA1PA2aDP3fooSiyFTW // protocol witness for P.foo() in conformance B + method #P.foo!1: (Self) -> () -> Int : @Pfoo // protocol witness for P.foo() in conformance B } From 33987c5380e5a97efb3c93ab7f4d8751c1d73101 Mon Sep 17 00:00:00 2001 From: zoecarver Date: Wed, 22 Jan 2020 21:33:41 -0800 Subject: [PATCH 18/36] Inline/remove getAddressOfStackInit and update tests to make them more clear --- lib/SILOptimizer/Utils/Existential.cpp | 36 +++-------- ...ualize_protocol_composition_two_stores.sil | 63 ++++++++++--------- 2 files changed, 45 insertions(+), 54 deletions(-) diff --git a/lib/SILOptimizer/Utils/Existential.cpp b/lib/SILOptimizer/Utils/Existential.cpp index 8053e32082017..00edb27c1fc2c 100644 --- a/lib/SILOptimizer/Utils/Existential.cpp +++ b/lib/SILOptimizer/Utils/Existential.cpp @@ -69,7 +69,7 @@ findInitExistentialFromGlobalAddr(GlobalAddrInst *GAI, SILInstruction *Insn) { /// Returns the instruction that initializes the given stack address. This is /// currently either a init_existential_addr, unconditional_checked_cast_addr, -/// or copy_addr (if the instruction initializing the source of the copy cannot +/// store, or copy_addr (if the instruction initializing the source of the copy cannot /// be determined). Returns nullptr if the initializer does not dominate the /// alloc_stack user \p ASIUser. If the value is copied from another stack /// location, \p isCopied is set to true. @@ -196,27 +196,6 @@ static SILInstruction *getStackInitInst(SILValue allocStackAddr, return CAI; } -/// Return the address of the value used to initialize the given stack location. -/// If the value originates from init_existential_addr, then it will be a -/// different type than \p allocStackAddr. -static SILValue getAddressOfStackInit(SILValue allocStackAddr, - SILInstruction *ASIUser, bool &isCopied) { - SILInstruction *initI = getStackInitInst(allocStackAddr, ASIUser, isCopied); - if (!initI) - return SILValue(); - - if (auto *IEA = dyn_cast(initI)) - return IEA; - - if (auto *CAI = dyn_cast(initI)) - return CAI->getSrc(); - - if (auto *store = dyn_cast(initI)) - return store->getSrc(); - - return SILValue(); -} - /// Check if the given operand originates from a recognized OpenArchetype /// instruction. If so, return the Opened, otherwise return nullptr. OpenedArchetypeInfo::OpenedArchetypeInfo(Operand &use) { @@ -226,11 +205,16 @@ OpenedArchetypeInfo::OpenedArchetypeInfo(Operand &use) { // Handle: // %opened = open_existential_addr // %instance = alloc $opened - // copy_addr %opened to %stack + // %opened to %stack // %instance - if (auto stackInitVal = - getAddressOfStackInit(instance, user, isOpenedValueCopied)) { - openedVal = stackInitVal; + if (auto *initI = getStackInitInst(instance, user, isOpenedValueCopied)) { + if (auto *IEA = dyn_cast(initI)) + // TODO: this case doesn't need to exist beacuse openedVal will never be used + openedVal = IEA; + if (auto *CAI = dyn_cast(initI)) + openedVal = CAI->getSrc(); + if (auto *store = dyn_cast(initI)) + openedVal = store->getSrc(); } } if (auto *Open = dyn_cast(openedVal)) { diff --git a/test/SILOptimizer/devirtualize_protocol_composition_two_stores.sil b/test/SILOptimizer/devirtualize_protocol_composition_two_stores.sil index ac7115966f202..0dcc2f1e2d624 100644 --- a/test/SILOptimizer/devirtualize_protocol_composition_two_stores.sil +++ b/test/SILOptimizer/devirtualize_protocol_composition_two_stores.sil @@ -31,34 +31,6 @@ func imp(x: A & P, y: A & P) -> Int public func test(x: B, y: C) -> Int -// CHECK-LABEL: sil shared [noinline] @{{.*}}impl{{.*}} -// In the optimization pass we look for uses of alloc_stack (aka %5). -// This test makes sure that we look at the correct uses with the -// correct dominance order (store before apply before dealloc). -// This function will be passed arguments of type B and C for arguments -// %0 and %1 respectively. We want to make sure that we call B's foo method -// and not C's foo method. -sil hidden [noinline] @impl : $@convention(thin) (@guaranteed A & P, @guaranteed A & P) -> Int { -bb0(%0 : $A & P, %1 : $A & P): - %2 = open_existential_ref %0 : $A & P to $@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P - %3 = unchecked_ref_cast %1 : $A & P to $@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P - - %5 = alloc_stack $@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P - store %2 to %5 : $*@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P - - // We want to make sure that we picked up on the FIRST store and not the second one. - // class C's foo method is named "bar" whereas class B's foo method is named "foo". - // We want to make sure that we call a function named "foo" not "bar". - // CHECK: [[FN:%[0-9]+]] = function_ref @${{.*}}foo{{.*}} : $@convention(thin) () -> Int - %7 = witness_method $@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P, #P.foo!1 : (Self) -> () -> Int, %2 : $@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> Int - // CHECK: apply [[FN]] - %8 = apply %7<@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P>(%5) : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> Int - - store %3 to %5 : $*@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P - dealloc_stack %5 : $*@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P - return %8 : $Int -} // end sil function 'impl' - // protocol witness for P.foo() in conformance B sil shared [transparent] [serialized] [thunk] @Pfoo : $@convention(witness_method: P) (@in_guaranteed B) -> Int { // %0 // user: %1 @@ -79,6 +51,10 @@ bb0(%0 : $B): return %3 : $Int // id: %4 } // end sil function 'foo' + +// This function calls @impl. The optimizer generates a specialization of impl +// based on the arguments passed to it here. Even though this function isn't +// directly checked, it is essentail for the optimization. // test(x:y:) sil @test :$@convention(thin) (@guaranteed B, @guaranteed C) -> Int { // %0 // users: %5, %4, %2 @@ -98,6 +74,37 @@ bb0(%0 : $B, %1 : $C): return %9 : $Int // id: %12 } // end sil function 'test' +// We're looking of an optimized spcialization of @impl, not @impl itself. +// CHECK-NOT: @impl +// CHECK-LABEL: sil shared [noinline] @{{.*}}impl{{.*}} +// In the optimization pass we look for uses of alloc_stack (aka %5). +// This test makes sure that we look at the correct uses with the +// correct dominance order (store before apply before dealloc). +// This function will be passed arguments of type B and C for arguments +// %0 and %1 respectively. We want to make sure that we call B's foo method +// and not C's foo method. +sil hidden [noinline] @impl : $@convention(thin) (@guaranteed A & P, @guaranteed A & P) -> Int { +bb0(%0 : $A & P, %1 : $A & P): + %2 = open_existential_ref %0 : $A & P to $@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P + %3 = unchecked_ref_cast %1 : $A & P to $@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P + + %5 = alloc_stack $@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P + store %2 to %5 : $*@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P + + // We want to make sure that we picked up on the FIRST store and not the second one. + // class C's foo method is named "bar" whereas class B's foo method is named "foo". + // We want to make sure that we call a function named "foo" not "bar". + // CHECK: [[FN:%[0-9]+]] = function_ref @${{.*}}foo{{.*}} : $@convention(thin) () -> Int + %7 = witness_method $@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P, #P.foo!1 : (Self) -> () -> Int, %2 : $@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> Int + // CHECK: apply [[FN]] + %8 = apply %7<@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P>(%5) : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> Int + + store %3 to %5 : $*@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P + dealloc_stack %5 : $*@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P + return %8 : $Int +// CHECK-LABEL: end sil function {{.*}}impl{{.*}} +} // end sil function 'impl' + // C.foo() sil @bar : $@convention(method) (@guaranteed C) -> Int From 0992e482d4417a50a76d6956a2836285d07ac480 Mon Sep 17 00:00:00 2001 From: zoecarver Date: Fri, 24 Jan 2020 11:07:31 -0800 Subject: [PATCH 19/36] Abort if not address type --- lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp | 7 ++++++- lib/SILOptimizer/Utils/Existential.cpp | 4 ++-- .../existential_specializer_indirect_class.sil | 2 +- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp b/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp index 90607e637f7d9..8a602b68435b4 100644 --- a/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp +++ b/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp @@ -814,6 +814,10 @@ struct ConcreteArgumentCopy { if (!paramInfo.isFormalIndirect()) return None; + // TODO: Support non-address values (e.g. from stores). + if (!CEI.ConcreteValue->getType().isAddress()) + return None; + SILBuilderWithScope B(apply.getInstruction(), BuilderCtx); auto loc = apply.getLoc(); auto *ASI = B.createAllocStack(loc, CEI.ConcreteValue->getType()); @@ -882,12 +886,13 @@ SILInstruction *SILCombiner::createApplyWithConcreteType( NewArgs.push_back(Apply.getArgument(ArgIdx)); continue; } - UpdatedArgs = true; + // Ensure that we have a concrete value to propagate. assert(CEI.ConcreteValue); auto argSub = ConcreteArgumentCopy::generate(CEI, Apply, ArgIdx, BuilderCtx); if (argSub) { + UpdatedArgs = true; concreteArgCopies.push_back(*argSub); NewArgs.push_back(argSub->tempArgCopy->getDest()); } else diff --git a/lib/SILOptimizer/Utils/Existential.cpp b/lib/SILOptimizer/Utils/Existential.cpp index 00edb27c1fc2c..28fd2baec1b11 100644 --- a/lib/SILOptimizer/Utils/Existential.cpp +++ b/lib/SILOptimizer/Utils/Existential.cpp @@ -213,8 +213,8 @@ OpenedArchetypeInfo::OpenedArchetypeInfo(Operand &use) { openedVal = IEA; if (auto *CAI = dyn_cast(initI)) openedVal = CAI->getSrc(); - if (auto *store = dyn_cast(initI)) - openedVal = store->getSrc(); +// if (auto *store = dyn_cast(initI)) +// openedVal = store->getSrc(); } } if (auto *Open = dyn_cast(openedVal)) { diff --git a/test/SILOptimizer/existential_specializer_indirect_class.sil b/test/SILOptimizer/existential_specializer_indirect_class.sil index 384931e2f09a4..2aa96abf1fbbd 100644 --- a/test/SILOptimizer/existential_specializer_indirect_class.sil +++ b/test/SILOptimizer/existential_specializer_indirect_class.sil @@ -40,7 +40,7 @@ bb0(%0 : $*ClassProtocol): // CHECK-NEXT: %7 = witness_method $C, #ClassProtocol.method %f = witness_method $@opened("ABCDEF01-ABCD-ABCD-ABCD-ABCDEFABCDEF") ClassProtocol, #ClassProtocol.method!1 : (Self) -> () -> (), %2 : $@opened("ABCDEF01-ABCD-ABCD-ABCD-ABCDEFABCDEF") ClassProtocol : $@convention(witness_method : ClassProtocol) (@guaranteed Self) -> () // CHECK-NEXT: %8 = unchecked_ref_cast %6 - // CHECK-NEXT: %9 = apply %7(%8) + // CHECK-NEXT: %9 = apply %7(%6) apply %f<@opened("ABCDEF01-ABCD-ABCD-ABCD-ABCDEFABCDEF") ClassProtocol>(%2) : $@convention(witness_method : ClassProtocol) (@guaranteed Self) -> () // CHECK-NEXT: dealloc_stack %3 // CHECK-NEXT: return undef From ae64719824621e2a70f1c574ba98f57ce4006e93 Mon Sep 17 00:00:00 2001 From: zoecarver Date: Fri, 24 Jan 2020 12:22:13 -0800 Subject: [PATCH 20/36] Support store instructions in createApplyWithConcreteType Supporting stores in more places means that more code can be _completely_ devirtualized which also means other optimizations (e.g. inlining) can also happen. Also, updates tests. --- .../SILCombiner/SILCombinerApplyVisitors.cpp | 48 ++++++------ lib/SILOptimizer/Utils/Existential.cpp | 4 +- ...existential_specializer_indirect_class.sil | 74 +++++++++---------- 3 files changed, 63 insertions(+), 63 deletions(-) diff --git a/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp b/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp index 8a602b68435b4..6c0b9528358b1 100644 --- a/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp +++ b/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp @@ -775,10 +775,10 @@ bool SILCombiner::canReplaceArg(FullApplySite Apply, /// (@in or @owned convention). struct ConcreteArgumentCopy { SILValue origArg; - CopyAddrInst *tempArgCopy; + AllocStackInst *tempArg; - ConcreteArgumentCopy(SILValue origArg, CopyAddrInst *tempArgCopy) - : origArg(origArg), tempArgCopy(tempArgCopy) { + ConcreteArgumentCopy(SILValue origArg, AllocStackInst *tempArg) + : origArg(origArg), tempArg(tempArg) { assert(origArg->getType().isAddress()); } @@ -791,8 +791,8 @@ struct ConcreteArgumentCopy { assert(!paramInfo.isIndirectMutating() && "A mutated opened existential value can't be replaced"); - if (!paramInfo.isConsumed()) - return None; +// if (!paramInfo.isConsumed()) +// return None; SILValue origArg = apply.getArgument(argIdx); // FIXME_opaque: With SIL opaque values, a formally indirect argument may be @@ -814,16 +814,18 @@ struct ConcreteArgumentCopy { if (!paramInfo.isFormalIndirect()) return None; - // TODO: Support non-address values (e.g. from stores). - if (!CEI.ConcreteValue->getType().isAddress()) - return None; - SILBuilderWithScope B(apply.getInstruction(), BuilderCtx); auto loc = apply.getLoc(); auto *ASI = B.createAllocStack(loc, CEI.ConcreteValue->getType()); - auto *CAI = B.createCopyAddr(loc, CEI.ConcreteValue, ASI, IsNotTake, - IsInitialization_t::IsInitialization); - return ConcreteArgumentCopy(origArg, CAI); + + if (CEI.ConcreteValue->getType().isAddress()) { + B.createCopyAddr(loc, CEI.ConcreteValue, ASI, IsNotTake, + IsInitialization_t::IsInitialization); + } else { + B.createStore(loc, CEI.ConcreteValue, ASI, + StoreOwnershipQualifier::Unqualified); + } + return ConcreteArgumentCopy(origArg, ASI); } }; @@ -889,14 +891,19 @@ SILInstruction *SILCombiner::createApplyWithConcreteType( // Ensure that we have a concrete value to propagate. assert(CEI.ConcreteValue); - auto argSub = - ConcreteArgumentCopy::generate(CEI, Apply, ArgIdx, BuilderCtx); - if (argSub) { - UpdatedArgs = true; - concreteArgCopies.push_back(*argSub); - NewArgs.push_back(argSub->tempArgCopy->getDest()); - } else + + if (Apply.getArgument(ArgIdx)->getType().isAddress()) { + auto argSub = + ConcreteArgumentCopy::generate(CEI, Apply, ArgIdx, BuilderCtx); + if (argSub) { + UpdatedArgs = true; + concreteArgCopies.push_back(*argSub); + NewArgs.push_back(argSub->tempArg); + } + } else { NewArgs.push_back(CEI.ConcreteValue); + UpdatedArgs = true; + } // Form a new set of substitutions where the argument is // replaced with a concrete type. @@ -962,8 +969,7 @@ SILInstruction *SILCombiner::createApplyWithConcreteType( auto cleanupLoc = RegularLocation::getAutoGeneratedLocation(); for (ConcreteArgumentCopy &argCopy : llvm::reverse(concreteArgCopies)) { cleanupBuilder.createDestroyAddr(cleanupLoc, argCopy.origArg); - cleanupBuilder.createDeallocStack(cleanupLoc, - argCopy.tempArgCopy->getDest()); + cleanupBuilder.createDeallocStack(cleanupLoc, argCopy.tempArg); } } return NewApply.getInstruction(); diff --git a/lib/SILOptimizer/Utils/Existential.cpp b/lib/SILOptimizer/Utils/Existential.cpp index 28fd2baec1b11..00edb27c1fc2c 100644 --- a/lib/SILOptimizer/Utils/Existential.cpp +++ b/lib/SILOptimizer/Utils/Existential.cpp @@ -213,8 +213,8 @@ OpenedArchetypeInfo::OpenedArchetypeInfo(Operand &use) { openedVal = IEA; if (auto *CAI = dyn_cast(initI)) openedVal = CAI->getSrc(); -// if (auto *store = dyn_cast(initI)) -// openedVal = store->getSrc(); + if (auto *store = dyn_cast(initI)) + openedVal = store->getSrc(); } } if (auto *Open = dyn_cast(openedVal)) { diff --git a/test/SILOptimizer/existential_specializer_indirect_class.sil b/test/SILOptimizer/existential_specializer_indirect_class.sil index 2aa96abf1fbbd..5fb9696cfd986 100644 --- a/test/SILOptimizer/existential_specializer_indirect_class.sil +++ b/test/SILOptimizer/existential_specializer_indirect_class.sil @@ -1,4 +1,4 @@ -// RUN: %target-sil-opt -wmo -enable-sil-verify-all -emit-sorted-sil %s -enable-existential-specializer -existential-specializer -inline -sil-combine -generic-specializer -devirtualizer 2>&1 | %FileCheck %s +// RUN: %target-sil-opt -O -wmo -enable-sil-verify-all %s | %FileCheck %s sil_stage canonical @@ -8,74 +8,68 @@ protocol ClassProtocol: AnyObject { func method() } class C: ClassProtocol { func method() {} } -// CHECK-LABEL: sil shared @$s28test_indirect_class_protocolTf4e_n : $@convention(thin) <τ_0_0 where τ_0_0 : ClassProtocol> (@in τ_0_0) -> () -sil hidden @test_indirect_class_protocol : $@convention(thin) (@in ClassProtocol) -> () { +// CHECK-LABEL: sil [signature_optimized_thunk] [always_inline] @test_indirect_class_protocol : $@convention(thin) (@in ClassProtocol) -> () +sil @test_indirect_class_protocol : $@convention(thin) (@in ClassProtocol) -> () { // CHECK-NEXT: // -// CHECK-NEXT: bb0(%0 : $*τ_0_0): -// CHECK-NEXT: %1 = load %0 -// CHECK-NEXT: %2 = init_existential_ref %1 -// CHECK-NEXT: %3 = alloc_stack $ClassProtocol -// CHECK-NEXT: store %2 to %3 +// CHECK-NEXT: bb0(%0 : $*ClassProtocol): bb0(%0 : $*ClassProtocol): - // CHECK-NEXT: destroy_addr %3 + // CHECK-NEXT: %1 = load %0 + // CHECK-NEXT: strong_release %1 + // CHECK-NEXT: strong_release %1 destroy_addr %0 : $*ClassProtocol - // CHECK-NEXT: dealloc_stack %3 // CHECK-NEXT: return undef return undef : $() } -// CHECK-LABEL: sil shared @$s39test_indirect_class_protocol_guaranteedTf4e_n : $@convention(thin) <τ_0_0 where τ_0_0 : ClassProtocol> (@in_guaranteed τ_0_0) -> () -sil hidden @test_indirect_class_protocol_guaranteed : $@convention(thin) (@in_guaranteed ClassProtocol) -> () { +// CHECK-LABEL: sil [signature_optimized_thunk] [always_inline] @test_indirect_class_protocol_guaranteed : $@convention(thin) (@in_guaranteed ClassProtocol) -> () +sil @test_indirect_class_protocol_guaranteed : $@convention(thin) (@in_guaranteed ClassProtocol) -> () { // CHECK-NEXT: // -// CHECK-NEXT: bb0(%0 : $*τ_0_0): -// CHECK-NEXT: %1 = load %0 -// CHECK-NEXT: %2 = init_existential_ref %1 -// CHECK-NEXT: %3 = alloc_stack $ClassProtocol -// CHECK-NEXT: store %2 to %3 +// CHECK-NEXT: bb0(%0 : $*ClassProtocol): bb0(%0 : $*ClassProtocol): - // CHECK-NEXT: %5 = load %3 : $*ClassProtocol +// CHECK-NEXT: [[INPUT:%1]] = load %0 %1 = load %0 : $*ClassProtocol - // CHECK-NEXT: %6 = open_existential_ref %5 + // CHECK-NEXT: [[OPENED:%.*]] = open_existential_ref [[INPUT]] + // CHECK-NEXT: [[CLOSED:%.*]] = unchecked_ref_cast [[OPENED]] %2 = open_existential_ref %1 : $ClassProtocol to $@opened("ABCDEF01-ABCD-ABCD-ABCD-ABCDEFABCDEF") ClassProtocol - // CHECK-NEXT: %7 = witness_method $C, #ClassProtocol.method + // CHECK-NEXT: [[METHOD:%.*]] = witness_method $C, #ClassProtocol.method %f = witness_method $@opened("ABCDEF01-ABCD-ABCD-ABCD-ABCDEFABCDEF") ClassProtocol, #ClassProtocol.method!1 : (Self) -> () -> (), %2 : $@opened("ABCDEF01-ABCD-ABCD-ABCD-ABCDEFABCDEF") ClassProtocol : $@convention(witness_method : ClassProtocol) (@guaranteed Self) -> () - // CHECK-NEXT: %8 = unchecked_ref_cast %6 - // CHECK-NEXT: %9 = apply %7(%6) + // CHECK-NEXT: apply [[METHOD]]([[CLOSED]]) apply %f<@opened("ABCDEF01-ABCD-ABCD-ABCD-ABCDEFABCDEF") ClassProtocol>(%2) : $@convention(witness_method : ClassProtocol) (@guaranteed Self) -> () - // CHECK-NEXT: dealloc_stack %3 + // CHECK-NEXT: strong_release [[INPUT]] // CHECK-NEXT: return undef return undef : $() } -// CHECK-LABEL: sil hidden @invoke_indirect_class_protocol -sil hidden @invoke_indirect_class_protocol : $@convention(thin) (@guaranteed C) -> () { +// CHECK-LABEL: sil @invoke_indirect_class_protocol : $@convention(thin) (@guaranteed C) -> () + +// Make sure both applies were inlined. + +// CHECK-NEXT: // +// CHECK-NEXT: bb0(%0 : $C): + +// CHECK-NEXT: [[METHOD:%.*]] = witness_method $C, #ClassProtocol.method +// CHECK-NEXT: strong_retain %0 +// CHECK-NEXT: apply [[METHOD]](%0) + +// CHECK-NEXT: strong_release %0 +// CHECK-NEXT: strong_release %0 +// CHECK-NEXT: strong_release %0 + +// CHECK-NEXT: return undef + +sil @invoke_indirect_class_protocol : $@convention(thin) (@guaranteed C) -> () { bb0(%0 : $C): %1 = init_existential_ref %0 : $C : $C, $ClassProtocol - // CHECK: [[INPUT:%.*]] = alloc_stack $ClassProtocol %z = alloc_stack $ClassProtocol retain_value %1 : $ClassProtocol store %1 to %z : $*ClassProtocol - // CHECK: [[SPECIALIZATION:%.*]] = function_ref @$s39test_indirect_class_protocol_guaranteedTf4e_n %f = function_ref @test_indirect_class_protocol_guaranteed : $@convention(thin) (@in_guaranteed ClassProtocol) -> () - // CHECK-NEXT: [[INPUT_LOAD:%.*]] = load [[INPUT]] - // CHECK-NEXT: [[INPUT_OPEN:%.*]] = open_existential_ref [[INPUT_LOAD]] : $ClassProtocol to $[[OPENED_TYPE:@opened(.*) ClassProtocol]] - // CHECK-NEXT: [[INPUT_OPEN_BUF:%.*]] = alloc_stack $[[OPENED_TYPE]] - // CHECK-NEXT: store [[INPUT_OPEN]] to [[INPUT_OPEN_BUF]] - // CHECK-NEXT: apply [[SPECIALIZATION]]<[[OPENED_TYPE]]>([[INPUT_OPEN_BUF]]) apply %f(%z) : $@convention(thin) (@in_guaranteed ClassProtocol) -> () - // CHECK-NEXT: dealloc_stack [[INPUT_OPEN_BUF]] - // CHECK: [[SPECIALIZATION:%.*]] = function_ref @$s28test_indirect_class_protocolTf4e_n %g = function_ref @test_indirect_class_protocol : $@convention(thin) (@in ClassProtocol) -> () - // CHECK-NEXT: [[INPUT_LOAD:%.*]] = load [[INPUT]] - // CHECK-NEXT: [[INPUT_OPEN:%.*]] = open_existential_ref [[INPUT_LOAD]] : $ClassProtocol to $[[OPENED_TYPE:@opened(.*) ClassProtocol]] - // CHECK-NEXT: [[INPUT_OPEN_BUF:%.*]] = alloc_stack $[[OPENED_TYPE]] - // CHECK-NEXT: store [[INPUT_OPEN]] to [[INPUT_OPEN_BUF]] - // CHECK-NEXT: apply [[SPECIALIZATION]]<[[OPENED_TYPE]]>([[INPUT_OPEN_BUF]]) apply %g(%z) : $@convention(thin) (@in ClassProtocol) -> () - // CHECK-NEXT: dealloc_stack [[INPUT_OPEN_BUF]] dealloc_stack %z : $*ClassProtocol return undef : $() From e93919b75495a56fbd94c9f59bb81598b5bd9a03 Mon Sep 17 00:00:00 2001 From: zoecarver Date: Fri, 24 Jan 2020 12:44:11 -0800 Subject: [PATCH 21/36] Add comments, remove commented code, and other general cleanup --- .../SILCombiner/SILCombinerApplyVisitors.cpp | 17 ++++++++--------- lib/SILOptimizer/Utils/Existential.cpp | 13 +++++++------ 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp b/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp index 6c0b9528358b1..cda4aaadcd594 100644 --- a/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp +++ b/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp @@ -791,9 +791,6 @@ struct ConcreteArgumentCopy { assert(!paramInfo.isIndirectMutating() && "A mutated opened existential value can't be replaced"); -// if (!paramInfo.isConsumed()) -// return None; - SILValue origArg = apply.getArgument(argIdx); // FIXME_opaque: With SIL opaque values, a formally indirect argument may be // passed as a SIL object. In this case, generate a copy_value for the new @@ -817,11 +814,13 @@ struct ConcreteArgumentCopy { SILBuilderWithScope B(apply.getInstruction(), BuilderCtx); auto loc = apply.getLoc(); auto *ASI = B.createAllocStack(loc, CEI.ConcreteValue->getType()); - + // If the type is an address, simple copy it. if (CEI.ConcreteValue->getType().isAddress()) { B.createCopyAddr(loc, CEI.ConcreteValue, ASI, IsNotTake, IsInitialization_t::IsInitialization); } else { + // Otherwise, we probably got the value from the source of a store + // instruction so, create a store into the temporary argument. B.createStore(loc, CEI.ConcreteValue, ASI, StoreOwnershipQualifier::Unqualified); } @@ -863,7 +862,6 @@ SILInstruction *SILCombiner::createApplyWithConcreteType( // Create the new set of arguments to apply including their substitutions. SubstitutionMap NewCallSubs = Apply.getSubstitutionMap(); SmallVector NewArgs; - bool UpdatedArgs = false; unsigned ArgIdx = 0; // Push the indirect result arguments. for (unsigned EndIdx = Apply.getSubstCalleeConv().getSILArgIndexOfFirstParam(); @@ -891,18 +889,19 @@ SILInstruction *SILCombiner::createApplyWithConcreteType( // Ensure that we have a concrete value to propagate. assert(CEI.ConcreteValue); - + + // If the parameter is expecting a pointer, then we need to create a + // alloc_stack to store the temporary value. if (Apply.getArgument(ArgIdx)->getType().isAddress()) { auto argSub = ConcreteArgumentCopy::generate(CEI, Apply, ArgIdx, BuilderCtx); if (argSub) { - UpdatedArgs = true; concreteArgCopies.push_back(*argSub); NewArgs.push_back(argSub->tempArg); } } else { + // Otherwise, we can just use the value itself. NewArgs.push_back(CEI.ConcreteValue); - UpdatedArgs = true; } // Form a new set of substitutions where the argument is @@ -926,7 +925,7 @@ SILInstruction *SILCombiner::createApplyWithConcreteType( }); } - if (!UpdatedArgs) { + if (NewArgs.empty()) { // Remove any new instructions created while attempting to optimize this // apply. Since the apply was never rewritten, if they aren't removed here, // they will be removed later as dead when visited by SILCombine, causing diff --git a/lib/SILOptimizer/Utils/Existential.cpp b/lib/SILOptimizer/Utils/Existential.cpp index 00edb27c1fc2c..626c717a640d2 100644 --- a/lib/SILOptimizer/Utils/Existential.cpp +++ b/lib/SILOptimizer/Utils/Existential.cpp @@ -69,9 +69,9 @@ findInitExistentialFromGlobalAddr(GlobalAddrInst *GAI, SILInstruction *Insn) { /// Returns the instruction that initializes the given stack address. This is /// currently either a init_existential_addr, unconditional_checked_cast_addr, -/// store, or copy_addr (if the instruction initializing the source of the copy cannot -/// be determined). Returns nullptr if the initializer does not dominate the -/// alloc_stack user \p ASIUser. If the value is copied from another stack +/// store, or copy_addr (if the instruction initializing the source of the copy +/// cannot be determined). Returns nullptr if the initializer does not dominate +/// the alloc_stack user \p ASIUser. If the value is copied from another stack /// location, \p isCopied is set to true. /// /// allocStackAddr may either itself be an AllocStackInst or an @@ -208,9 +208,10 @@ OpenedArchetypeInfo::OpenedArchetypeInfo(Operand &use) { // %opened to %stack // %instance if (auto *initI = getStackInitInst(instance, user, isOpenedValueCopied)) { - if (auto *IEA = dyn_cast(initI)) - // TODO: this case doesn't need to exist beacuse openedVal will never be used - openedVal = IEA; + // init_existential_addr isn't handled here because it isn't considered an + // "opened" archtype. init_existential_addr should be handled by + // ConcreteExistentialInfo. + if (auto *CAI = dyn_cast(initI)) openedVal = CAI->getSrc(); if (auto *store = dyn_cast(initI)) From 2ac715c22f5d398c93b73bcab8d7417bc623214d Mon Sep 17 00:00:00 2001 From: zoecarver Date: Fri, 24 Jan 2020 12:48:34 -0800 Subject: [PATCH 22/36] Update devirtualize_(...)_two_stores RUN to use sil-opt --- .../devirtualize_protocol_composition_two_stores.sil | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/SILOptimizer/devirtualize_protocol_composition_two_stores.sil b/test/SILOptimizer/devirtualize_protocol_composition_two_stores.sil index 0dcc2f1e2d624..86cf074464c15 100644 --- a/test/SILOptimizer/devirtualize_protocol_composition_two_stores.sil +++ b/test/SILOptimizer/devirtualize_protocol_composition_two_stores.sil @@ -1,4 +1,4 @@ -// RUN: %target-build-swift %s -O -wmo -emit-sil | %FileCheck %s +// RUN: %target-sil-opt -O -wmo -enable-sil-verify-all %s | %FileCheck %s sil_stage canonical From e38da06f7c4e86584fffd8ca94a75d5e2f763b36 Mon Sep 17 00:00:00 2001 From: zoecarver Date: Sun, 26 Jan 2020 19:34:16 -0800 Subject: [PATCH 23/36] Stash debugging changes --- lib/IRGen/AllocStackHoisting.cpp | 3 +++ lib/SILOptimizer/PassManager/PassManager.cpp | 5 ++++ .../SILCombiner/SILCombinerApplyVisitors.cpp | 23 +++++++++++-------- lib/Sema/TypeChecker.cpp | 6 +++-- 4 files changed, 26 insertions(+), 11 deletions(-) diff --git a/lib/IRGen/AllocStackHoisting.cpp b/lib/IRGen/AllocStackHoisting.cpp index e4c125fdbe1cd..8e5fc1fbad84d 100644 --- a/lib/IRGen/AllocStackHoisting.cpp +++ b/lib/IRGen/AllocStackHoisting.cpp @@ -421,6 +421,8 @@ void HoistAllocStack::hoist() { /// Try to hoist generic alloc_stack instructions to the entry block. /// Returns true if the function was changed. bool HoistAllocStack::run() { + llvm::errs() << "Running hoist alloc stack on: " << F->getName() << "\n"; + collectHoistableInstructions(); // Nothing to hoist? @@ -441,6 +443,7 @@ class AllocStackHoisting : public SILFunctionTransform { if (Changed) { PM->invalidateAnalysis(F, SILAnalysis::InvalidationKind::Instructions); } + llvm::errs() << "Completed alloc stack hoisting pass.\n"; } }; } // end anonymous namespace diff --git a/lib/SILOptimizer/PassManager/PassManager.cpp b/lib/SILOptimizer/PassManager/PassManager.cpp index a1b5200e81ebb..93bc1506f81c9 100644 --- a/lib/SILOptimizer/PassManager/PassManager.cpp +++ b/lib/SILOptimizer/PassManager/PassManager.cpp @@ -407,6 +407,11 @@ void SILPassManager::runPassOnFunction(unsigned TransIdx, SILFunction *F) { Mod->registerDeleteNotificationHandler(SFT); if (breakBeforeRunning(F->getName(), SFT)) LLVM_BUILTIN_DEBUGTRAP; + llvm::errs() << "Running SIL pass: " << SFT->getID() << "\n"; + llvm::errs() << "Function named: " << F->getName() << "\n"; + F->getLocation().dump(F->getModule().getSourceManager()); + llvm::errs() << "***** \n"; + SFT->run(); assert(analysesUnlocked() && "Expected all analyses to be unlocked!"); Mod->removeDeleteNotificationHandler(SFT); diff --git a/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp b/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp index cda4aaadcd594..6526567750344 100644 --- a/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp +++ b/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp @@ -791,6 +791,9 @@ struct ConcreteArgumentCopy { assert(!paramInfo.isIndirectMutating() && "A mutated opened existential value can't be replaced"); + if (!paramInfo.isConsumed()) + return None; + SILValue origArg = apply.getArgument(argIdx); // FIXME_opaque: With SIL opaque values, a formally indirect argument may be // passed as a SIL object. In this case, generate a copy_value for the new @@ -821,6 +824,7 @@ struct ConcreteArgumentCopy { } else { // Otherwise, we probably got the value from the source of a store // instruction so, create a store into the temporary argument. + B.createStrongRetain(loc, CEI.ConcreteValue, B.getDefaultAtomicity()); B.createStore(loc, CEI.ConcreteValue, ASI, StoreOwnershipQualifier::Unqualified); } @@ -856,6 +860,11 @@ SILInstruction *SILCombiner::createApplyWithConcreteType( const llvm::SmallDenseMap &COAIs, SILBuilderContext &BuilderCtx) { + // Bail on try_apply. + // TODO: support try_apply. + if (isa(Apply)) + return nullptr; + // Ensure that the callee is polymorphic. assert(Apply.getOrigCalleeType()->isPolymorphic()); @@ -898,6 +907,8 @@ SILInstruction *SILCombiner::createApplyWithConcreteType( if (argSub) { concreteArgCopies.push_back(*argSub); NewArgs.push_back(argSub->tempArg); + } else { + NewArgs.clear(); } } else { // Otherwise, we can just use the value itself. @@ -936,15 +947,9 @@ SILInstruction *SILCombiner::createApplyWithConcreteType( } // Now create the new apply instruction. SILBuilderWithScope ApplyBuilder(Apply.getInstruction(), BuilderCtx); - FullApplySite NewApply; - if (auto *TAI = dyn_cast(Apply)) - NewApply = ApplyBuilder.createTryApply( - Apply.getLoc(), Apply.getCallee(), NewCallSubs, NewArgs, - TAI->getNormalBB(), TAI->getErrorBB()); - else - NewApply = ApplyBuilder.createApply( - Apply.getLoc(), Apply.getCallee(), NewCallSubs, NewArgs, - cast(Apply)->isNonThrowing()); + FullApplySite NewApply = ApplyBuilder.createApply( + Apply.getLoc(), Apply.getCallee(), NewCallSubs, NewArgs, + cast(Apply)->isNonThrowing()); if (auto NewAI = dyn_cast(NewApply)) replaceInstUsesWith(*cast(Apply.getInstruction()), NewAI); diff --git a/lib/Sema/TypeChecker.cpp b/lib/Sema/TypeChecker.cpp index 4918500c12e5f..8f1ea1b517438 100644 --- a/lib/Sema/TypeChecker.cpp +++ b/lib/Sema/TypeChecker.cpp @@ -559,12 +559,14 @@ swift::handleSILGenericParams(GenericParamList *genericParams, auto genericParams = nestedList[i]; genericParams->setDepth(i); } - + llvm::errs() << "Checking generic sig..."; auto sig = TypeChecker::checkGenericSignature(nestedList.back(), DC, /*parentSig=*/nullptr, /*allowConcreteGenericParams=*/true); - return (sig ? sig->getGenericEnvironment() : nullptr); + auto out = (sig ? sig->getGenericEnvironment() : nullptr); + llvm::errs() << "done.\n"; + return out; } void swift::typeCheckPatternBinding(PatternBindingDecl *PBD, From 809f569bccb3886ff6394bfd04ce7d2b0163c63e Mon Sep 17 00:00:00 2001 From: zoecarver Date: Tue, 28 Jan 2020 15:27:37 -0800 Subject: [PATCH 24/36] Remove debug code and update check for successful conversion of arguments. --- lib/IRGen/AllocStackHoisting.cpp | 3 --- lib/SILOptimizer/PassManager/PassManager.cpp | 5 ----- lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp | 4 +--- lib/Sema/TypeChecker.cpp | 6 ++---- 4 files changed, 3 insertions(+), 15 deletions(-) diff --git a/lib/IRGen/AllocStackHoisting.cpp b/lib/IRGen/AllocStackHoisting.cpp index 8e5fc1fbad84d..e4c125fdbe1cd 100644 --- a/lib/IRGen/AllocStackHoisting.cpp +++ b/lib/IRGen/AllocStackHoisting.cpp @@ -421,8 +421,6 @@ void HoistAllocStack::hoist() { /// Try to hoist generic alloc_stack instructions to the entry block. /// Returns true if the function was changed. bool HoistAllocStack::run() { - llvm::errs() << "Running hoist alloc stack on: " << F->getName() << "\n"; - collectHoistableInstructions(); // Nothing to hoist? @@ -443,7 +441,6 @@ class AllocStackHoisting : public SILFunctionTransform { if (Changed) { PM->invalidateAnalysis(F, SILAnalysis::InvalidationKind::Instructions); } - llvm::errs() << "Completed alloc stack hoisting pass.\n"; } }; } // end anonymous namespace diff --git a/lib/SILOptimizer/PassManager/PassManager.cpp b/lib/SILOptimizer/PassManager/PassManager.cpp index 93bc1506f81c9..a1b5200e81ebb 100644 --- a/lib/SILOptimizer/PassManager/PassManager.cpp +++ b/lib/SILOptimizer/PassManager/PassManager.cpp @@ -407,11 +407,6 @@ void SILPassManager::runPassOnFunction(unsigned TransIdx, SILFunction *F) { Mod->registerDeleteNotificationHandler(SFT); if (breakBeforeRunning(F->getName(), SFT)) LLVM_BUILTIN_DEBUGTRAP; - llvm::errs() << "Running SIL pass: " << SFT->getID() << "\n"; - llvm::errs() << "Function named: " << F->getName() << "\n"; - F->getLocation().dump(F->getModule().getSourceManager()); - llvm::errs() << "***** \n"; - SFT->run(); assert(analysesUnlocked() && "Expected all analyses to be unlocked!"); Mod->removeDeleteNotificationHandler(SFT); diff --git a/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp b/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp index 6526567750344..9b68c3d6af41e 100644 --- a/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp +++ b/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp @@ -907,8 +907,6 @@ SILInstruction *SILCombiner::createApplyWithConcreteType( if (argSub) { concreteArgCopies.push_back(*argSub); NewArgs.push_back(argSub->tempArg); - } else { - NewArgs.clear(); } } else { // Otherwise, we can just use the value itself. @@ -936,7 +934,7 @@ SILInstruction *SILCombiner::createApplyWithConcreteType( }); } - if (NewArgs.empty()) { + if (NewArgs.size() != Apply.getNumArguments()) { // Remove any new instructions created while attempting to optimize this // apply. Since the apply was never rewritten, if they aren't removed here, // they will be removed later as dead when visited by SILCombine, causing diff --git a/lib/Sema/TypeChecker.cpp b/lib/Sema/TypeChecker.cpp index 8f1ea1b517438..4918500c12e5f 100644 --- a/lib/Sema/TypeChecker.cpp +++ b/lib/Sema/TypeChecker.cpp @@ -559,14 +559,12 @@ swift::handleSILGenericParams(GenericParamList *genericParams, auto genericParams = nestedList[i]; genericParams->setDepth(i); } - llvm::errs() << "Checking generic sig..."; + auto sig = TypeChecker::checkGenericSignature(nestedList.back(), DC, /*parentSig=*/nullptr, /*allowConcreteGenericParams=*/true); - auto out = (sig ? sig->getGenericEnvironment() : nullptr); - llvm::errs() << "done.\n"; - return out; + return (sig ? sig->getGenericEnvironment() : nullptr); } void swift::typeCheckPatternBinding(PatternBindingDecl *PBD, From ddeef6483623af432fed5fdbbd6c7e84df351085 Mon Sep 17 00:00:00 2001 From: zoecarver Date: Tue, 28 Jan 2020 17:09:52 -0800 Subject: [PATCH 25/36] Re-add support for try_apply instruction and update tests. Re-work tests based on updated optimizations and check specialized functions. --- .../SILCombiner/SILCombinerApplyVisitors.cpp | 18 ++++---- ...existential_specializer_indirect_class.sil | 42 +++++++++---------- 2 files changed, 28 insertions(+), 32 deletions(-) diff --git a/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp b/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp index 9b68c3d6af41e..d20998b4b9a5d 100644 --- a/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp +++ b/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp @@ -859,12 +859,6 @@ SILInstruction *SILCombiner::createApplyWithConcreteType( FullApplySite Apply, const llvm::SmallDenseMap &COAIs, SILBuilderContext &BuilderCtx) { - - // Bail on try_apply. - // TODO: support try_apply. - if (isa(Apply)) - return nullptr; - // Ensure that the callee is polymorphic. assert(Apply.getOrigCalleeType()->isPolymorphic()); @@ -945,9 +939,15 @@ SILInstruction *SILCombiner::createApplyWithConcreteType( } // Now create the new apply instruction. SILBuilderWithScope ApplyBuilder(Apply.getInstruction(), BuilderCtx); - FullApplySite NewApply = ApplyBuilder.createApply( - Apply.getLoc(), Apply.getCallee(), NewCallSubs, NewArgs, - cast(Apply)->isNonThrowing()); + FullApplySite NewApply; + if (auto *TAI = dyn_cast(Apply)) + NewApply = ApplyBuilder.createTryApply( + Apply.getLoc(), Apply.getCallee(), NewCallSubs, NewArgs, + TAI->getNormalBB(), TAI->getErrorBB()); + else + NewApply = ApplyBuilder.createApply( + Apply.getLoc(), Apply.getCallee(), NewCallSubs, NewArgs, + cast(Apply)->isNonThrowing()); if (auto NewAI = dyn_cast(NewApply)) replaceInstUsesWith(*cast(Apply.getInstruction()), NewAI); diff --git a/test/SILOptimizer/existential_specializer_indirect_class.sil b/test/SILOptimizer/existential_specializer_indirect_class.sil index 5fb9696cfd986..7f0ea56c56d81 100644 --- a/test/SILOptimizer/existential_specializer_indirect_class.sil +++ b/test/SILOptimizer/existential_specializer_indirect_class.sil @@ -1,4 +1,4 @@ -// RUN: %target-sil-opt -O -wmo -enable-sil-verify-all %s | %FileCheck %s +// RUN: %target-sil-opt -O -wmo -enable-sil-verify-all -sil-disable-pass=DeadFunctionElimination %s | %FileCheck %s sil_stage canonical @@ -15,46 +15,42 @@ sil @test_indirect_class_protocol : $@convention(thin) (@in ClassProtocol) -> () bb0(%0 : $*ClassProtocol): // CHECK-NEXT: %1 = load %0 // CHECK-NEXT: strong_release %1 - // CHECK-NEXT: strong_release %1 destroy_addr %0 : $*ClassProtocol // CHECK-NEXT: return undef return undef : $() } -// CHECK-LABEL: sil [signature_optimized_thunk] [always_inline] @test_indirect_class_protocol_guaranteed : $@convention(thin) (@in_guaranteed ClassProtocol) -> () -sil @test_indirect_class_protocol_guaranteed : $@convention(thin) (@in_guaranteed ClassProtocol) -> () { +// Check that all the opened types are optimized away in the specialization of test_indirect_class_protocol_guaranteed +// CHECK-LABEL: sil shared @$s39test_indirect_class_protocol_guaranteedTf4e_n : $@convention(thin) <τ_0_0 where τ_0_0 : ClassProtocol> (@in_guaranteed τ_0_0) -> () // CHECK-NEXT: // -// CHECK-NEXT: bb0(%0 : $*ClassProtocol): +// CHECK-NEXT: bb0(%0 : $*τ_0_0): + // CHECK-NEXT: [[INPUT:%1]] = load %0 + // CHECK-NEXT: [[METHOD:%.*]] = witness_method $C, #ClassProtocol.method + // CHECK-NEXT: [[ARG:%.*]] = unchecked_ref_cast [[INPUT]] + // CHECK-NEXT: apply [[METHOD]]([[ARG]]) + // CHECK-NEXT: return undef + +sil @test_indirect_class_protocol_guaranteed : $@convention(thin) (@in_guaranteed ClassProtocol) -> () { bb0(%0 : $*ClassProtocol): -// CHECK-NEXT: [[INPUT:%1]] = load %0 %1 = load %0 : $*ClassProtocol - // CHECK-NEXT: [[OPENED:%.*]] = open_existential_ref [[INPUT]] - // CHECK-NEXT: [[CLOSED:%.*]] = unchecked_ref_cast [[OPENED]] %2 = open_existential_ref %1 : $ClassProtocol to $@opened("ABCDEF01-ABCD-ABCD-ABCD-ABCDEFABCDEF") ClassProtocol - // CHECK-NEXT: [[METHOD:%.*]] = witness_method $C, #ClassProtocol.method %f = witness_method $@opened("ABCDEF01-ABCD-ABCD-ABCD-ABCDEFABCDEF") ClassProtocol, #ClassProtocol.method!1 : (Self) -> () -> (), %2 : $@opened("ABCDEF01-ABCD-ABCD-ABCD-ABCDEFABCDEF") ClassProtocol : $@convention(witness_method : ClassProtocol) (@guaranteed Self) -> () - // CHECK-NEXT: apply [[METHOD]]([[CLOSED]]) apply %f<@opened("ABCDEF01-ABCD-ABCD-ABCD-ABCDEFABCDEF") ClassProtocol>(%2) : $@convention(witness_method : ClassProtocol) (@guaranteed Self) -> () - // CHECK-NEXT: strong_release [[INPUT]] - // CHECK-NEXT: return undef return undef : $() } -// CHECK-LABEL: sil @invoke_indirect_class_protocol : $@convention(thin) (@guaranteed C) -> () - -// Make sure both applies were inlined. - +// Check that a specialization of test_indirect_class_protocol is created. +// CHECK-LABEL: sil shared [signature_optimized_thunk] [always_inline] @$s28test_indirect_class_protocolTf4e_n4main1CC_Tg5 : $@convention(thin) (@owned C) -> () // CHECK-NEXT: // // CHECK-NEXT: bb0(%0 : $C): - -// CHECK-NEXT: [[METHOD:%.*]] = witness_method $C, #ClassProtocol.method -// CHECK-NEXT: strong_retain %0 -// CHECK-NEXT: apply [[METHOD]](%0) - -// CHECK-NEXT: strong_release %0 -// CHECK-NEXT: strong_release %0 // CHECK-NEXT: strong_release %0 +// CHECK-NEXT: return undef +// CHECK-LABEL: end sil function '$s28test_indirect_class_protocolTf4e_n4main1CC_Tg5' +// Check the generated specialization of test_indirect_class_protocol +// CHECK-LABEL: sil shared @$s28test_indirect_class_protocolTf4e_n4main1CC_Tg5Tf4d_n : $@convention(thin) () -> () +// Make sure *everything* was inlined / optimized away +// CHECK-NEXT: bb0: // CHECK-NEXT: return undef sil @invoke_indirect_class_protocol : $@convention(thin) (@guaranteed C) -> () { From ac4fa332abfc5318a6c3ee4f346f613ed5d72b10 Mon Sep 17 00:00:00 2001 From: zoecarver Date: Fri, 31 Jan 2020 09:17:07 -0800 Subject: [PATCH 26/36] Stash optimization debugging --- .../ExistentialSpecializer.cpp | 3 +++ lib/SILOptimizer/PassManager/PassManager.cpp | 15 +++++++++++++++ lib/SILOptimizer/Transforms/AllocBoxToStack.cpp | 2 ++ lib/SILOptimizer/Transforms/Devirtualizer.cpp | 5 ++++- .../Transforms/GenericSpecializer.cpp | 2 ++ 5 files changed, 26 insertions(+), 1 deletion(-) diff --git a/lib/SILOptimizer/FunctionSignatureTransforms/ExistentialSpecializer.cpp b/lib/SILOptimizer/FunctionSignatureTransforms/ExistentialSpecializer.cpp index 44954304de13c..667f4b47791e4 100644 --- a/lib/SILOptimizer/FunctionSignatureTransforms/ExistentialSpecializer.cpp +++ b/lib/SILOptimizer/FunctionSignatureTransforms/ExistentialSpecializer.cpp @@ -336,6 +336,9 @@ void ExistentialSpecializer::specializeExistentialArgsInAppliesWithinFunction( /// Update statistics on the number of functions specialized. ++NumFunctionsWithExistentialArgsSpecialized; + llvm::errs() << "Adding function to pass manger in existential\n"; + llvm::errs() << ET.getExistentialSpecializedFunction()->getName() << "\n"; + /// Make sure the PM knows about the new specialized inner function. addFunctionToPassManagerWorklist(ET.getExistentialSpecializedFunction(), Callee); diff --git a/lib/SILOptimizer/PassManager/PassManager.cpp b/lib/SILOptimizer/PassManager/PassManager.cpp index a1b5200e81ebb..73e77ca0aad9e 100644 --- a/lib/SILOptimizer/PassManager/PassManager.cpp +++ b/lib/SILOptimizer/PassManager/PassManager.cpp @@ -407,9 +407,24 @@ void SILPassManager::runPassOnFunction(unsigned TransIdx, SILFunction *F) { Mod->registerDeleteNotificationHandler(SFT); if (breakBeforeRunning(F->getName(), SFT)) LLVM_BUILTIN_DEBUGTRAP; + + + llvm::errs() << "FUNC NAME: "; + F->printName(llvm::errs()); + llvm::errs() << "\nDEMA NAME: " << Demangle::demangleSymbolAsString(F->getName()) << "\n"; + llvm::errs() << "PASS NAME: " << SFT->getID() << "\n"; + SFT->run(); assert(analysesUnlocked() && "Expected all analyses to be unlocked!"); Mod->removeDeleteNotificationHandler(SFT); + + llvm::errs() << "Finished running pass\n\t\t****\n"; + if (F->getName().contains("ss6UInt32VSjsSj9magnitude9MagnitudeQzvgTW") || + F->getName().contains("ss2geoiySbx_q_q0_q1_q2_q3_t_x_q_q0_q1_q2_q3_ttSLRzSLR_SLR0_SLR1_SLR2_SLR3_r4_lF")) { + llvm::errs() << "Dumping module to file..."; + F->getModule().dump("/Users/zoe/Developer/swift-source/build/buildbot_incremental/swift-macosx-x86_64/module_dump.txt"); + llvm::errs() << "done.\n"; + } auto Delta = (std::chrono::system_clock::now() - StartTime).count(); if (SILPrintPassTime) { diff --git a/lib/SILOptimizer/Transforms/AllocBoxToStack.cpp b/lib/SILOptimizer/Transforms/AllocBoxToStack.cpp index ddf770d2a0874..2e79bfcae2175 100644 --- a/lib/SILOptimizer/Transforms/AllocBoxToStack.cpp +++ b/lib/SILOptimizer/Transforms/AllocBoxToStack.cpp @@ -793,6 +793,8 @@ specializePartialApply(SILOptFunctionBuilder &FuncBuilder, ClonedName); Cloner.populateCloned(); ClonedFn = Cloner.getCloned(); + llvm::errs() << "Adding function to pass manger in alloc box\n"; + llvm::errs() << ClonedFn->getName() << "\n"; pass.T->addFunctionToPassManagerWorklist(ClonedFn, F); } diff --git a/lib/SILOptimizer/Transforms/Devirtualizer.cpp b/lib/SILOptimizer/Transforms/Devirtualizer.cpp index 27e140b6993be..b14f7fb12bc28 100644 --- a/lib/SILOptimizer/Transforms/Devirtualizer.cpp +++ b/lib/SILOptimizer/Transforms/Devirtualizer.cpp @@ -102,8 +102,11 @@ bool Devirtualizer::devirtualizeAppliesInFunction(SILFunction &F, // We may not have optimized these functions yet, and it could // be beneficial to rerun some earlier passes on the current // function now that we've made these direct references visible. - if (CalleeFn->isDefinition() && CalleeFn->shouldOptimize()) + if (CalleeFn->isDefinition() && CalleeFn->shouldOptimize()) { + llvm::errs() << "Adding function to pass manger in devirt\n"; + llvm::errs() << CalleeFn->getName() << "\n"; addFunctionToPassManagerWorklist(CalleeFn, nullptr); + } } return Changed; diff --git a/lib/SILOptimizer/Transforms/GenericSpecializer.cpp b/lib/SILOptimizer/Transforms/GenericSpecializer.cpp index 82dd9d4d33ce8..5817677c58302 100644 --- a/lib/SILOptimizer/Transforms/GenericSpecializer.cpp +++ b/lib/SILOptimizer/Transforms/GenericSpecializer.cpp @@ -129,6 +129,8 @@ bool GenericSpecializer::specializeAppliesInFunction(SILFunction &F) { // (as opposed to returning a previous specialization), we need to notify // the pass manager so that the new functions get optimized. for (SILFunction *NewF : reverse(NewFunctions)) { + llvm::errs() << "Adding function to pass manger in generic spec\n"; + llvm::errs() << NewF->getName() << "\n"; addFunctionToPassManagerWorklist(NewF, Callee); } } From 4d2c342639057c5748e8b5c71e3a52b509e415c5 Mon Sep 17 00:00:00 2001 From: zoecarver Date: Sun, 2 Feb 2020 12:39:30 -0800 Subject: [PATCH 27/36] Update how createApplyWithConcreteType checks if it can update Apply args createApplyWithConcreteType used to update Apply unconditionally. There may have been cases prior to supporting store instructions that miscompiled because of this but, there are certainly cases after supporting store instructions. Therefore, it is important that the apply is updated only when the substitution can accept the new args. --- .../ExistentialSpecializer.cpp | 3 --- lib/SILOptimizer/PassManager/PassManager.cpp | 15 ----------- .../SILCombiner/SILCombinerApplyVisitors.cpp | 27 +++++++++++-------- .../Transforms/AllocBoxToStack.cpp | 2 -- lib/SILOptimizer/Transforms/Devirtualizer.cpp | 5 +--- .../Transforms/GenericSpecializer.cpp | 2 -- 6 files changed, 17 insertions(+), 37 deletions(-) diff --git a/lib/SILOptimizer/FunctionSignatureTransforms/ExistentialSpecializer.cpp b/lib/SILOptimizer/FunctionSignatureTransforms/ExistentialSpecializer.cpp index 667f4b47791e4..44954304de13c 100644 --- a/lib/SILOptimizer/FunctionSignatureTransforms/ExistentialSpecializer.cpp +++ b/lib/SILOptimizer/FunctionSignatureTransforms/ExistentialSpecializer.cpp @@ -336,9 +336,6 @@ void ExistentialSpecializer::specializeExistentialArgsInAppliesWithinFunction( /// Update statistics on the number of functions specialized. ++NumFunctionsWithExistentialArgsSpecialized; - llvm::errs() << "Adding function to pass manger in existential\n"; - llvm::errs() << ET.getExistentialSpecializedFunction()->getName() << "\n"; - /// Make sure the PM knows about the new specialized inner function. addFunctionToPassManagerWorklist(ET.getExistentialSpecializedFunction(), Callee); diff --git a/lib/SILOptimizer/PassManager/PassManager.cpp b/lib/SILOptimizer/PassManager/PassManager.cpp index 73e77ca0aad9e..a1b5200e81ebb 100644 --- a/lib/SILOptimizer/PassManager/PassManager.cpp +++ b/lib/SILOptimizer/PassManager/PassManager.cpp @@ -407,24 +407,9 @@ void SILPassManager::runPassOnFunction(unsigned TransIdx, SILFunction *F) { Mod->registerDeleteNotificationHandler(SFT); if (breakBeforeRunning(F->getName(), SFT)) LLVM_BUILTIN_DEBUGTRAP; - - - llvm::errs() << "FUNC NAME: "; - F->printName(llvm::errs()); - llvm::errs() << "\nDEMA NAME: " << Demangle::demangleSymbolAsString(F->getName()) << "\n"; - llvm::errs() << "PASS NAME: " << SFT->getID() << "\n"; - SFT->run(); assert(analysesUnlocked() && "Expected all analyses to be unlocked!"); Mod->removeDeleteNotificationHandler(SFT); - - llvm::errs() << "Finished running pass\n\t\t****\n"; - if (F->getName().contains("ss6UInt32VSjsSj9magnitude9MagnitudeQzvgTW") || - F->getName().contains("ss2geoiySbx_q_q0_q1_q2_q3_t_x_q_q0_q1_q2_q3_ttSLRzSLR_SLR0_SLR1_SLR2_SLR3_r4_lF")) { - llvm::errs() << "Dumping module to file..."; - F->getModule().dump("/Users/zoe/Developer/swift-source/build/buildbot_incremental/swift-macosx-x86_64/module_dump.txt"); - llvm::errs() << "done.\n"; - } auto Delta = (std::chrono::system_clock::now() - StartTime).count(); if (SILPrintPassTime) { diff --git a/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp b/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp index d20998b4b9a5d..7d013e39589c0 100644 --- a/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp +++ b/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp @@ -893,17 +893,12 @@ SILInstruction *SILCombiner::createApplyWithConcreteType( // Ensure that we have a concrete value to propagate. assert(CEI.ConcreteValue); - // If the parameter is expecting a pointer, then we need to create a - // alloc_stack to store the temporary value. - if (Apply.getArgument(ArgIdx)->getType().isAddress()) { - auto argSub = - ConcreteArgumentCopy::generate(CEI, Apply, ArgIdx, BuilderCtx); - if (argSub) { - concreteArgCopies.push_back(*argSub); - NewArgs.push_back(argSub->tempArg); - } + auto argSub = + ConcreteArgumentCopy::generate(CEI, Apply, ArgIdx, BuilderCtx); + if (argSub) { + concreteArgCopies.push_back(*argSub); + NewArgs.push_back(argSub->tempArg); } else { - // Otherwise, we can just use the value itself. NewArgs.push_back(CEI.ConcreteValue); } @@ -928,7 +923,17 @@ SILInstruction *SILCombiner::createApplyWithConcreteType( }); } - if (NewArgs.size() != Apply.getNumArguments()) { + bool canUpdateArgs = [&]() { + auto substTy = Apply.getCallee()->getType().substGenericArgs(Apply.getModule(), NewCallSubs, Apply.getFunction()->getTypeExpansionContext()).getAs(); + SILFunctionConventions conv(substTy, SILModuleConventions(Apply.getModule())); + bool canUpdate = true; + for (unsigned index = 0; index < conv.getNumSILArguments(); ++index) { + canUpdate &= conv.getSILArgumentType(index) == NewArgs[index]->getType(); + } + return canUpdate; + }(); + + if (!canUpdateArgs) { // Remove any new instructions created while attempting to optimize this // apply. Since the apply was never rewritten, if they aren't removed here, // they will be removed later as dead when visited by SILCombine, causing diff --git a/lib/SILOptimizer/Transforms/AllocBoxToStack.cpp b/lib/SILOptimizer/Transforms/AllocBoxToStack.cpp index 2e79bfcae2175..ddf770d2a0874 100644 --- a/lib/SILOptimizer/Transforms/AllocBoxToStack.cpp +++ b/lib/SILOptimizer/Transforms/AllocBoxToStack.cpp @@ -793,8 +793,6 @@ specializePartialApply(SILOptFunctionBuilder &FuncBuilder, ClonedName); Cloner.populateCloned(); ClonedFn = Cloner.getCloned(); - llvm::errs() << "Adding function to pass manger in alloc box\n"; - llvm::errs() << ClonedFn->getName() << "\n"; pass.T->addFunctionToPassManagerWorklist(ClonedFn, F); } diff --git a/lib/SILOptimizer/Transforms/Devirtualizer.cpp b/lib/SILOptimizer/Transforms/Devirtualizer.cpp index b14f7fb12bc28..27e140b6993be 100644 --- a/lib/SILOptimizer/Transforms/Devirtualizer.cpp +++ b/lib/SILOptimizer/Transforms/Devirtualizer.cpp @@ -102,11 +102,8 @@ bool Devirtualizer::devirtualizeAppliesInFunction(SILFunction &F, // We may not have optimized these functions yet, and it could // be beneficial to rerun some earlier passes on the current // function now that we've made these direct references visible. - if (CalleeFn->isDefinition() && CalleeFn->shouldOptimize()) { - llvm::errs() << "Adding function to pass manger in devirt\n"; - llvm::errs() << CalleeFn->getName() << "\n"; + if (CalleeFn->isDefinition() && CalleeFn->shouldOptimize()) addFunctionToPassManagerWorklist(CalleeFn, nullptr); - } } return Changed; diff --git a/lib/SILOptimizer/Transforms/GenericSpecializer.cpp b/lib/SILOptimizer/Transforms/GenericSpecializer.cpp index 5817677c58302..82dd9d4d33ce8 100644 --- a/lib/SILOptimizer/Transforms/GenericSpecializer.cpp +++ b/lib/SILOptimizer/Transforms/GenericSpecializer.cpp @@ -129,8 +129,6 @@ bool GenericSpecializer::specializeAppliesInFunction(SILFunction &F) { // (as opposed to returning a previous specialization), we need to notify // the pass manager so that the new functions get optimized. for (SILFunction *NewF : reverse(NewFunctions)) { - llvm::errs() << "Adding function to pass manger in generic spec\n"; - llvm::errs() << NewF->getName() << "\n"; addFunctionToPassManagerWorklist(NewF, Callee); } } From a0fb139260d7dae389037d96958d7aecdf02e621 Mon Sep 17 00:00:00 2001 From: zoecarver Date: Sun, 2 Feb 2020 12:57:50 -0800 Subject: [PATCH 28/36] Format --- .../SILCombiner/SILCombinerApplyVisitors.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp b/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp index 7d013e39589c0..76f38c5e97de1 100644 --- a/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp +++ b/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp @@ -792,7 +792,7 @@ struct ConcreteArgumentCopy { && "A mutated opened existential value can't be replaced"); if (!paramInfo.isConsumed()) - return None; + return None; SILValue origArg = apply.getArgument(argIdx); // FIXME_opaque: With SIL opaque values, a formally indirect argument may be @@ -924,8 +924,14 @@ SILInstruction *SILCombiner::createApplyWithConcreteType( } bool canUpdateArgs = [&]() { - auto substTy = Apply.getCallee()->getType().substGenericArgs(Apply.getModule(), NewCallSubs, Apply.getFunction()->getTypeExpansionContext()).getAs(); - SILFunctionConventions conv(substTy, SILModuleConventions(Apply.getModule())); + auto substTy = + Apply.getCallee() + ->getType() + .substGenericArgs(Apply.getModule(), NewCallSubs, + Apply.getFunction()->getTypeExpansionContext()) + .getAs(); + SILFunctionConventions conv(substTy, + SILModuleConventions(Apply.getModule())); bool canUpdate = true; for (unsigned index = 0; index < conv.getNumSILArguments(); ++index) { canUpdate &= conv.getSILArgumentType(index) == NewArgs[index]->getType(); From 65e8d37b6f011ee0231466c2d3cc254171444469 Mon Sep 17 00:00:00 2001 From: zoecarver Date: Sun, 2 Feb 2020 19:15:01 -0800 Subject: [PATCH 29/36] Add a `madeUpdate` check to `createApplyWithConcreteType` to prevent infinite loop --- lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp b/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp index 76f38c5e97de1..b5115659411a1 100644 --- a/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp +++ b/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp @@ -871,6 +871,11 @@ SILInstruction *SILCombiner::createApplyWithConcreteType( ArgIdx < EndIdx; ++ArgIdx) { NewArgs.push_back(Apply.getArgument(ArgIdx)); } + + // Keep track of weather we made any updates at all. Otherwise, we will + // have an infinite loop. + bool madeUpdate = false; + // Transform the parameter arguments. SmallVector concreteArgCopies; for (unsigned EndIdx = Apply.getNumArguments(); ArgIdx < EndIdx; ++ArgIdx) { @@ -898,6 +903,7 @@ SILInstruction *SILCombiner::createApplyWithConcreteType( if (argSub) { concreteArgCopies.push_back(*argSub); NewArgs.push_back(argSub->tempArg); + madeUpdate = true; } else { NewArgs.push_back(CEI.ConcreteValue); } @@ -939,7 +945,7 @@ SILInstruction *SILCombiner::createApplyWithConcreteType( return canUpdate; }(); - if (!canUpdateArgs) { + if (!canUpdateArgs || !madeUpdate) { // Remove any new instructions created while attempting to optimize this // apply. Since the apply was never rewritten, if they aren't removed here, // they will be removed later as dead when visited by SILCombine, causing From 37c0eb2a5cb48106ac1af5cc4276852e7f592973 Mon Sep 17 00:00:00 2001 From: zoecarver Date: Mon, 3 Feb 2020 10:10:10 -0800 Subject: [PATCH 30/36] Manually check if types are the same when checking for change and update tests --- .../SILCombiner/SILCombinerApplyVisitors.cpp | 16 +++--- ...ualize_protocol_composition_two_stores.sil | 50 +++++++++---------- 2 files changed, 33 insertions(+), 33 deletions(-) diff --git a/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp b/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp index b5115659411a1..8612580196e8b 100644 --- a/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp +++ b/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp @@ -872,10 +872,6 @@ SILInstruction *SILCombiner::createApplyWithConcreteType( NewArgs.push_back(Apply.getArgument(ArgIdx)); } - // Keep track of weather we made any updates at all. Otherwise, we will - // have an infinite loop. - bool madeUpdate = false; - // Transform the parameter arguments. SmallVector concreteArgCopies; for (unsigned EndIdx = Apply.getNumArguments(); ArgIdx < EndIdx; ++ArgIdx) { @@ -903,7 +899,6 @@ SILInstruction *SILCombiner::createApplyWithConcreteType( if (argSub) { concreteArgCopies.push_back(*argSub); NewArgs.push_back(argSub->tempArg); - madeUpdate = true; } else { NewArgs.push_back(CEI.ConcreteValue); } @@ -929,7 +924,8 @@ SILInstruction *SILCombiner::createApplyWithConcreteType( }); } - bool canUpdateArgs = [&]() { + bool canUpdateArgs, madeUpdate; + std::tie(canUpdateArgs, madeUpdate) = [&]() { auto substTy = Apply.getCallee() ->getType() @@ -939,12 +935,16 @@ SILInstruction *SILCombiner::createApplyWithConcreteType( SILFunctionConventions conv(substTy, SILModuleConventions(Apply.getModule())); bool canUpdate = true; + // Keep track of weather we made any updates at all. Otherwise, we will + // have an infinite loop. + bool madeUpdate = false; for (unsigned index = 0; index < conv.getNumSILArguments(); ++index) { canUpdate &= conv.getSILArgumentType(index) == NewArgs[index]->getType(); + madeUpdate |= NewArgs[index]->getType() != Apply.getArgument(index)->getType(); } - return canUpdate; + return std::make_tuple(canUpdate, madeUpdate); }(); - + if (!canUpdateArgs || !madeUpdate) { // Remove any new instructions created while attempting to optimize this // apply. Since the apply was never rewritten, if they aren't removed here, diff --git a/test/SILOptimizer/devirtualize_protocol_composition_two_stores.sil b/test/SILOptimizer/devirtualize_protocol_composition_two_stores.sil index 86cf074464c15..231a6a1c88e18 100644 --- a/test/SILOptimizer/devirtualize_protocol_composition_two_stores.sil +++ b/test/SILOptimizer/devirtualize_protocol_composition_two_stores.sil @@ -12,43 +12,43 @@ public class A { } public protocol P { - func foo() -> Int + func foo() -> Int64 } public class B : A, P { - public func foo() -> Int + public func foo() -> Int64 @objc deinit override init() } public class C : A, P { - public func foo() -> Int + public func foo() -> Int64 @objc deinit override init() } -func imp(x: A & P, y: A & P) -> Int +func imp(x: A & P, y: A & P) -> Int64 -public func test(x: B, y: C) -> Int +public func test(x: B, y: C) -> Int64 // protocol witness for P.foo() in conformance B -sil shared [transparent] [serialized] [thunk] @Pfoo : $@convention(witness_method: P) (@in_guaranteed B) -> Int { +sil shared [transparent] [serialized] [thunk] @Pfoo : $@convention(witness_method: P) (@in_guaranteed B) -> Int64 { // %0 // user: %1 bb0(%0 : $*B): %1 = load %0 : $*B // users: %2, %3 - %2 = class_method %1 : $B, #B.foo!1 : (B) -> () -> Int, $@convention(method) (@guaranteed B) -> Int // user: %3 - %3 = apply %2(%1) : $@convention(method) (@guaranteed B) -> Int // user: %4 - return %3 : $Int // id: %4 + %2 = class_method %1 : $B, #B.foo!1 : (B) -> () -> Int64, $@convention(method) (@guaranteed B) -> Int64 // user: %3 + %3 = apply %2(%1) : $@convention(method) (@guaranteed B) -> Int64 // user: %4 + return %3 : $Int64 // id: %4 } // end sil function 'Pfoo' // B.foo() -sil [noinline] @foo : $@convention(method) (@guaranteed B) -> Int { +sil [noinline] @foo : $@convention(method) (@guaranteed B) -> Int64 { // %0 // user: %1 bb0(%0 : $B): debug_value %0 : $B, let, name "self", argno 1 // id: %1 %2 = integer_literal $Builtin.Int64, 1 // user: %3 - %3 = struct $Int (%2 : $Builtin.Int64) // user: %4 - return %3 : $Int // id: %4 + %3 = struct $Int64 (%2 : $Builtin.Int64) // user: %4 + return %3 : $Int64 // id: %4 } // end sil function 'foo' @@ -56,7 +56,7 @@ bb0(%0 : $B): // based on the arguments passed to it here. Even though this function isn't // directly checked, it is essentail for the optimization. // test(x:y:) -sil @test :$@convention(thin) (@guaranteed B, @guaranteed C) -> Int { +sil @test :$@convention(thin) (@guaranteed B, @guaranteed C) -> Int64 { // %0 // users: %5, %4, %2 // %1 // users: %7, %6, %3 bb0(%0 : $B, %1 : $C): @@ -67,11 +67,11 @@ bb0(%0 : $B, %1 : $C): strong_retain %1 : $C // id: %6 %7 = init_existential_ref %1 : $C : $C, $A & P // users: %10, %9 // function_ref imp(x:y:) - %8 = function_ref @impl : $@convention(thin) (@guaranteed A & P, @guaranteed A & P) -> Int // user: %9 - %9 = apply %8(%5, %7) : $@convention(thin) (@guaranteed A & P, @guaranteed A & P) -> Int // user: %12 + %8 = function_ref @impl : $@convention(thin) (@guaranteed A & P, @guaranteed A & P) -> Int64 // user: %9 + %9 = apply %8(%5, %7) : $@convention(thin) (@guaranteed A & P, @guaranteed A & P) -> Int64 // user: %12 strong_release %7 : $A & P // id: %10 strong_release %5 : $A & P // id: %11 - return %9 : $Int // id: %12 + return %9 : $Int64 // id: %12 } // end sil function 'test' // We're looking of an optimized spcialization of @impl, not @impl itself. @@ -83,7 +83,7 @@ bb0(%0 : $B, %1 : $C): // This function will be passed arguments of type B and C for arguments // %0 and %1 respectively. We want to make sure that we call B's foo method // and not C's foo method. -sil hidden [noinline] @impl : $@convention(thin) (@guaranteed A & P, @guaranteed A & P) -> Int { +sil hidden [noinline] @impl : $@convention(thin) (@guaranteed A & P, @guaranteed A & P) -> Int64 { bb0(%0 : $A & P, %1 : $A & P): %2 = open_existential_ref %0 : $A & P to $@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P %3 = unchecked_ref_cast %1 : $A & P to $@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P @@ -94,28 +94,28 @@ bb0(%0 : $A & P, %1 : $A & P): // We want to make sure that we picked up on the FIRST store and not the second one. // class C's foo method is named "bar" whereas class B's foo method is named "foo". // We want to make sure that we call a function named "foo" not "bar". - // CHECK: [[FN:%[0-9]+]] = function_ref @${{.*}}foo{{.*}} : $@convention(thin) () -> Int - %7 = witness_method $@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P, #P.foo!1 : (Self) -> () -> Int, %2 : $@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> Int + // CHECK: [[FN:%[0-9]+]] = function_ref @${{.*}}foo{{.*}} : $@convention(thin) () -> Int64 + %7 = witness_method $@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P, #P.foo!1 : (Self) -> () -> Int64, %2 : $@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> Int64 // CHECK: apply [[FN]] - %8 = apply %7<@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P>(%5) : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> Int + %8 = apply %7<@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P>(%5) : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> Int64 store %3 to %5 : $*@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P dealloc_stack %5 : $*@opened("A1F1B526-1463-11EA-932F-ACBC329C418B") A & P - return %8 : $Int + return %8 : $Int64 // CHECK-LABEL: end sil function {{.*}}impl{{.*}} } // end sil function 'impl' // C.foo() -sil @bar : $@convention(method) (@guaranteed C) -> Int +sil @bar : $@convention(method) (@guaranteed C) -> Int64 sil_vtable [serialized] B { - #B.foo!1: (B) -> () -> Int : @foo // B.foo() + #B.foo!1: (B) -> () -> Int64 : @foo // B.foo() } sil_vtable [serialized] C { - #C.foo!1: (C) -> () -> Int : @bar + #C.foo!1: (C) -> () -> Int64 : @bar } sil_witness_table [serialized] B: P module run { - method #P.foo!1: (Self) -> () -> Int : @Pfoo // protocol witness for P.foo() in conformance B + method #P.foo!1: (Self) -> () -> Int64 : @Pfoo // protocol witness for P.foo() in conformance B } From 727cd962bc62800e94883a1446e4ad64823a2189 Mon Sep 17 00:00:00 2001 From: zoecarver Date: Mon, 3 Feb 2020 10:12:57 -0800 Subject: [PATCH 31/36] Format --- lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp b/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp index 8612580196e8b..4e82cf92ccad8 100644 --- a/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp +++ b/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp @@ -940,11 +940,12 @@ SILInstruction *SILCombiner::createApplyWithConcreteType( bool madeUpdate = false; for (unsigned index = 0; index < conv.getNumSILArguments(); ++index) { canUpdate &= conv.getSILArgumentType(index) == NewArgs[index]->getType(); - madeUpdate |= NewArgs[index]->getType() != Apply.getArgument(index)->getType(); + madeUpdate |= + NewArgs[index]->getType() != Apply.getArgument(index)->getType(); } return std::make_tuple(canUpdate, madeUpdate); }(); - + if (!canUpdateArgs || !madeUpdate) { // Remove any new instructions created while attempting to optimize this // apply. Since the apply was never rewritten, if they aren't removed here, From dbadd99b62531e844e7d63f0bff7a5af7a893e6f Mon Sep 17 00:00:00 2001 From: zoecarver Date: Mon, 3 Feb 2020 15:20:59 -0800 Subject: [PATCH 32/36] Add benchmark for devirtualization performance measurements --- benchmark/CMakeLists.txt | 1 + .../DevirtualizeProtocolComposition.swift | 65 +++++++++++++++++++ benchmark/utils/main.swift | 2 + 3 files changed, 68 insertions(+) create mode 100644 benchmark/single-source/DevirtualizeProtocolComposition.swift diff --git a/benchmark/CMakeLists.txt b/benchmark/CMakeLists.txt index fbf1709638d3b..58c85675ad2d1 100644 --- a/benchmark/CMakeLists.txt +++ b/benchmark/CMakeLists.txt @@ -62,6 +62,7 @@ set(SWIFT_BENCH_MODULES single-source/Combos single-source/DataBenchmarks single-source/DeadArray + single-source/DevirtualizeProtocolComposition single-source/DictOfArraysToArrayOfDicts single-source/DictTest single-source/DictTest2 diff --git a/benchmark/single-source/DevirtualizeProtocolComposition.swift b/benchmark/single-source/DevirtualizeProtocolComposition.swift new file mode 100644 index 0000000000000..4bb8efdc5ce6a --- /dev/null +++ b/benchmark/single-source/DevirtualizeProtocolComposition.swift @@ -0,0 +1,65 @@ +//===--- DevirtualizeProtocolComposition.swift -------------------------------------------===// +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2014 - 2019 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// + +import TestsUtils + +public let DevirtualizeProtocolComposition = [ + BenchmarkInfo(name: "DevirtualizeProtocolComposition", runFunction: run_DevirtualizeProtocolComposition, tags: [.validation, .api]), +] + +public class ClassA { } + +protocol ProtocolA { + func foo() -> Int +} + +protocol ProtocolB { + func bar() -> Int +} + +public class ClassB: ClassA { + func foo() -> Int { + return 10 + } +} + +extension ClassB: ProtocolA { } + +func quadratic(a: Int) -> Int { + var sum = 0 + for _ in 0..(_ x: ClassA & ProtocolA, count: Int) -> Int { + var sum = 0 + for _ in 0.. Int { + return shouldOptimize1(c, count: 25) +} + +@inline(never) +public func run_DevirtualizeProtocolComposition(N: Int) { + for _ in 0.. Date: Mon, 3 Feb 2020 15:39:06 -0800 Subject: [PATCH 33/36] Update added tests to require objec interop --- test/SILOptimizer/devirtualize_protocol_composition.swift | 2 ++ .../devirtualize_protocol_composition_two_stores.sil | 2 ++ 2 files changed, 4 insertions(+) diff --git a/test/SILOptimizer/devirtualize_protocol_composition.swift b/test/SILOptimizer/devirtualize_protocol_composition.swift index 75d6d7bf5dbb0..9c54736ef2177 100644 --- a/test/SILOptimizer/devirtualize_protocol_composition.swift +++ b/test/SILOptimizer/devirtualize_protocol_composition.swift @@ -1,6 +1,8 @@ // RUN: %target-swift-frontend -parse-as-library -O -wmo -emit-sil %s | %FileCheck %s // RUN: %target-swift-frontend -parse-as-library -Osize -wmo -emit-sil %s | %FileCheck %s +// REQUIRES: objc_interop + // This is an end-to-end test to ensure that the optimizer devertualizes // calls to a protocol composition type. diff --git a/test/SILOptimizer/devirtualize_protocol_composition_two_stores.sil b/test/SILOptimizer/devirtualize_protocol_composition_two_stores.sil index 231a6a1c88e18..cf16ae6c167da 100644 --- a/test/SILOptimizer/devirtualize_protocol_composition_two_stores.sil +++ b/test/SILOptimizer/devirtualize_protocol_composition_two_stores.sil @@ -1,5 +1,7 @@ // RUN: %target-sil-opt -O -wmo -enable-sil-verify-all %s | %FileCheck %s +// REQUIRES: objc_interop + sil_stage canonical import Builtin From ea821b1adb17d04eb8171dab70b2cf2986247f94 Mon Sep 17 00:00:00 2001 From: zoecarver Date: Mon, 3 Feb 2020 16:48:01 -0800 Subject: [PATCH 34/36] Use blackHole to make sure benchmark isn't optimized away --- benchmark/single-source/DevirtualizeProtocolComposition.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/benchmark/single-source/DevirtualizeProtocolComposition.swift b/benchmark/single-source/DevirtualizeProtocolComposition.swift index 4bb8efdc5ce6a..97fc75b3a4011 100644 --- a/benchmark/single-source/DevirtualizeProtocolComposition.swift +++ b/benchmark/single-source/DevirtualizeProtocolComposition.swift @@ -59,7 +59,7 @@ public func entryPoint1(c: ClassB) -> Int { @inline(never) public func run_DevirtualizeProtocolComposition(N: Int) { - for _ in 0.. Date: Tue, 4 Feb 2020 16:27:17 -0800 Subject: [PATCH 35/36] Break argument checking out of a lambda and add comments --- .../SILCombiner/SILCombinerApplyVisitors.cpp | 46 ++++++++++--------- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp b/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp index 2a389da526441..c76a6c3f43073 100644 --- a/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp +++ b/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp @@ -924,28 +924,32 @@ SILInstruction *SILCombiner::createApplyWithConcreteType( }); } - bool canUpdateArgs, madeUpdate; - std::tie(canUpdateArgs, madeUpdate) = [&]() { - auto substTy = - Apply.getCallee() - ->getType() - .substGenericArgs(Apply.getModule(), NewCallSubs, - Apply.getFunction()->getTypeExpansionContext()) - .getAs(); - SILFunctionConventions conv(substTy, - SILModuleConventions(Apply.getModule())); - bool canUpdate = true; - // Keep track of weather we made any updates at all. Otherwise, we will - // have an infinite loop. - bool madeUpdate = false; - for (unsigned index = 0; index < conv.getNumSILArguments(); ++index) { - canUpdate &= conv.getSILArgumentType(index) == NewArgs[index]->getType(); - madeUpdate |= - NewArgs[index]->getType() != Apply.getArgument(index)->getType(); - } - return std::make_tuple(canUpdate, madeUpdate); - }(); + // We need to make sure that we can a) update Apply to use the new args and b) + // at least one argument has changed. If no arguments have changed, we need + // to return nullptr. Otherwise, we will have an infinite loop. + auto substTy = + Apply.getCallee() + ->getType() + .substGenericArgs(Apply.getModule(), NewCallSubs, + Apply.getFunction()->getTypeExpansionContext()) + .getAs(); + SILFunctionConventions conv(substTy, + SILModuleConventions(Apply.getModule())); + bool canUpdateArgs = true; + bool madeUpdate = false; + for (unsigned index = 0; index < conv.getNumSILArguments(); ++index) { + // Make sure that *all* the arguments in both the new substitution function + // and our vector of new arguments have the same type. + canUpdateArgs &= + conv.getSILArgumentType(index) == NewArgs[index]->getType(); + // Make sure that we have changed at least one argument. + madeUpdate |= + NewArgs[index]->getType() != Apply.getArgument(index)->getType(); + } + // If we can't update the args (because of a type mismatch) or the args don't + // change, bail out by removing the instructions we've added and returning + // nullptr. if (!canUpdateArgs || !madeUpdate) { // Remove any new instructions created while attempting to optimize this // apply. Since the apply was never rewritten, if they aren't removed here, From 464b585c9cfb79b0249e8add9607859faa06ec45 Mon Sep 17 00:00:00 2001 From: zoecarver Date: Thu, 6 Feb 2020 14:14:51 -0800 Subject: [PATCH 36/36] Update benchmark to be more similar to other devirtualization benchmarks --- .../DevirtualizeProtocolComposition.swift | 42 +++++-------------- 1 file changed, 11 insertions(+), 31 deletions(-) diff --git a/benchmark/single-source/DevirtualizeProtocolComposition.swift b/benchmark/single-source/DevirtualizeProtocolComposition.swift index 97fc75b3a4011..d90bbd1da71e6 100644 --- a/benchmark/single-source/DevirtualizeProtocolComposition.swift +++ b/benchmark/single-source/DevirtualizeProtocolComposition.swift @@ -16,50 +16,30 @@ public let DevirtualizeProtocolComposition = [ BenchmarkInfo(name: "DevirtualizeProtocolComposition", runFunction: run_DevirtualizeProtocolComposition, tags: [.validation, .api]), ] -public class ClassA { } +protocol Pingable { func ping() -> Int; func pong() -> Int} -protocol ProtocolA { - func foo() -> Int +public class Game { + func length() -> Int { return 10 } } -protocol ProtocolB { - func bar() -> Int -} +public class PingPong: Game { } -public class ClassB: ClassA { - func foo() -> Int { - return 10 - } +extension PingPong : Pingable { + func ping() -> Int { return 1 } + func pong() -> Int { return 2 } } -extension ClassB: ProtocolA { } - -func quadratic(a: Int) -> Int { +func playGame(_ x: Game & Pingable) -> Int { var sum = 0 - for _ in 0..(_ x: ClassA & ProtocolA, count: Int) -> Int { - var sum = 0 - for _ in 0.. Int { - return shouldOptimize1(c, count: 25) -} - @inline(never) public func run_DevirtualizeProtocolComposition(N: Int) { for _ in 0..