From a67a4e1a38109f7679b81fbb3dc80904bdf081cc Mon Sep 17 00:00:00 2001 From: Erik Eckstein Date: Sat, 18 Feb 2017 18:03:46 -0800 Subject: [PATCH] Don't de-virtualize in SILCombine. It is important to de-serialize the devirtualized function (and its callees), especially because we must make sure that all transparent functions are de-serialized. SILCombine did not do that. But as we have the same optimization in the Devirtualizer, it's not needed to duplicate the code in SILCombine. The only reason we had this peephole in SILCombine is that the Devirtualizer pass could not handle partial applies. So with this change the Devirtualizer can now also handle partial applies. Fixes rdar://problem/30544344 (again, after my first attempt failed) --- .../swift/SILOptimizer/Utils/Devirtualize.h | 3 +- lib/SILOptimizer/SILCombiner/SILCombiner.h | 1 - .../SILCombiner/SILCombinerMiscVisitors.cpp | 27 -------- lib/SILOptimizer/Transforms/Devirtualizer.cpp | 15 ++-- lib/SILOptimizer/Utils/Devirtualize.cpp | 29 ++++---- .../devirt_default_witness_method.sil | 2 +- .../devirt_static_witness_method.sil | 2 +- .../existential_type_propagation.sil | 69 ++++++++++++++++++- test/SILOptimizer/sil_combine.sil | 66 ------------------ 9 files changed, 96 insertions(+), 118 deletions(-) diff --git a/include/swift/SILOptimizer/Utils/Devirtualize.h b/include/swift/SILOptimizer/Utils/Devirtualize.h index edcfbc13214..dfcfe5eabea 100644 --- a/include/swift/SILOptimizer/Utils/Devirtualize.h +++ b/include/swift/SILOptimizer/Utils/Devirtualize.h @@ -43,8 +43,7 @@ namespace swift { /// casted to produce a properly typed value (first element). typedef std::pair DevirtualizationResult; -DevirtualizationResult tryDevirtualizeApply(FullApplySite AI); -DevirtualizationResult tryDevirtualizeApply(FullApplySite AI, +DevirtualizationResult tryDevirtualizeApply(ApplySite AI, ClassHierarchyAnalysis *CHA); bool canDevirtualizeApply(FullApplySite AI, ClassHierarchyAnalysis *CHA); bool isNominalTypeWithUnboundGenericParameters(SILType Ty, SILModule &M); diff --git a/lib/SILOptimizer/SILCombiner/SILCombiner.h b/lib/SILOptimizer/SILCombiner/SILCombiner.h index 114507541d0..81ff67262f1 100644 --- a/lib/SILOptimizer/SILCombiner/SILCombiner.h +++ b/lib/SILOptimizer/SILCombiner/SILCombiner.h @@ -226,7 +226,6 @@ public: SILInstruction *visitAllocRefDynamicInst(AllocRefDynamicInst *ARDI); SILInstruction *visitEnumInst(EnumInst *EI); SILInstruction *visitConvertFunctionInst(ConvertFunctionInst *CFI); - SILInstruction *visitWitnessMethodInst(WitnessMethodInst *WMI); /// Instruction visitor helpers. SILInstruction *optimizeBuiltinCanBeObjCClass(BuiltinInst *AI); diff --git a/lib/SILOptimizer/SILCombiner/SILCombinerMiscVisitors.cpp b/lib/SILOptimizer/SILCombiner/SILCombinerMiscVisitors.cpp index 6329a6b5fea..a20c9e891c0 100644 --- a/lib/SILOptimizer/SILCombiner/SILCombinerMiscVisitors.cpp +++ b/lib/SILOptimizer/SILCombiner/SILCombinerMiscVisitors.cpp @@ -1304,30 +1304,3 @@ SILInstruction *SILCombiner::visitEnumInst(EnumInst *EI) { return nullptr; } -SILInstruction *SILCombiner::visitWitnessMethodInst(WitnessMethodInst *WMI) { - // Try to devirtualize a witness_method if it is statically possible. - // Many cases are handled by the inliner/devirtualizer, but certain - // special cases are not covered there, e.g. partial_apply(witness_method) - SILFunction *F; - SILWitnessTable *WT; - - std::tie(F, WT) = - WMI->getModule().lookUpFunctionInWitnessTable(WMI->getConformance(), - WMI->getMember()); - - if (!F) - return nullptr; - - for (auto U = WMI->use_begin(), E = WMI->use_end(); U != E;) { - auto User = U->getUser(); - ++U; - if (auto AI = ApplySite::isa(User)) { - auto Result = tryDevirtualizeWitnessMethod(AI); - if (Result.first) { - User->replaceAllUsesWith(Result.first); - eraseInstFromFunction(*User); - } - } - } - return nullptr; -} diff --git a/lib/SILOptimizer/Transforms/Devirtualizer.cpp b/lib/SILOptimizer/Transforms/Devirtualizer.cpp index a652f5c8317..8dd42ba37a0 100644 --- a/lib/SILOptimizer/Transforms/Devirtualizer.cpp +++ b/lib/SILOptimizer/Transforms/Devirtualizer.cpp @@ -55,14 +55,14 @@ bool Devirtualizer::devirtualizeAppliesInFunction(SILFunction &F, llvm::SmallVector DeadApplies; llvm::SmallVector NewApplies; - SmallVector Applies; + SmallVector Applies; for (auto &BB : F) { for (auto It = BB.begin(), End = BB.end(); It != End;) { auto &I = *It++; // Skip non-apply instructions. - auto Apply = FullApplySite::isa(&I); + auto Apply = ApplySite::isa(&I); if (!Apply) continue; Applies.push_back(Apply); @@ -102,11 +102,12 @@ bool Devirtualizer::devirtualizeAppliesInFunction(SILFunction &F, auto *CalleeFn = Apply.getReferencedFunction(); assert(CalleeFn && "Expected devirtualized callee!"); - // FIXME: Until we link everything in up front we need to ensure - // that we link after devirtualizing in order to pull in - // everything we reference from the stdlib. After we do that we - // can move the notification code below back into the main loop - // above. + // We need to ensure that we link after devirtualizing in order to pull in + // everything we reference from another module. This is especially important + // for transparent functions, because if transparent functions are not + // inlined for some reason, we need to generate code for them. + // Note that functions, which are only referenced from witness/vtables, are + // not linked upfront by the SILLinker. if (!CalleeFn->isDefinition()) F.getModule().linkFunction(CalleeFn, SILModule::LinkingMode::LinkAll); diff --git a/lib/SILOptimizer/Utils/Devirtualize.cpp b/lib/SILOptimizer/Utils/Devirtualize.cpp index e8fb220ee3b..4fa10940045 100644 --- a/lib/SILOptimizer/Utils/Devirtualize.cpp +++ b/lib/SILOptimizer/Utils/Devirtualize.cpp @@ -1080,7 +1080,7 @@ DevirtualizationResult swift::tryDevirtualizeWitnessMethod(ApplySite AI) { /// Attempt to devirtualize the given apply if possible, and return a /// new instruction in that case, or nullptr otherwise. DevirtualizationResult -swift::tryDevirtualizeApply(FullApplySite AI, ClassHierarchyAnalysis *CHA) { +swift::tryDevirtualizeApply(ApplySite AI, ClassHierarchyAnalysis *CHA) { DEBUG(llvm::dbgs() << " Trying to devirtualize: " << *AI.getInstruction()); // Devirtualize apply instructions that call witness_method instructions: @@ -1091,6 +1091,11 @@ swift::tryDevirtualizeApply(FullApplySite AI, ClassHierarchyAnalysis *CHA) { if (isa(AI.getCallee())) return tryDevirtualizeWitnessMethod(AI); + // TODO: check if we can also de-virtualize partial applies of class methods. + FullApplySite FAS = FullApplySite::isa(AI.getInstruction()); + if (!FAS) + return std::make_pair(nullptr, ApplySite()); + /// Optimize a class_method and alloc_ref pair into a direct function /// reference: /// @@ -1107,8 +1112,8 @@ swift::tryDevirtualizeApply(FullApplySite AI, ClassHierarchyAnalysis *CHA) { /// into /// /// %YY = function_ref @... - if (auto *CMI = dyn_cast(AI.getCallee())) { - auto &M = AI.getModule(); + if (auto *CMI = dyn_cast(FAS.getCallee())) { + auto &M = FAS.getModule(); auto Instance = stripUpCasts(CMI->getOperand()); auto ClassType = Instance->getType(); if (ClassType.is()) @@ -1116,34 +1121,34 @@ swift::tryDevirtualizeApply(FullApplySite AI, ClassHierarchyAnalysis *CHA) { auto *CD = ClassType.getClassOrBoundGenericClass(); - if (isEffectivelyFinalMethod(AI, ClassType, CD, CHA)) - return tryDevirtualizeClassMethod(AI, Instance); + if (isEffectivelyFinalMethod(FAS, ClassType, CD, CHA)) + return tryDevirtualizeClassMethod(FAS, Instance); // Try to check if the exact dynamic type of the instance is statically // known. if (auto Instance = getInstanceWithExactDynamicType(CMI->getOperand(), CMI->getModule(), CHA)) - return tryDevirtualizeClassMethod(AI, Instance); + return tryDevirtualizeClassMethod(FAS, Instance); if (auto ExactTy = getExactDynamicType(CMI->getOperand(), CMI->getModule(), CHA)) { if (ExactTy == CMI->getOperand()->getType()) - return tryDevirtualizeClassMethod(AI, CMI->getOperand()); + return tryDevirtualizeClassMethod(FAS, CMI->getOperand()); } } - if (isa(AI.getCallee())) { - if (AI.hasSelfArgument()) { - return tryDevirtualizeClassMethod(AI, AI.getSelfArgument()); + if (isa(FAS.getCallee())) { + if (FAS.hasSelfArgument()) { + return tryDevirtualizeClassMethod(FAS, FAS.getSelfArgument()); } // It is an invocation of a class method. // Last operand is the metatype that should be used for dispatching. - return tryDevirtualizeClassMethod(AI, AI.getArguments().back()); + return tryDevirtualizeClassMethod(FAS, FAS.getArguments().back()); } - return std::make_pair(nullptr, FullApplySite()); + return std::make_pair(nullptr, ApplySite()); } bool swift::canDevirtualizeApply(FullApplySite AI, ClassHierarchyAnalysis *CHA) { diff --git a/test/SILOptimizer/devirt_default_witness_method.sil b/test/SILOptimizer/devirt_default_witness_method.sil index b5fc06cf1f4..d4ce7669bbe 100644 --- a/test/SILOptimizer/devirt_default_witness_method.sil +++ b/test/SILOptimizer/devirt_default_witness_method.sil @@ -1,4 +1,4 @@ -// RUN: %target-sil-opt -assume-parsing-unqualified-ownership-sil -enable-sil-verify-all %s -sil-combine -enable-resilience | %FileCheck %s +// RUN: %target-sil-opt -assume-parsing-unqualified-ownership-sil -enable-sil-verify-all %s -devirtualizer -sil-combine -enable-resilience | %FileCheck %s sil_stage canonical import Builtin diff --git a/test/SILOptimizer/devirt_static_witness_method.sil b/test/SILOptimizer/devirt_static_witness_method.sil index 45b16d1f08b..47e9aa10d6f 100644 --- a/test/SILOptimizer/devirt_static_witness_method.sil +++ b/test/SILOptimizer/devirt_static_witness_method.sil @@ -1,4 +1,4 @@ -// RUN: %target-sil-opt -assume-parsing-unqualified-ownership-sil -enable-sil-verify-all %s -sil-combine | %FileCheck %s +// RUN: %target-sil-opt -assume-parsing-unqualified-ownership-sil -enable-sil-verify-all %s -devirtualizer -sil-combine | %FileCheck %s sil_stage canonical import Builtin diff --git a/test/SILOptimizer/existential_type_propagation.sil b/test/SILOptimizer/existential_type_propagation.sil index 5554b044cd0..814ae81aae9 100644 --- a/test/SILOptimizer/existential_type_propagation.sil +++ b/test/SILOptimizer/existential_type_propagation.sil @@ -1,4 +1,4 @@ -// RUN: %target-sil-opt -assume-parsing-unqualified-ownership-sil -enable-sil-verify-all %s -sil-combine -inline -sil-combine | %FileCheck %s +// RUN: %target-sil-opt -assume-parsing-unqualified-ownership-sil -enable-sil-verify-all %s -sil-combine -devirtualizer -inline -sil-combine | %FileCheck %s // Check that type propagation is performed correctly for existentials. // The concrete type set in init_existential instructions should be propagated @@ -235,3 +235,70 @@ sil_witness_table public_external ArrayClassReaderWriter: ReaderWriterType modul method #ReaderWriterType.read!1: @_TTWC28existential_type_propagation22ArrayClassReaderWriterS_16ReaderWriterTypeS_FS1_4readuRq_S1__fq_FSiSi method #ReaderWriterType.write!1: @_TTWC28existential_type_propagation22ArrayClassReaderWriterS_16ReaderWriterTypeS_FS1_5writeuRq_S1__fRq_FTSi5valueSi_T_ } + +protocol PPP : class { + func foo() +} + +class BBB: PPP { + @inline(never) + func foo() +} + +final class XXX : BBB { + @inline(never) + override func foo() +} + +// Check that sil-combine does not crash on this example and does not generate a wrong +// upcast. +// CHECK-LABEL: sil @silcombine_dont_generate_wrong_upcasts_during_devirt +// CHECK-NOT: upcast +// CHECK: witness_method $XXX, #PPP.foo!1 : {{.*}} : $@convention(witness_method) <τ_0_0 where τ_0_0 : PPP> (@guaranteed τ_0_0) -> () +// CHECK-NOT: upcast +// CHECK: return +sil @silcombine_dont_generate_wrong_upcasts_during_devirt: $@convention(thin) (@owned BBB) -> () { +bb0(%0 : $BBB): + strong_retain %0 : $BBB + %3 = init_existential_ref %0 : $BBB : $BBB, $PPP + %5 = open_existential_ref %3 : $PPP to $@opened("0AC9A62E-926E-11E6-8BF5-685B35C48C83") PPP + %6 = witness_method $XXX, #PPP.foo!1, %5 : $@opened("0AC9A62E-926E-11E6-8BF5-685B35C48C83") PPP : $@convention(witness_method) <τ_0_0 where τ_0_0 : PPP> (@guaranteed τ_0_0) -> () + %7 = apply %6<@opened("0AC9A62E-926E-11E6-8BF5-685B35C48C83") PPP>(%5) : $@convention(witness_method) <τ_0_0 where τ_0_0 : PPP> (@guaranteed τ_0_0) -> () + %8 = apply %6<@opened("0AC9A62E-926E-11E6-8BF5-685B35C48C83") PPP>(%5) : $@convention(witness_method) <τ_0_0 where τ_0_0 : PPP> (@guaranteed τ_0_0) -> () + strong_release %3 : $PPP + %9 = tuple () + strong_release %0 : $BBB + %11 = tuple () + return %11 : $() +} + +// Check that both applies can be devirtualized by means of propagating the concrete +// type of the existential into witness_method and apply instructions. +// CHECK-LABEL: sil @silcombine_devirt_both_applies_of_witness_method +// CHECK-NOT: open_existential_ref +// CHECK-NOT: witness_method +// CHECK: [[FR1:%.*]] = function_ref @_TTWC4nix23XXXS_3PPPS_FS1_3foofT_T_ +// CHECK: apply [[FR1]](%0) +// CHECK: [[FR2:%.*]] = function_ref @_TTWC4nix23XXXS_3PPPS_FS1_3foofT_T_ +// CHECK: apply [[FR2]](%0) +// CHECK: return +sil @silcombine_devirt_both_applies_of_witness_method : $@convention(thin) (@owned XXX) -> () { +bb0(%0 : $XXX): + strong_retain %0 : $XXX + %3 = init_existential_ref %0 : $XXX : $XXX, $PPP + %5 = open_existential_ref %3 : $PPP to $@opened("0AC9A62E-926E-11E6-8BF5-685B35C48C83") PPP + %6 = witness_method $@opened("0AC9A62E-926E-11E6-8BF5-685B35C48C83") PPP, #PPP.foo!1, %5 : $@opened("0AC9A62E-926E-11E6-8BF5-685B35C48C83") PPP : $@convention(witness_method) <τ_0_0 where τ_0_0 : PPP> (@guaranteed τ_0_0) -> () + %7 = apply %6<@opened("0AC9A62E-926E-11E6-8BF5-685B35C48C83") PPP>(%5) : $@convention(witness_method) <τ_0_0 where τ_0_0 : PPP> (@guaranteed τ_0_0) -> () + %8 = apply %6<@opened("0AC9A62E-926E-11E6-8BF5-685B35C48C83") PPP>(%5) : $@convention(witness_method) <τ_0_0 where τ_0_0 : PPP> (@guaranteed τ_0_0) -> () + strong_release %3 : $PPP + %9 = tuple () + strong_release %0 : $XXX + %11 = tuple () + return %11 : $() +} + +sil @_TTWC4nix23XXXS_3PPPS_FS1_3foofT_T_ : $@convention(witness_method) (@guaranteed XXX) -> () + +sil_witness_table XXX: PPP module nix2 { + method #PPP.foo!1: @_TTWC4nix23XXXS_3PPPS_FS1_3foofT_T_ // protocol witness for PPP.foo() -> () in conformance XXX +} diff --git a/test/SILOptimizer/sil_combine.sil b/test/SILOptimizer/sil_combine.sil index 2113a743401..511a6b2e7da 100644 --- a/test/SILOptimizer/sil_combine.sil +++ b/test/SILOptimizer/sil_combine.sil @@ -3137,69 +3137,3 @@ bb0(%0 : $VV): return %26 : $() } -protocol PPP : class { - func foo() -} - -class BBB: PPP { - @inline(never) - func foo() -} - -final class XXX : BBB { - @inline(never) - override func foo() -} - -// Check that sil-combine does not crash on this example and does not generate a wrong -// upcast. -// CHECK-LABEL: sil @silcombine_dont_generate_wrong_upcasts_during_devirt -// CHECK-NOT: upcast -// CHECK: witness_method $XXX, #PPP.foo!1 : {{.*}} : $@convention(witness_method) <τ_0_0 where τ_0_0 : PPP> (@guaranteed τ_0_0) -> () -// CHECK-NOT: upcast -// CHECK: return -sil @silcombine_dont_generate_wrong_upcasts_during_devirt: $@convention(thin) (@owned BBB) -> () { -bb0(%0 : $BBB): - strong_retain %0 : $BBB - %3 = init_existential_ref %0 : $BBB : $BBB, $PPP - %5 = open_existential_ref %3 : $PPP to $@opened("0AC9A62E-926E-11E6-8BF5-685B35C48C83") PPP - %6 = witness_method $XXX, #PPP.foo!1, %5 : $@opened("0AC9A62E-926E-11E6-8BF5-685B35C48C83") PPP : $@convention(witness_method) <τ_0_0 where τ_0_0 : PPP> (@guaranteed τ_0_0) -> () - %7 = apply %6<@opened("0AC9A62E-926E-11E6-8BF5-685B35C48C83") PPP>(%5) : $@convention(witness_method) <τ_0_0 where τ_0_0 : PPP> (@guaranteed τ_0_0) -> () - %8 = apply %6<@opened("0AC9A62E-926E-11E6-8BF5-685B35C48C83") PPP>(%5) : $@convention(witness_method) <τ_0_0 where τ_0_0 : PPP> (@guaranteed τ_0_0) -> () - strong_release %3 : $PPP - %9 = tuple () - strong_release %0 : $BBB - %11 = tuple () - return %11 : $() -} - -// Check that both applies can be devirtualized by means of propagating the concrete -// type of the existential into witness_method and apply instructions. -// CHECK-LABEL: sil @silcombine_devirt_both_applies_of_witness_method -// CHECK-NOT: open_existential_ref -// CHECK-NOT: witness_method -// CHECK: [[FR1:%.*]] = function_ref @_TTWC4nix23XXXS_3PPPS_FS1_3foofT_T_ -// CHECK: apply [[FR1]](%0) -// CHECK: [[FR2:%.*]] = function_ref @_TTWC4nix23XXXS_3PPPS_FS1_3foofT_T_ -// CHECK: apply [[FR2]](%0) -// CHECK: return -sil @silcombine_devirt_both_applies_of_witness_method : $@convention(thin) (@owned XXX) -> () { -bb0(%0 : $XXX): - strong_retain %0 : $XXX - %3 = init_existential_ref %0 : $XXX : $XXX, $PPP - %5 = open_existential_ref %3 : $PPP to $@opened("0AC9A62E-926E-11E6-8BF5-685B35C48C83") PPP - %6 = witness_method $@opened("0AC9A62E-926E-11E6-8BF5-685B35C48C83") PPP, #PPP.foo!1, %5 : $@opened("0AC9A62E-926E-11E6-8BF5-685B35C48C83") PPP : $@convention(witness_method) <τ_0_0 where τ_0_0 : PPP> (@guaranteed τ_0_0) -> () - %7 = apply %6<@opened("0AC9A62E-926E-11E6-8BF5-685B35C48C83") PPP>(%5) : $@convention(witness_method) <τ_0_0 where τ_0_0 : PPP> (@guaranteed τ_0_0) -> () - %8 = apply %6<@opened("0AC9A62E-926E-11E6-8BF5-685B35C48C83") PPP>(%5) : $@convention(witness_method) <τ_0_0 where τ_0_0 : PPP> (@guaranteed τ_0_0) -> () - strong_release %3 : $PPP - %9 = tuple () - strong_release %0 : $XXX - %11 = tuple () - return %11 : $() -} - -sil @_TTWC4nix23XXXS_3PPPS_FS1_3foofT_T_ : $@convention(witness_method) (@guaranteed XXX) -> () - -sil_witness_table XXX: PPP module nix2 { - method #PPP.foo!1: @_TTWC4nix23XXXS_3PPPS_FS1_3foofT_T_ // protocol witness for PPP.foo() -> () in conformance XXX -}