diff --git a/lib/SILOptimizer/LoopTransforms/LICM.cpp b/lib/SILOptimizer/LoopTransforms/LICM.cpp index efd423ced97f1..5c2e306db8b69 100644 --- a/lib/SILOptimizer/LoopTransforms/LICM.cpp +++ b/lib/SILOptimizer/LoopTransforms/LICM.cpp @@ -146,6 +146,57 @@ static bool mayWriteTo(AliasAnalysis *AA, SideEffectAnalysis *SEA, return false; } +/// Returns true if \p sideEffectInst cannot be reordered with a call to a +/// global initialier. +static bool mayConflictWithGlobalInit(AliasAnalysis *AA, + SILInstruction *sideEffectInst, ApplyInst *globalInitCall) { + if (auto *SI = dyn_cast(sideEffectInst)) { + return AA->mayReadOrWriteMemory(globalInitCall, SI->getDest()); + } + if (auto *LI = dyn_cast(sideEffectInst)) { + return AA->mayWriteToMemory(globalInitCall, LI->getOperand()); + } + return true; +} + +/// Returns true if any of the instructions in \p sideEffectInsts which are +/// post-dominated by a call to a global initialier cannot be reordered with +/// the call. +static bool mayConflictWithGlobalInit(AliasAnalysis *AA, + InstSet &sideEffectInsts, + ApplyInst *globalInitCall, + SILBasicBlock *preHeader, PostDominanceInfo *PD) { + if (!PD->dominates(globalInitCall->getParent(), preHeader)) + return true; + + SILBasicBlock *globalInitBlock = globalInitCall->getParent(); + for (auto *seInst : sideEffectInsts) { + // Only check instructions in blocks which are "before" (i.e. post-dominated + // by) the block which contains the init-call. + // Instructions which are before the call in the same block have already + // been checked. + if (PD->properlyDominates(globalInitBlock, seInst->getParent())) { + if (mayConflictWithGlobalInit(AA, seInst, globalInitCall)) + return true; + } + } + return false; +} + +/// Returns true if any of the instructions in \p sideEffectInsts cannot be +/// reordered with a call to a global initialier (which is in the same basic +/// block). +static bool mayConflictWithGlobalInit(AliasAnalysis *AA, + ArrayRef sideEffectInsts, + ApplyInst *globalInitCall) { + for (auto *seInst : sideEffectInsts) { + assert(seInst->getParent() == globalInitCall->getParent()); + if (mayConflictWithGlobalInit(AA, seInst, globalInitCall)) + return true; + } + return false; +} + // When Hoisting / Sinking, // Don't descend into control-dependent code. // Only traverse into basic blocks that dominate all exits. @@ -409,6 +460,8 @@ class LoopTreeOptimization { AliasAnalysis *AA; SideEffectAnalysis *SEA; DominanceInfo *DomTree; + PostDominanceAnalysis *PDA; + PostDominanceInfo *postDomTree = nullptr; AccessedStorageAnalysis *ASA; bool Changed; @@ -435,10 +488,11 @@ class LoopTreeOptimization { public: LoopTreeOptimization(SILLoop *TopLevelLoop, SILLoopInfo *LI, AliasAnalysis *AA, SideEffectAnalysis *SEA, - DominanceInfo *DT, AccessedStorageAnalysis *ASA, + DominanceInfo *DT, PostDominanceAnalysis *PDA, + AccessedStorageAnalysis *ASA, bool RunsOnHighLevelSil) - : LoopInfo(LI), AA(AA), SEA(SEA), DomTree(DT), ASA(ASA), Changed(false), - RunsOnHighLevelSIL(RunsOnHighLevelSil) { + : LoopInfo(LI), AA(AA), SEA(SEA), DomTree(DT), PDA(PDA), ASA(ASA), + Changed(false), RunsOnHighLevelSIL(RunsOnHighLevelSil) { // Collect loops for a recursive bottom-up traversal in the loop tree. BotUpWorkList.push_back(TopLevelLoop); for (unsigned i = 0; i < BotUpWorkList.size(); ++i) { @@ -556,9 +610,11 @@ static bool isSafeReadOnlyApply(SideEffectAnalysis *SEA, ApplyInst *AI) { } static void checkSideEffects(swift::SILInstruction &Inst, - InstSet &SideEffectInsts) { + InstSet &SideEffectInsts, + SmallVectorImpl &sideEffectsInBlock) { if (Inst.mayHaveSideEffects()) { SideEffectInsts.insert(&Inst); + sideEffectsInBlock.push_back(&Inst); } } @@ -708,6 +764,7 @@ void LoopTreeOptimization::analyzeCurrentLoop( // Interesting instructions in the loop: SmallVector ReadOnlyApplies; + SmallVector globalInitCalls; SmallVector Loads; SmallVector Stores; SmallVector FixLifetimes; @@ -715,6 +772,7 @@ void LoopTreeOptimization::analyzeCurrentLoop( SmallVector fullApplies; for (auto *BB : Loop->getBlocks()) { + SmallVector sideEffectsInBlock; for (auto &Inst : *BB) { switch (Inst.getKind()) { case SILInstructionKind::FixLifetimeInst: { @@ -731,12 +789,12 @@ void LoopTreeOptimization::analyzeCurrentLoop( case SILInstructionKind::StoreInst: { Stores.push_back(cast(&Inst)); LoadsAndStores.push_back(&Inst); - checkSideEffects(Inst, sideEffects); + checkSideEffects(Inst, sideEffects, sideEffectsInBlock); break; } case SILInstructionKind::BeginAccessInst: BeginAccesses.push_back(cast(&Inst)); - checkSideEffects(Inst, sideEffects); + checkSideEffects(Inst, sideEffects, sideEffectsInBlock); break; case SILInstructionKind::RefElementAddrInst: SpecialHoist.push_back(cast(&Inst)); @@ -747,12 +805,21 @@ void LoopTreeOptimization::analyzeCurrentLoop( // cond_fail that would have protected (executed before) a memory access // must - after hoisting - also be executed before said access. HoistUp.insert(&Inst); - checkSideEffects(Inst, sideEffects); + checkSideEffects(Inst, sideEffects, sideEffectsInBlock); break; case SILInstructionKind::ApplyInst: { auto *AI = cast(&Inst); if (isSafeReadOnlyApply(SEA, AI)) { ReadOnlyApplies.push_back(AI); + } else if (SILFunction *callee = AI->getReferencedFunctionOrNull()) { + // Calls to global inits are different because we don't care about + // side effects which are "after" the call in the loop. + if (callee->isGlobalInit() && + // Check against side-effects within the same block. + // Side-effects in other blocks are checked later (after we + // scanned all blocks of the loop). + !mayConflictWithGlobalInit(AA, sideEffectsInBlock, AI)) + globalInitCalls.push_back(AI); } // check for array semantics and side effects - same as default LLVM_FALLTHROUGH; @@ -761,7 +828,7 @@ void LoopTreeOptimization::analyzeCurrentLoop( if (auto fullApply = FullApplySite::isa(&Inst)) { fullApplies.push_back(fullApply); } - checkSideEffects(Inst, sideEffects); + checkSideEffects(Inst, sideEffects, sideEffectsInBlock); if (canHoistUpDefault(&Inst, Loop, DomTree, RunsOnHighLevelSIL)) { HoistUp.insert(&Inst); } @@ -780,6 +847,23 @@ void LoopTreeOptimization::analyzeCurrentLoop( HoistUp.insert(LI); } } + + if (!globalInitCalls.empty()) { + if (!postDomTree) { + postDomTree = PDA->get(Preheader->getParent()); + } + if (postDomTree->getRootNode()) { + for (ApplyInst *ginitCall : globalInitCalls) { + // Check against side effects which are "before" (i.e. post-dominated + // by) the global initializer call. + if (!mayConflictWithGlobalInit(AA, sideEffects, ginitCall, Preheader, + postDomTree)) { + HoistUp.insert(ginitCall); + } + } + } + } + // Collect memory locations for which we can move all loads and stores out // of the loop. for (StoreInst *SI : Stores) { @@ -1041,6 +1125,7 @@ class LICM : public SILFunctionTransform { } DominanceAnalysis *DA = PM->getAnalysis(); + PostDominanceAnalysis *PDA = PM->getAnalysis(); AliasAnalysis *AA = PM->getAnalysis(); SideEffectAnalysis *SEA = PM->getAnalysis(); AccessedStorageAnalysis *ASA = getAnalysis(); @@ -1051,8 +1136,8 @@ class LICM : public SILFunctionTransform { for (auto *TopLevelLoop : *LoopInfo) { if (!DomTree) DomTree = DA->get(F); - LoopTreeOptimization Opt(TopLevelLoop, LoopInfo, AA, SEA, DomTree, ASA, - RunsOnHighLevelSil); + LoopTreeOptimization Opt(TopLevelLoop, LoopInfo, AA, SEA, DomTree, PDA, + ASA, RunsOnHighLevelSil); Changed |= Opt.optimize(); } diff --git a/test/SILOptimizer/global_hoisting_crash.swift b/test/SILOptimizer/global_hoisting_crash.swift new file mode 100644 index 0000000000000..b640f656e9bbb --- /dev/null +++ b/test/SILOptimizer/global_hoisting_crash.swift @@ -0,0 +1,24 @@ +// RUN: %empty-directory(%t) +// RUN: %target-build-swift -O %s -o %t/a.out +// RUN: %target-run %t/a.out | %FileCheck %s + +// REQUIRES: executable_test + +struct Teststruct { + static let s = Teststruct() + + @inline(never) + init() { + let set = Set() + for _ in set { + // Check that the global initializer is not hoisted out of this loop, + // resulting in a dispatch_once re-retrance crash. + _ = Teststruct.s + } + } +} + +// CHECK: Teststruct +print(Teststruct.s) + + diff --git a/test/SILOptimizer/global_init_opt.swift b/test/SILOptimizer/global_init_opt.swift index d32756d11a588..d6e127b84bf75 100644 --- a/test/SILOptimizer/global_init_opt.swift +++ b/test/SILOptimizer/global_init_opt.swift @@ -16,3 +16,18 @@ var gg: Int = { public func cse() -> Int { return gg + gg } + +// CHECK-LABEL: sil @$s4test4licmSiyF +// CHECK: bb0: +// CHECK: builtin "once" +// CHECK: bb1: +// CHECK-NOT: builtin "once" +// CHECK: } // end sil function '$s4test4licmSiyF' +public func licm() -> Int { + var s = 0 + for _ in 0..<100 { + s += gg + } + return s +} + diff --git a/test/SILOptimizer/licm_apply.sil b/test/SILOptimizer/licm_apply.sil index 0f6a3ae31b78a..a7b95254f4090 100644 --- a/test/SILOptimizer/licm_apply.sil +++ b/test/SILOptimizer/licm_apply.sil @@ -5,6 +5,8 @@ sil_stage canonical import Builtin import Swift +sil @unknown : $@convention(thin) () -> () + sil @read_from_param : $@convention(thin) (@inout Int64, Int64) -> Int64 { bb0(%0 : $*Int64, %1 : $Int64): debug_value %1 : $Int64 @@ -119,3 +121,190 @@ bb2: return %15 : $() } +sil [global_init] @$s4test10testGlobalSivau : $@convention(thin) () -> Builtin.RawPointer + +// CHECK-LABEL: sil @licm_global_init +// CHECK: apply +// CHECK: bb1: +// CHECK-NOT: apply +// CHECK: } // end sil function 'licm_global_init' +sil @licm_global_init : $@convention(thin) () -> () { +bb0: + br bb1 + +bb1: + %2 = function_ref @$s4test10testGlobalSivau : $@convention(thin) () -> Builtin.RawPointer + %3 = apply %2() : $@convention(thin) () -> Builtin.RawPointer + cond_br undef, bb1, bb2 + +bb2: + %15 = tuple () + return %15 : $() +} + +// CHECK-LABEL: sil @dont_licm_ginit_prehead_not_pdom +// CHECK-NOT: apply +// CHECK: bb1: +// CHECK: apply +// CHECK: bb2: +// CHECK-NOT: apply +// CHECK: } // end sil function 'dont_licm_ginit_prehead_not_pdom' +sil @dont_licm_ginit_prehead_not_pdom : $@convention(thin) () -> () { +bb0: + cond_br undef, bb1, bb2 + +bb1: + %2 = function_ref @$s4test10testGlobalSivau : $@convention(thin) () -> Builtin.RawPointer + %3 = apply %2() : $@convention(thin) () -> Builtin.RawPointer + cond_br undef, bb1, bb2 + +bb2: + %15 = tuple () + return %15 : $() +} + +// CHECK-LABEL: sil @licm_ginit_complex_cfg +// CHECK-NOT: apply +// CHECK: bb0: +// CHECK: apply +// CHECK: bb1: +// CHECK-NOT: apply +// CHECK: } // end sil function 'licm_ginit_complex_cfg' +sil @licm_ginit_complex_cfg : $@convention(thin) () -> () { +bb0: + br bb1 + +bb1: + cond_br undef, bb2, bb3 + +bb2: + br bb4 + +bb3: + br bb4 + +bb4: + %2 = function_ref @$s4test10testGlobalSivau : $@convention(thin) () -> Builtin.RawPointer + %3 = apply %2() : $@convention(thin) () -> Builtin.RawPointer + cond_br undef, bb1, bb5 + +bb5: + %15 = tuple () + return %15 : $() +} + +// CHECK-LABEL: sil @dont_licm_ginit_apply_not_pdom +// CHECK-NOT: apply +// CHECK: bb2: +// CHECK: apply +// CHECK: bb3: +// CHECK-NOT: apply +// CHECK: } // end sil function 'dont_licm_ginit_apply_not_pdom' +sil @dont_licm_ginit_apply_not_pdom : $@convention(thin) () -> () { +bb0: + br bb1 + +bb1: + cond_br undef, bb2, bb3 + +bb2: + %2 = function_ref @$s4test10testGlobalSivau : $@convention(thin) () -> Builtin.RawPointer + %3 = apply %2() : $@convention(thin) () -> Builtin.RawPointer + br bb4 + +bb3: + br bb4 + +bb4: + cond_br undef, bb1, bb5 + +bb5: + %15 = tuple () + return %15 : $() +} + +// CHECK-LABEL: sil @dont_licm_ginit_dependency1 +// CHECK: bb0: +// CHECK: [[I:%[0-9]+]] = function_ref @$s4test10testGlobalSivau +// CHECK: bb2: +// CHECK: [[U:%[0-9]+]] = function_ref @unknown +// CHECK: apply [[U]] +// CHECK: bb4: +// CHECK: apply [[I]] +// CHECK: bb5: +// CHECK: } // end sil function 'dont_licm_ginit_dependency1' +sil @dont_licm_ginit_dependency1 : $@convention(thin) () -> () { +bb0: + br bb1 + +bb1: + cond_br undef, bb2, bb3 + +bb2: + %f = function_ref @unknown : $@convention(thin) () -> () + %a = apply %f() : $@convention(thin) () -> () + br bb4 + +bb3: + br bb4 + +bb4: + %2 = function_ref @$s4test10testGlobalSivau : $@convention(thin) () -> Builtin.RawPointer + %3 = apply %2() : $@convention(thin) () -> Builtin.RawPointer + cond_br undef, bb1, bb5 + +bb5: + %15 = tuple () + return %15 : $() +} + +// CHECK-LABEL: sil @dont_licm_ginit_dependency2 +// CHECK: bb0: +// CHECK-DAG: [[I:%[0-9]+]] = function_ref @$s4test10testGlobalSivau +// CHECK-DAG: [[U:%[0-9]+]] = function_ref @unknown +// CHECK: bb1: +// CHECK: apply [[U]] +// CHECK: apply [[I]] +// CHECK: bb2: +// CHECK: } // end sil function 'dont_licm_ginit_dependency2' +sil @dont_licm_ginit_dependency2 : $@convention(thin) () -> () { +bb0: + br bb1 + +bb1: + %f = function_ref @unknown : $@convention(thin) () -> () + %a = apply %f() : $@convention(thin) () -> () + %2 = function_ref @$s4test10testGlobalSivau : $@convention(thin) () -> Builtin.RawPointer + %3 = apply %2() : $@convention(thin) () -> Builtin.RawPointer + cond_br undef, bb1, bb2 + +bb2: + %15 = tuple () + return %15 : $() +} + +// CHECK-LABEL: sil @licm_ginit_no_dependency +// CHECK: bb0: +// CHECK-DAG: [[I:%[0-9]+]] = function_ref @$s4test10testGlobalSivau +// CHECK-DAG: [[U:%[0-9]+]] = function_ref @unknown +// CHECK-DAG: apply [[I]] +// CHECK: bb1: +// CHECK: apply [[U]] +// CHECK: bb2: +// CHECK: } // end sil function 'licm_ginit_no_dependency' +sil @licm_ginit_no_dependency : $@convention(thin) () -> () { +bb0: + br bb1 + +bb1: + %2 = function_ref @$s4test10testGlobalSivau : $@convention(thin) () -> Builtin.RawPointer + %3 = apply %2() : $@convention(thin) () -> Builtin.RawPointer + %f = function_ref @unknown : $@convention(thin) () -> () + %a = apply %f() : $@convention(thin) () -> () + cond_br undef, bb1, bb2 + +bb2: + %15 = tuple () + return %15 : $() +} + diff --git a/test/SILOptimizer/licm_exclusivity.sil b/test/SILOptimizer/licm_exclusivity.sil index 6e7a5fd821cb7..39deb412efdb5 100644 --- a/test/SILOptimizer/licm_exclusivity.sil +++ b/test/SILOptimizer/licm_exclusivity.sil @@ -21,7 +21,7 @@ var globalY: X sil_global hidden @globalY : $X -sil hidden_external [global_init] @globalAddressor : $@convention(thin) () -> Builtin.RawPointer +sil hidden_external @globalAddressor : $@convention(thin) () -> Builtin.RawPointer // public func hoist_access_with_conflict() { // Tests Hoisting of begin/end access when there's a "sandwiched" unidentified access