Skip to content

Commit 369b1f4

Browse files
committed
[epilogue-release-matcher] Pass DenseSet by reference instead of by value.
This is particularly egrigious since we are only /reading/ from the DenseSet. So we are basically mallocing/copying a DenseSet just to read from it... I don't think I need to say more. rdar://41146023
1 parent 595a7db commit 369b1f4

File tree

2 files changed

+14
-10
lines changed

2 files changed

+14
-10
lines changed

include/swift/SILOptimizer/Analysis/ARCAnalysis.h

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,16 +13,17 @@
1313
#ifndef SWIFT_SILOPTIMIZER_ANALYSIS_ARCANALYSIS_H
1414
#define SWIFT_SILOPTIMIZER_ANALYSIS_ARCANALYSIS_H
1515

16+
#include "swift/Basic/LLVM.h"
1617
#include "swift/SIL/SILArgument.h"
17-
#include "swift/SIL/SILValue.h"
1818
#include "swift/SIL/SILBasicBlock.h"
19+
#include "swift/SIL/SILValue.h"
1920
#include "swift/SILOptimizer/Analysis/AliasAnalysis.h"
2021
#include "swift/SILOptimizer/Analysis/PostOrderAnalysis.h"
2122
#include "swift/SILOptimizer/Analysis/RCIdentityAnalysis.h"
2223
#include "llvm/ADT/BitVector.h"
23-
#include "llvm/ADT/SmallPtrSet.h"
2424
#include "llvm/ADT/MapVector.h"
2525
#include "llvm/ADT/SetVector.h"
26+
#include "llvm/ADT/SmallPtrSet.h"
2627
#include "llvm/ADT/TinyPtrVector.h"
2728

2829
namespace swift {
@@ -181,7 +182,8 @@ class ConsumedResultToEpilogueRetainMatcher {
181182
private:
182183
/// Return true if all the successors of the EpilogueRetainInsts do not have
183184
/// a retain.
184-
bool isTransitiveSuccessorsRetainFree(llvm::DenseSet<SILBasicBlock *> BBs);
185+
bool
186+
isTransitiveSuccessorsRetainFree(const llvm::DenseSet<SILBasicBlock *> &BBs);
185187

186188
/// Finds matching releases in the provided block \p BB.
187189
RetainKindValue findMatchingRetainsInBasicBlock(SILBasicBlock *BB,

lib/SILOptimizer/Analysis/ARCAnalysis.cpp

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -498,22 +498,24 @@ void ConsumedResultToEpilogueRetainMatcher::recompute() {
498498
findMatchingRetains(&*BB);
499499
}
500500

501-
bool
502-
ConsumedResultToEpilogueRetainMatcher::
503-
isTransitiveSuccessorsRetainFree(llvm::DenseSet<SILBasicBlock *> BBs) {
501+
bool ConsumedResultToEpilogueRetainMatcher::isTransitiveSuccessorsRetainFree(
502+
const llvm::DenseSet<SILBasicBlock *> &BBs) {
504503
// For every block with retain, we need to check the transitive
505504
// closure of its successors are retain-free.
506505
for (auto &I : EpilogueRetainInsts) {
507-
auto *CBB = I->getParent();
508-
for (auto &Succ : CBB->getSuccessors()) {
509-
if (BBs.find(Succ) != BBs.end())
506+
for (auto &Succ : I->getParent()->getSuccessors()) {
507+
if (BBs.count(Succ))
510508
continue;
511509
return false;
512510
}
513511
}
512+
513+
// FIXME: We are iterating over a DenseSet. That can lead to non-determinism
514+
// and is in general pretty inefficient since we are iterating over a hash
515+
// table.
514516
for (auto CBB : BBs) {
515517
for (auto &Succ : CBB->getSuccessors()) {
516-
if (BBs.find(Succ) != BBs.end())
518+
if (BBs.count(Succ))
517519
continue;
518520
return false;
519521
}

0 commit comments

Comments
 (0)