Skip to content

Commit 814d890

Browse files
committed
Use the new side effects in LICM
1 parent f082584 commit 814d890

File tree

3 files changed

+92
-53
lines changed

3 files changed

+92
-53
lines changed

lib/SILOptimizer/LoopTransforms/LICM.cpp

Lines changed: 87 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,9 @@
2424
#include "swift/SILOptimizer/Analysis/AliasAnalysis.h"
2525
#include "swift/SILOptimizer/Analysis/Analysis.h"
2626
#include "swift/SILOptimizer/Analysis/ArraySemantic.h"
27+
#include "swift/SILOptimizer/Analysis/BasicCalleeAnalysis.h"
2728
#include "swift/SILOptimizer/Analysis/DominanceAnalysis.h"
2829
#include "swift/SILOptimizer/Analysis/LoopAnalysis.h"
29-
#include "swift/SILOptimizer/Analysis/SideEffectAnalysis.h"
3030
#include "swift/SILOptimizer/PassManager/Passes.h"
3131
#include "swift/SILOptimizer/PassManager/Transforms.h"
3232
#include "swift/SILOptimizer/Utils/CFGOptUtils.h"
@@ -166,31 +166,56 @@ static bool isOnlyLoadedAndStored(AliasAnalysis *AA, InstSet &SideEffectInsts,
166166
/// Returns true if the \p SideEffectInsts set contains any memory writes which
167167
/// may alias with any memory which is read by \p AI.
168168
/// Note: This function should only be called on a read-only apply!
169-
static bool mayWriteTo(AliasAnalysis *AA, SideEffectAnalysis *SEA,
169+
static bool mayWriteTo(AliasAnalysis *AA, BasicCalleeAnalysis *BCA,
170170
InstSet &SideEffectInsts, ApplyInst *AI) {
171-
FunctionSideEffects E;
172-
SEA->getCalleeEffects(E, AI);
173-
assert(E.getMemBehavior(RetainObserveKind::IgnoreRetains) <=
174-
SILInstruction::MemoryBehavior::MayRead &&
175-
"apply should only read from memory");
176-
assert(!E.getGlobalEffects().mayRead() &&
177-
"apply should not have global effects");
178-
179-
for (unsigned Idx = 0, End = AI->getNumArguments(); Idx < End; ++Idx) {
180-
auto &ArgEffect = E.getParameterEffects()[Idx];
181-
assert(!ArgEffect.mayRelease() && "apply should only read from memory");
182-
if (!ArgEffect.mayRead())
183-
continue;
184171

185-
SILValue Arg = AI->getArgument(Idx);
172+
if (BCA->getMemoryBehavior(FullApplySite::isa(AI), /*observeRetains*/true) ==
173+
SILInstruction::MemoryBehavior::None) {
174+
return false;
175+
}
186176

187-
// Check if the memory addressed by the argument may alias any writes.
188-
for (auto *I : SideEffectInsts) {
189-
if (AA->mayWriteToMemory(I, Arg)) {
190-
LLVM_DEBUG(llvm::dbgs() << " mayWriteTo\n" << *I << " to "
191-
<< *AI << "\n");
192-
return true;
177+
// Check if the memory addressed by the argument may alias any writes.
178+
for (auto *inst : SideEffectInsts) {
179+
switch (inst->getKind()) {
180+
case SILInstructionKind::StoreInst: {
181+
auto *si = cast<StoreInst>(inst);
182+
if (si->getOwnershipQualifier() == StoreOwnershipQualifier::Assign)
183+
return true;
184+
if (AA->mayReadFromMemory(AI, si->getDest()))
185+
return true;
186+
break;
187+
}
188+
case SILInstructionKind::CopyAddrInst: {
189+
auto *ca = cast<CopyAddrInst>(inst);
190+
if (!ca->isInitializationOfDest())
191+
return true;
192+
if (AA->mayReadFromMemory(AI, ca->getDest()))
193+
return true;
194+
break;
193195
}
196+
case SILInstructionKind::ApplyInst:
197+
case SILInstructionKind::BeginApplyInst:
198+
case SILInstructionKind::TryApplyInst: {
199+
if (BCA->getMemoryBehavior(FullApplySite::isa(inst), /*observeRetains*/false) >
200+
SILInstruction::MemoryBehavior::MayRead)
201+
return true;
202+
break;
203+
}
204+
case SILInstructionKind::CondFailInst:
205+
case SILInstructionKind::StrongRetainInst:
206+
case SILInstructionKind::UnmanagedRetainValueInst:
207+
case SILInstructionKind::RetainValueInst:
208+
case SILInstructionKind::StrongRetainUnownedInst:
209+
case SILInstructionKind::FixLifetimeInst:
210+
case SILInstructionKind::KeyPathInst:
211+
case SILInstructionKind::DeallocStackInst:
212+
case SILInstructionKind::DeallocStackRefInst:
213+
case SILInstructionKind::DeallocRefInst:
214+
break;
215+
default:
216+
if (inst->mayWriteToMemory())
217+
return true;
218+
break;
194219
}
195220
}
196221
return false;
@@ -509,7 +534,7 @@ class LoopTreeOptimization {
509534
InstSet toDelete;
510535
SILLoopInfo *LoopInfo;
511536
AliasAnalysis *AA;
512-
SideEffectAnalysis *SEA;
537+
BasicCalleeAnalysis *BCA;
513538
DominanceInfo *DomTree;
514539
PostDominanceAnalysis *PDA;
515540
PostDominanceInfo *postDomTree = nullptr;
@@ -540,11 +565,11 @@ class LoopTreeOptimization {
540565

541566
public:
542567
LoopTreeOptimization(SILLoop *TopLevelLoop, SILLoopInfo *LI,
543-
AliasAnalysis *AA, SideEffectAnalysis *SEA,
568+
AliasAnalysis *AA, BasicCalleeAnalysis *BCA,
544569
DominanceInfo *DT, PostDominanceAnalysis *PDA,
545570
AccessStorageAnalysis *ASA,
546571
bool RunsOnHighLevelSil)
547-
: LoopInfo(LI), AA(AA), SEA(SEA), DomTree(DT), PDA(PDA), ASA(ASA),
572+
: LoopInfo(LI), AA(AA), BCA(BCA), DomTree(DT), PDA(PDA), ASA(ASA),
548573
Changed(false), RunsOnHighLevelSIL(RunsOnHighLevelSil) {
549574
// Collect loops for a recursive bottom-up traversal in the loop tree.
550575
BotUpWorkList.push_back(TopLevelLoop);
@@ -562,6 +587,8 @@ class LoopTreeOptimization {
562587
/// Propagate the sub-loops' summaries up to the current loop.
563588
void propagateSummaries(std::unique_ptr<LoopNestSummary> &CurrSummary);
564589

590+
bool isSafeReadOnlyApply(BasicCalleeAnalysis *BCA, ApplyInst *AI);
591+
565592
/// Collect a set of instructions that can be hoisted
566593
void analyzeCurrentLoop(std::unique_ptr<LoopNestSummary> &CurrSummary);
567594

@@ -658,19 +685,22 @@ void LoopTreeOptimization::propagateSummaries(
658685
}
659686
}
660687

661-
static bool isSafeReadOnlyApply(SideEffectAnalysis *SEA, ApplyInst *AI) {
662-
FunctionSideEffects E;
663-
SEA->getCalleeEffects(E, AI);
688+
bool LoopTreeOptimization::isSafeReadOnlyApply(BasicCalleeAnalysis *BCA, ApplyInst *AI) {
689+
if (auto ri = AI->getSingleResult()) {
690+
// We don't balance CSE'd apply results which return an owned value.
691+
if (ri.getValue().getConvention() != ResultConvention::Unowned)
692+
return false;
693+
}
664694

665-
if (E.getGlobalEffects().mayRead()) {
666-
// If we have Global effects,
667-
// we don't know which memory is read in the callee.
668-
// Therefore we bail for safety
669-
return false;
695+
if (RunsOnHighLevelSIL) {
696+
// The array-property-opt needs this semantic call inside the loop.
697+
// After high-level SIL we can hoist it (if it's not inlined already).
698+
if (ArraySemanticsCall(AI, "array.props.isNativeTypeChecked"))
699+
return false;
670700
}
671701

672-
auto MB = E.getMemBehavior(RetainObserveKind::ObserveRetains);
673-
return (MB <= SILInstruction::MemoryBehavior::MayRead);
702+
return BCA->getMemoryBehavior(AI, /*observeRetains*/false) <=
703+
SILInstruction::MemoryBehavior::MayRead;
674704
}
675705

676706
static void checkSideEffects(swift::SILInstruction &Inst,
@@ -695,26 +725,31 @@ static bool canHoistUpDefault(SILInstruction *inst, SILLoop *Loop,
695725
return false;
696726
}
697727

698-
if (inst->getMemoryBehavior() == SILInstruction::MemoryBehavior::None) {
699-
return true;
700-
}
701-
702-
if (!RunsOnHighLevelSil) {
703-
return false;
704-
}
705-
706728
// We can’t hoist everything that is hoist-able
707729
// The canHoist method does not do all the required analysis
708730
// Some of the work is done at COW Array Opt
709731
// TODO: Refactor COW Array Opt + canHoist - radar 41601468
710732
ArraySemanticsCall semCall(inst);
711733
switch (semCall.getKind()) {
712-
case ArrayCallKind::kGetCount:
713-
case ArrayCallKind::kGetCapacity:
714-
return semCall.canHoist(Preheader->getTerminator(), DT);
715-
default:
716-
return false;
734+
case ArrayCallKind::kGetCount:
735+
case ArrayCallKind::kGetCapacity:
736+
if (RunsOnHighLevelSil && semCall.canHoist(Preheader->getTerminator(), DT))
737+
return true;
738+
break;
739+
case ArrayCallKind::kArrayPropsIsNativeTypeChecked:
740+
// The array-property-opt needs this semantic call inside the loop.
741+
// After high-level SIL we can hoist it (if it's not inlined already).
742+
if (RunsOnHighLevelSil)
743+
return false;
744+
break;
745+
default:
746+
break;
747+
}
748+
749+
if (inst->getMemoryBehavior() == SILInstruction::MemoryBehavior::None) {
750+
return true;
717751
}
752+
return false;
718753
}
719754

720755
// Check If all the end accesses of the given begin do not prevent hoisting
@@ -874,7 +909,7 @@ void LoopTreeOptimization::analyzeCurrentLoop(
874909
break;
875910
case SILInstructionKind::ApplyInst: {
876911
auto *AI = cast<ApplyInst>(&Inst);
877-
if (isSafeReadOnlyApply(SEA, AI)) {
912+
if (isSafeReadOnlyApply(BCA, AI)) {
878913
ReadOnlyApplies.push_back(AI);
879914
} else if (SILFunction *callee = AI->getReferencedFunctionOrNull()) {
880915
// Calls to global inits are different because we don't care about
@@ -903,7 +938,7 @@ void LoopTreeOptimization::analyzeCurrentLoop(
903938
}
904939

905940
for (auto *AI : ReadOnlyApplies) {
906-
if (!mayWriteTo(AA, SEA, sideEffects, AI)) {
941+
if (!mayWriteTo(AA, BCA, sideEffects, AI)) {
907942
HoistUp.insert(AI);
908943
}
909944
}
@@ -1471,7 +1506,7 @@ class LICM : public SILFunctionTransform {
14711506
DominanceAnalysis *DA = PM->getAnalysis<DominanceAnalysis>();
14721507
PostDominanceAnalysis *PDA = PM->getAnalysis<PostDominanceAnalysis>();
14731508
AliasAnalysis *AA = PM->getAnalysis<AliasAnalysis>(F);
1474-
SideEffectAnalysis *SEA = PM->getAnalysis<SideEffectAnalysis>();
1509+
BasicCalleeAnalysis *BCA = PM->getAnalysis<BasicCalleeAnalysis>();
14751510
AccessStorageAnalysis *ASA = getAnalysis<AccessStorageAnalysis>();
14761511
DominanceInfo *DomTree = nullptr;
14771512

@@ -1480,7 +1515,7 @@ class LICM : public SILFunctionTransform {
14801515

14811516
for (auto *TopLevelLoop : *LoopInfo) {
14821517
if (!DomTree) DomTree = DA->get(F);
1483-
LoopTreeOptimization Opt(TopLevelLoop, LoopInfo, AA, SEA, DomTree, PDA,
1518+
LoopTreeOptimization Opt(TopLevelLoop, LoopInfo, AA, BCA, DomTree, PDA,
14841519
ASA, RunsOnHighLevelSil);
14851520
Changed |= Opt.optimize();
14861521
}

test/SILOptimizer/licm.sil

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
// RUN: %target-sil-opt -enforce-exclusivity=none -enable-sil-verify-all %s -licm | %FileCheck %s
22

3+
// REQUIRES: swift_in_compiler
4+
35
// Declare this SIL to be canonical because some tests break raw SIL
46
// conventions. e.g. address-type block args. -enforce-exclusivity=none is also
57
// required to allow address-type block args in canonical SIL.

test/SILOptimizer/licm_apply.sil

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
// RUN: %target-sil-opt -enable-sil-verify-all %s -licm | %FileCheck %s
1+
// RUN: %target-sil-opt -enable-sil-verify-all %s -compute-side-effects -licm | %FileCheck %s
2+
3+
// REQUIRES: swift_in_compiler
24

35
sil_stage canonical
46

0 commit comments

Comments
 (0)