Skip to content

Commit d5e8972

Browse files
committed
[rbi] Remove some unnecessary copying when constructing partition ops.
Just a small efficiency win that @rjmccall and I noticed when I was bringing him up on rbi.
1 parent cda4eba commit d5e8972

File tree

1 file changed

+42
-23
lines changed

1 file changed

+42
-23
lines changed

lib/SILOptimizer/Analysis/RegionAnalysis.cpp

Lines changed: 42 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1598,11 +1598,33 @@ struct PartitionOpBuilder {
15981598

15991599
/// List of partition ops mapped to the current instruction. Used when
16001600
/// generating partition ops.
1601-
SmallVector<PartitionOp, 8> currentInstPartitionOps;
1601+
std::vector<PartitionOp> *currentInstPartitionOps = nullptr;
1602+
1603+
/// The initial partition op index since the last reset. We use this so that
1604+
/// we can dump out the partition ops that we generated for a specific
1605+
/// instruction without needing to waste cycles maintaining a separate
1606+
/// SmallVector of PartitionOps inside of PartitionOpBuilder.
1607+
unsigned initialPartitionOpIndex = 0;
1608+
1609+
void initialize(std::vector<PartitionOp> &foundPartitionOps) {
1610+
assert(foundPartitionOps.empty());
1611+
currentInstPartitionOps = &foundPartitionOps;
1612+
initialPartitionOpIndex = currentInstPartitionOps->size();
1613+
}
16021614

16031615
void reset(SILInstruction *inst) {
1616+
assert(currentInstPartitionOps);
16041617
currentInst = inst;
1605-
currentInstPartitionOps.clear();
1618+
initialPartitionOpIndex = currentInstPartitionOps->size();
1619+
}
1620+
1621+
/// Return an ArrayRef containing the PartitionOps associated with the current
1622+
/// instruction being built.
1623+
ArrayRef<PartitionOp> getPartitionOpsForCurrentInst() const {
1624+
assert(currentInstPartitionOps);
1625+
unsigned numElts =
1626+
currentInstPartitionOps->size() - initialPartitionOpIndex;
1627+
return llvm::ArrayRef(*currentInstPartitionOps).take_back(numElts);
16061628
}
16071629

16081630
Element lookupValueID(SILValue value);
@@ -1614,7 +1636,7 @@ struct PartitionOpBuilder {
16141636
if (value.isSendable())
16151637
return;
16161638
auto id = lookupValueID(value.getRepresentative().getValue());
1617-
currentInstPartitionOps.emplace_back(
1639+
currentInstPartitionOps->emplace_back(
16181640
PartitionOp::AssignFresh(id, currentInst));
16191641
}
16201642

@@ -1623,15 +1645,15 @@ struct PartitionOpBuilder {
16231645
return;
16241646

16251647
auto first = lookupValueID(values.front());
1626-
currentInstPartitionOps.emplace_back(
1648+
currentInstPartitionOps->emplace_back(
16271649
PartitionOp::AssignFresh(first, currentInst));
16281650

16291651
auto transformedCollection =
16301652
makeTransformRange(values.drop_front(), [&](SILValue value) {
16311653
return lookupValueID(value);
16321654
});
16331655
for (auto id : transformedCollection) {
1634-
currentInstPartitionOps.emplace_back(
1656+
currentInstPartitionOps->emplace_back(
16351657
PartitionOp::AssignFreshAssign(id, first, currentInst));
16361658
}
16371659
}
@@ -1650,7 +1672,7 @@ struct PartitionOpBuilder {
16501672
return;
16511673
}
16521674

1653-
currentInstPartitionOps.emplace_back(
1675+
currentInstPartitionOps->emplace_back(
16541676
PartitionOp::Assign(lookupValueID(destValue),
16551677
lookupValueID(srcOperand->get()), srcOperand));
16561678
}
@@ -1663,15 +1685,15 @@ struct PartitionOpBuilder {
16631685
assert(valueHasID(representative) &&
16641686
"sent value should already have been encountered");
16651687

1666-
currentInstPartitionOps.emplace_back(
1688+
currentInstPartitionOps->emplace_back(
16671689
PartitionOp::Send(lookupValueID(representative), op));
16681690
}
16691691

16701692
void addUndoSend(SILValue representative, SILInstruction *unsendingInst) {
16711693
assert(valueHasID(representative) &&
16721694
"value should already have been encountered");
16731695

1674-
currentInstPartitionOps.emplace_back(
1696+
currentInstPartitionOps->emplace_back(
16751697
PartitionOp::UndoSend(lookupValueID(representative), unsendingInst));
16761698
}
16771699

@@ -1686,7 +1708,7 @@ struct PartitionOpBuilder {
16861708
if (lookupValueID(rep) == lookupValueID(srcOperand->get()))
16871709
return;
16881710

1689-
currentInstPartitionOps.emplace_back(PartitionOp::Merge(
1711+
currentInstPartitionOps->emplace_back(PartitionOp::Merge(
16901712
lookupValueID(rep), lookupValueID(srcOperand->get()), srcOperand));
16911713
}
16921714

@@ -1698,9 +1720,9 @@ struct PartitionOpBuilder {
16981720
"merged values should already have been encountered");
16991721

17001722
auto elt = getActorIntroducingRepresentative(actorIsolation);
1701-
currentInstPartitionOps.emplace_back(
1723+
currentInstPartitionOps->emplace_back(
17021724
PartitionOp::AssignFresh(elt, currentInst));
1703-
currentInstPartitionOps.emplace_back(
1725+
currentInstPartitionOps->emplace_back(
17041726
PartitionOp::Merge(lookupValueID(sourceValue), elt, sourceOperand));
17051727
}
17061728

@@ -1723,7 +1745,7 @@ struct PartitionOpBuilder {
17231745
auto rep = value.getRepresentative().getValue();
17241746
assert(valueHasID(rep, /*dumpIfHasNoID=*/true) &&
17251747
"required value should already have been encountered");
1726-
currentInstPartitionOps.emplace_back(
1748+
currentInstPartitionOps->emplace_back(
17271749
PartitionOp::InOutSendingAtFunctionExit(lookupValueID(rep),
17281750
currentInst));
17291751
}
@@ -1733,15 +1755,15 @@ struct PartitionOpBuilder {
17331755
llvm::report_fatal_error(
17341756
"RegionIsolation: Aborting on unknown pattern match error");
17351757
}
1736-
currentInstPartitionOps.emplace_back(
1758+
currentInstPartitionOps->emplace_back(
17371759
PartitionOp::UnknownPatternError(lookupValueID(value), currentInst));
17381760
}
17391761

17401762
void addNonSendableIsolationCrossingResultError(TrackableValue value) {
17411763
if (value.isSendable())
17421764
return;
17431765
auto rep = value.getRepresentative().getValue();
1744-
currentInstPartitionOps.emplace_back(
1766+
currentInstPartitionOps->emplace_back(
17451767
PartitionOp::NonSendableIsolationCrossingResult(lookupValueID(rep),
17461768
currentInst));
17471769
}
@@ -1986,7 +2008,7 @@ class PartitionOpTranslator {
19862008
PartitionOpTranslator(SILFunction *function, PostOrderFunctionInfo *pofi,
19872009
RegionAnalysisValueMap &valueMap,
19882010
IsolationHistory::Factory &historyFactory)
1989-
: function(function), initialEntryBlockPartition(), builder(),
2011+
: function(function), initialEntryBlockPartition(),
19902012
partialApplyReachabilityDataflow(function, valueMap, pofi),
19912013
valueMap(valueMap) {
19922014
builder.translator = this;
@@ -3007,13 +3029,13 @@ class PartitionOpTranslator {
30073029
basicBlock->printID(llvm::dbgs()); llvm::dbgs() << SEP_STR;
30083030
basicBlock->print(llvm::dbgs());
30093031
llvm::dbgs() << SEP_STR << "Results:\n";);
3032+
30103033
// Translate each SIL instruction to the PartitionOps that it represents if
30113034
// any.
3035+
builder.initialize(foundPartitionOps);
30123036
for (auto &instruction : *basicBlock) {
30133037
REGIONBASEDISOLATION_LOG(llvm::dbgs() << "Visiting: " << instruction);
30143038
translateSILInstruction(&instruction);
3015-
copy(builder.currentInstPartitionOps,
3016-
std::back_inserter(foundPartitionOps));
30173039
}
30183040
}
30193041

@@ -3141,7 +3163,8 @@ bool PartitionOpBuilder::valueHasID(SILValue value, bool dumpIfHasNoID) {
31413163
void PartitionOpBuilder::print(llvm::raw_ostream &os) const {
31423164
#ifndef NDEBUG
31433165
// If we do not have anything to dump, just return.
3144-
if (currentInstPartitionOps.empty())
3166+
auto ops = getPartitionOpsForCurrentInst();
3167+
if (ops.empty())
31453168
return;
31463169

31473170
// First line.
@@ -3153,8 +3176,6 @@ void PartitionOpBuilder::print(llvm::raw_ostream &os) const {
31533176
currentInst->getLoc().getSourceLoc().printLineAndColumn(
31543177
llvm::dbgs(), currentInst->getFunction()->getASTContext().SourceMgr);
31553178

3156-
auto ops = llvm::ArrayRef(currentInstPartitionOps);
3157-
31583179
// First op on its own line.
31593180
llvm::dbgs() << "\n ├─────╼ ";
31603181
ops.front().print(llvm::dbgs());
@@ -3197,9 +3218,7 @@ void PartitionOpBuilder::addRequire(TrackableValue value,
31973218
assert(valueHasID(silValue, /*dumpIfHasNoID=*/true) &&
31983219
"required value should already have been encountered");
31993220

3200-
// Check if this value
3201-
3202-
currentInstPartitionOps.emplace_back(
3221+
currentInstPartitionOps->emplace_back(
32033222
PartitionOp::Require(lookupValueID(silValue), currentInst, options));
32043223
}
32053224

0 commit comments

Comments
 (0)