Skip to content

Commit 595a7db

Browse files
committed
[epilogue-arc-analysis] Do not copy lists by value to read them. Use an ArrayRef instead.
I don't think I need to say more here. rdar://41146023
1 parent f46de47 commit 595a7db

File tree

4 files changed

+40
-38
lines changed

4 files changed

+40
-38
lines changed

include/swift/SILOptimizer/Analysis/ARCAnalysis.h

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,6 @@ bool isRetainInstruction(SILInstruction *II);
4646
/// Return true if this is a release instruction.
4747
bool isReleaseInstruction(SILInstruction *II);
4848

49-
using RetainList = TinyPtrVector<SILInstruction *>;
50-
using ReleaseList = TinyPtrVector<SILInstruction *>;
51-
5249
/// \returns True if the user \p User decrements the ref count of \p Ptr.
5350
bool mayDecrementRefCount(SILInstruction *User, SILValue Ptr,
5451
AliasAnalysis *AA);
@@ -145,7 +142,7 @@ class ConsumedResultToEpilogueRetainMatcher {
145142

146143
// We use a list of instructions for now so that we can keep the same interface
147144
// and handle exploded retain_value later.
148-
RetainList EpilogueRetainInsts;
145+
TinyPtrVector<SILInstruction *> EpilogueRetainInsts;
149146

150147
public:
151148
/// Finds matching releases in the return block of the function \p F.
@@ -156,7 +153,9 @@ class ConsumedResultToEpilogueRetainMatcher {
156153
/// Finds matching releases in the provided block \p BB.
157154
void findMatchingRetains(SILBasicBlock *BB);
158155

159-
RetainList getEpilogueRetains() { return EpilogueRetainInsts; }
156+
ArrayRef<SILInstruction *> getEpilogueRetains() const {
157+
return EpilogueRetainInsts;
158+
}
160159

161160
/// Recompute the mapping from argument to consumed arg.
162161
void recompute();
@@ -202,7 +201,9 @@ class ConsumedArgToEpilogueReleaseMatcher {
202201
RCIdentityFunctionInfo *RCFI;
203202
ExitKind Kind;
204203
ArrayRef<SILArgumentConvention> ArgumentConventions;
205-
llvm::SmallMapVector<SILArgument *, ReleaseList, 8> ArgInstMap;
204+
205+
using InstListTy = TinyPtrVector<SILInstruction *>;
206+
llvm::SmallMapVector<SILArgument *, InstListTy, 8> ArgInstMap;
206207

207208
/// Set to true if we found some releases but not all for the argument.
208209
llvm::DenseSet<SILArgument *> FoundSomeReleases;
@@ -267,33 +268,32 @@ class ConsumedArgToEpilogueReleaseMatcher {
267268
return getSingleReleaseForArgument(Arg);
268269
}
269270

270-
ReleaseList getReleasesForArgument(SILArgument *Arg) {
271-
ReleaseList Releases;
271+
ArrayRef<SILInstruction *> getReleasesForArgument(SILArgument *Arg) {
272272
auto I = ArgInstMap.find(Arg);
273273
if (I == ArgInstMap.end())
274-
return Releases;
275-
return I->second;
274+
return {};
275+
return I->second;
276276
}
277277

278-
ReleaseList getReleasesForArgument(SILValue V) {
279-
ReleaseList Releases;
278+
ArrayRef<SILInstruction *> getReleasesForArgument(SILValue V) {
280279
auto *Arg = dyn_cast<SILArgument>(V);
281280
if (!Arg)
282-
return Releases;
281+
return {};
283282
return getReleasesForArgument(Arg);
284283
}
285284

286285
/// Recompute the mapping from argument to consumed arg.
287286
void recompute();
288287

289288
bool isSingleReleaseMatchedToArgument(SILInstruction *Inst) {
290-
auto Pred = [&Inst](const std::pair<SILArgument *,
291-
ReleaseList> &P) -> bool {
292-
if (P.second.size() > 1)
293-
return false;
294-
return *P.second.begin() == Inst;
295-
};
296-
return count_if(ArgInstMap, Pred);
289+
return count_if(
290+
ArgInstMap,
291+
[&Inst](const std::pair<SILArgument *, ArrayRef<SILInstruction *>> &P)
292+
-> bool {
293+
if (P.second.size() > 1)
294+
return false;
295+
return *P.second.begin() == Inst;
296+
});
297297
}
298298

299299
using iterator = decltype(ArgInstMap)::iterator;
@@ -322,11 +322,12 @@ class ConsumedArgToEpilogueReleaseMatcher {
322322
/// between the releases values in \p Insts and \p Derived, it also bails
323323
/// out and return true if projection path can not be formed between Base
324324
/// and any one the released values.
325-
bool isRedundantRelease(ReleaseList Insts, SILValue Base, SILValue Derived);
325+
bool isRedundantRelease(ArrayRef<SILInstruction *> Insts, SILValue Base,
326+
SILValue Derived);
326327

327328
/// Return true if we have a release instruction for all the reference
328329
/// semantics part of \p Argument.
329-
bool releaseArgument(ReleaseList Insts, SILValue Argument);
330+
bool releaseArgument(ArrayRef<SILInstruction *> Insts, SILValue Argument);
330331

331332
/// Walk the basic block and find all the releases that match to function
332333
/// arguments.

lib/SILOptimizer/Analysis/ARCAnalysis.cpp

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -707,9 +707,8 @@ void ConsumedArgToEpilogueReleaseMatcher::recompute() {
707707
findMatchingReleases(&*BB);
708708
}
709709

710-
bool
711-
ConsumedArgToEpilogueReleaseMatcher::
712-
isRedundantRelease(ReleaseList Insts, SILValue Base, SILValue Derived) {
710+
bool ConsumedArgToEpilogueReleaseMatcher::isRedundantRelease(
711+
ArrayRef<SILInstruction *> Insts, SILValue Base, SILValue Derived) {
713712
// We use projection path to analyze the relation.
714713
auto POp = ProjectionPath::getProjectionPath(Base, Derived);
715714
// We can not build a projection path from the base to the derived, bail out.
@@ -730,9 +729,8 @@ isRedundantRelease(ReleaseList Insts, SILValue Base, SILValue Derived) {
730729
return false;
731730
}
732731

733-
bool
734-
ConsumedArgToEpilogueReleaseMatcher::
735-
releaseArgument(ReleaseList Insts, SILValue Arg) {
732+
bool ConsumedArgToEpilogueReleaseMatcher::releaseArgument(
733+
ArrayRef<SILInstruction *> Insts, SILValue Arg) {
736734
// Reason about whether all parts are released.
737735
SILModule *Mod = &(*Insts.begin())->getModule();
738736

lib/SILOptimizer/FunctionSignatureTransforms/FunctionSignatureOpts.h

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -65,14 +65,14 @@ struct ArgumentDescriptor {
6565
/// Is this parameter an indirect result?
6666
bool IsIndirectResult;
6767

68-
/// If non-null, this is the release in the return block of the callee, which
69-
/// is associated with this parameter if it is @owned. If the parameter is not
70-
/// @owned or we could not find such a release in the callee, this is null.
71-
ReleaseList CalleeRelease;
72-
73-
/// The same as CalleeRelease, but the release in the throw block, if it is a
74-
/// function which has a throw block.
75-
ReleaseList CalleeReleaseInThrowBlock;
68+
/// If non-empty, this is the set of releases in the return block of
69+
/// the callee associated with this parameter if it is @owned. If it
70+
/// is empty then we could not find any such releases.
71+
TinyPtrVector<SILInstruction *> CalleeRelease;
72+
73+
/// The same as CalleeRelease, but the releases are post-dominated
74+
/// by the throw block, if it is a function which has a throw block.
75+
TinyPtrVector<SILInstruction *> CalleeReleaseInThrowBlock;
7676

7777
/// The projection tree of this arguments.
7878
ProjectionTree ProjTree;

lib/SILOptimizer/FunctionSignatureTransforms/OwnedToGuaranteedTransform.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,11 @@ bool FunctionSignatureTransform::OwnedToGuaranteedAnalyzeParameters() {
9292
auto ReleasesInThrow =
9393
ArgToThrowReleaseMap.getReleasesForArgument(A.Arg);
9494
if (!ArgToThrowReleaseMap.hasBlock() || !ReleasesInThrow.empty()) {
95-
A.CalleeRelease = Releases;
96-
A.CalleeReleaseInThrowBlock = ReleasesInThrow;
95+
assert(A.CalleeRelease.empty());
96+
assert(A.CalleeReleaseInThrowBlock.empty());
97+
copy(Releases, std::back_inserter(A.CalleeRelease));
98+
copy(ReleasesInThrow,
99+
std::back_inserter(A.CalleeReleaseInThrowBlock));
97100
// We can convert this parameter to a @guaranteed.
98101
A.OwnedToGuaranteed = true;
99102
SignatureOptimize = true;

0 commit comments

Comments
 (0)