Skip to content

Commit b5ce28f

Browse files
committed
[region-isolation] Cache getUnderlyingTrackedValue.
TLDR: Was looking at some performance traces and saw that we need to cache the result of this value. ---- Specifically, I noticed that we were spending a lot of time computing this operation. When I looked at the code I saw that we already had a cache along the relevant code paths... but the cache was from equivalence class representative -> state. Before we hit that cache, we were performing the work to map the value to the equivalence class representative... so the work to perform the relevant lookup from value -> state (which goes through the equivalence class representative) was not just a hash table lookup. This operation makes it cheaper by making it two cache lookups. It may be possible to make this cheaper by redoing the actual mapping of information so that we can go straight from value to state. I think it would be slightly different since we would probably need to represent the state in a separate array and map with indices... which is really just a more efficient hash table. We could also use malloc/etc but lets not even talk about that. rdar://139520959
1 parent edd6bf5 commit b5ce28f

File tree

2 files changed

+165
-107
lines changed

2 files changed

+165
-107
lines changed

include/swift/SILOptimizer/Analysis/RegionAnalysis.h

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -295,13 +295,52 @@ class RegionAnalysisValueMap {
295295
/// into this map
296296
llvm::DenseMap<RepresentativeValue, TrackableValueState>
297297
equivalenceClassValuesToState;
298+
299+
/// The inverse map of equivalenceClassValuesToState.
298300
llvm::DenseMap<unsigned, RepresentativeValue> stateIndexToEquivalenceClass;
299301

302+
/// State that the value -> representative computation yields to us.
303+
struct UnderlyingTrackedValueInfo {
304+
SILValue value;
305+
306+
/// Only used for addresses.
307+
std::optional<ActorIsolation> actorIsolation;
308+
309+
explicit UnderlyingTrackedValueInfo(SILValue value) : value(value) {}
310+
311+
UnderlyingTrackedValueInfo() : value(), actorIsolation() {}
312+
313+
UnderlyingTrackedValueInfo(const UnderlyingTrackedValueInfo &newVal)
314+
: value(newVal.value), actorIsolation(newVal.actorIsolation) {}
315+
316+
UnderlyingTrackedValueInfo &
317+
operator=(const UnderlyingTrackedValueInfo &newVal) {
318+
value = newVal.value;
319+
actorIsolation = newVal.actorIsolation;
320+
return *this;
321+
}
322+
323+
UnderlyingTrackedValueInfo(SILValue value,
324+
std::optional<ActorIsolation> actorIsolation)
325+
: value(value), actorIsolation(actorIsolation) {}
326+
327+
operator bool() const { return value; }
328+
};
329+
330+
/// A map from a SILValue to its equivalence class representative.
331+
llvm::DenseMap<SILValue, UnderlyingTrackedValueInfo> valueToEquivalenceClass;
332+
300333
SILFunction *fn;
301334

302335
public:
303336
RegionAnalysisValueMap(SILFunction *fn) : fn(fn) { }
304337

338+
/// Maps a value to its representative value if one exists. Return an empty
339+
/// representative value if we do not find one.
340+
SILValue getRepresentative(SILValue value) const {
341+
return getUnderlyingTrackedValue(value).value;
342+
}
343+
305344
/// Returns the value for this instruction if it isn't a fake "represenative
306345
/// value" to inject actor isolatedness. Asserts in such a case.
307346
SILValue getRepresentative(Element trackableValueID) const;
@@ -354,6 +393,30 @@ class RegionAnalysisValueMap {
354393
/// {TrackableValue(), false}.
355394
std::pair<TrackableValue, bool>
356395
initializeTrackableValue(SILValue value, SILIsolationInfo info) const;
396+
397+
/// A helper function that performs the actual getUnderlyingTrackedValue
398+
/// computation that is cached in getUnderlyingTrackedValue(). Please never
399+
/// call this directly! Only call it from getUnderlyingTrackedValue.
400+
UnderlyingTrackedValueInfo
401+
getUnderlyingTrackedValueHelper(SILValue value) const;
402+
403+
UnderlyingTrackedValueInfo getUnderlyingTrackedValue(SILValue value) const {
404+
// Use try_emplace so we only construct underlying tracked value info on
405+
// success and only lookup once in the hash table.
406+
auto *self = const_cast<RegionAnalysisValueMap *>(this);
407+
auto iter = self->valueToEquivalenceClass.try_emplace(
408+
value, UnderlyingTrackedValueInfo());
409+
410+
// Didn't insert... we have a value!
411+
if (!iter.second)
412+
return iter.first->getSecond();
413+
414+
// Otherwise, update with the actual tracked value info.
415+
iter.first->getSecond() = getUnderlyingTrackedValueHelper(value);
416+
417+
// And return the value.
418+
return iter.first->getSecond();
419+
}
357420
};
358421

359422
class RegionAnalysisFunctionInfo {
@@ -485,7 +548,9 @@ class RegionAnalysisFunctionInfo {
485548

486549
bool isClosureCaptured(SILValue value, Operand *op);
487550

488-
static SILValue getUnderlyingTrackedValue(SILValue value);
551+
SILValue getUnderlyingTrackedValue(SILValue value) {
552+
return getValueMap().getRepresentative(value);
553+
}
489554

490555
private:
491556
void runDataflow();

lib/SILOptimizer/Analysis/RegionAnalysis.cpp

Lines changed: 99 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -93,19 +93,6 @@ regionanalysisimpl::getApplyIsolationCrossing(SILInstruction *inst) {
9393

9494
namespace {
9595

96-
struct UnderlyingTrackedValueInfo {
97-
SILValue value;
98-
99-
/// Only used for addresses.
100-
std::optional<ActorIsolation> actorIsolation;
101-
102-
explicit UnderlyingTrackedValueInfo(SILValue value) : value(value) {}
103-
104-
UnderlyingTrackedValueInfo(SILValue value,
105-
std::optional<ActorIsolation> actorIsolation)
106-
: value(value), actorIsolation(actorIsolation) {}
107-
};
108-
10996
struct UseDefChainVisitor
11097
: public AccessUseDefChainVisitor<UseDefChainVisitor, SILValue> {
11198
bool isMerge = false;
@@ -337,95 +324,6 @@ static bool isLookThroughIfOperandAndResultNonSendable(SILInstruction *inst) {
337324
}
338325
}
339326

340-
static SILValue getUnderlyingTrackedObjectValue(SILValue value) {
341-
auto *fn = value->getFunction();
342-
SILValue result = value;
343-
while (true) {
344-
SILValue temp = result;
345-
346-
if (auto *svi = dyn_cast<SingleValueInstruction>(temp)) {
347-
if (isStaticallyLookThroughInst(svi)) {
348-
temp = svi->getOperand(0);
349-
}
350-
351-
// If we have a cast and our operand and result are non-Sendable, treat it
352-
// as a look through.
353-
if (isLookThroughIfOperandAndResultNonSendable(svi)) {
354-
if (SILIsolationInfo::isNonSendableType(svi->getType(), fn) &&
355-
SILIsolationInfo::isNonSendableType(svi->getOperand(0)->getType(),
356-
fn)) {
357-
temp = svi->getOperand(0);
358-
}
359-
}
360-
361-
if (isLookThroughIfResultNonSendable(svi)) {
362-
if (SILIsolationInfo::isNonSendableType(svi->getType(), fn)) {
363-
temp = svi->getOperand(0);
364-
}
365-
}
366-
367-
if (isLookThroughIfOperandNonSendable(svi)) {
368-
// If our operand is a non-Sendable type, look through this instruction.
369-
if (SILIsolationInfo::isNonSendableType(svi->getOperand(0)->getType(),
370-
fn)) {
371-
temp = svi->getOperand(0);
372-
}
373-
}
374-
}
375-
376-
if (auto *inst = temp->getDefiningInstruction()) {
377-
if (isStaticallyLookThroughInst(inst)) {
378-
temp = inst->getOperand(0);
379-
}
380-
}
381-
382-
if (temp != result) {
383-
result = temp;
384-
continue;
385-
}
386-
387-
return result;
388-
}
389-
}
390-
391-
static UnderlyingTrackedValueInfo getUnderlyingTrackedValue(SILValue value) {
392-
// Before a check if the value we are attempting to access is Sendable. In
393-
// such a case, just return early.
394-
if (!SILIsolationInfo::isNonSendableType(value))
395-
return UnderlyingTrackedValueInfo(value);
396-
397-
// Look through a project_box, so that we process it like its operand object.
398-
if (auto *pbi = dyn_cast<ProjectBoxInst>(value)) {
399-
value = pbi->getOperand();
400-
}
401-
402-
if (!value->getType().isAddress()) {
403-
SILValue underlyingValue = getUnderlyingTrackedObjectValue(value);
404-
405-
// If we do not have a load inst, just return the value.
406-
if (!isa<LoadInst, LoadBorrowInst>(underlyingValue)) {
407-
return UnderlyingTrackedValueInfo(underlyingValue);
408-
}
409-
410-
// If we got an address, lets see if we can do even better by looking at the
411-
// address.
412-
value = cast<SingleValueInstruction>(underlyingValue)->getOperand(0);
413-
}
414-
assert(value->getType().isAddress());
415-
416-
UseDefChainVisitor visitor;
417-
SILValue base = visitor.visitAll(value);
418-
assert(base);
419-
if (base->getType().isObject())
420-
return {getUnderlyingTrackedValue(base).value, visitor.actorIsolation};
421-
422-
return {base, visitor.actorIsolation};
423-
}
424-
425-
SILValue RegionAnalysisFunctionInfo::getUnderlyingTrackedValue(SILValue value) {
426-
return ::getUnderlyingTrackedValue(value).value;
427-
}
428-
429327
namespace {
430328

431329
struct TermArgSources {
@@ -915,6 +813,97 @@ void RegionAnalysisValueMap::print(llvm::raw_ostream &os) const {
915813
#endif
916814
}
917815

816+
static SILValue getUnderlyingTrackedObjectValue(SILValue value) {
817+
auto *fn = value->getFunction();
818+
SILValue result = value;
819+
while (true) {
820+
SILValue temp = result;
821+
822+
if (auto *svi = dyn_cast<SingleValueInstruction>(temp)) {
823+
if (isStaticallyLookThroughInst(svi)) {
824+
temp = svi->getOperand(0);
825+
}
826+
827+
// If we have a cast and our operand and result are non-Sendable, treat it
828+
// as a look through.
829+
if (isLookThroughIfOperandAndResultNonSendable(svi)) {
830+
if (SILIsolationInfo::isNonSendableType(svi->getType(), fn) &&
831+
SILIsolationInfo::isNonSendableType(svi->getOperand(0)->getType(),
832+
fn)) {
833+
temp = svi->getOperand(0);
834+
}
835+
}
836+
837+
if (isLookThroughIfResultNonSendable(svi)) {
838+
if (SILIsolationInfo::isNonSendableType(svi->getType(), fn)) {
839+
temp = svi->getOperand(0);
840+
}
841+
}
842+
843+
if (isLookThroughIfOperandNonSendable(svi)) {
844+
// If our operand is a non-Sendable type, look through this instruction.
845+
if (SILIsolationInfo::isNonSendableType(svi->getOperand(0)->getType(),
846+
fn)) {
847+
temp = svi->getOperand(0);
848+
}
849+
}
850+
}
851+
852+
if (auto *inst = temp->getDefiningInstruction()) {
853+
if (isStaticallyLookThroughInst(inst)) {
854+
temp = inst->getOperand(0);
855+
}
856+
}
857+
858+
if (temp != result) {
859+
result = temp;
860+
continue;
861+
}
862+
863+
return result;
864+
}
865+
}
866+
867+
RegionAnalysisValueMap::UnderlyingTrackedValueInfo
868+
RegionAnalysisValueMap::getUnderlyingTrackedValueHelper(SILValue value) const {
869+
// Before a check if the value we are attempting to access is Sendable. In
870+
// such a case, just return early.
871+
if (!SILIsolationInfo::isNonSendableType(value))
872+
return UnderlyingTrackedValueInfo(value);
873+
874+
// Look through a project_box, so that we process it like its operand object.
875+
if (auto *pbi = dyn_cast<ProjectBoxInst>(value)) {
876+
value = pbi->getOperand();
877+
}
878+
879+
if (!value->getType().isAddress()) {
880+
SILValue underlyingValue = getUnderlyingTrackedObjectValue(value);
881+
882+
// If we do not have a load inst, just return the value.
883+
if (!isa<LoadInst, LoadBorrowInst>(underlyingValue)) {
884+
return UnderlyingTrackedValueInfo(underlyingValue);
885+
}
886+
887+
// If we got an address, lets see if we can do even better by looking at the
888+
// address.
889+
value = cast<SingleValueInstruction>(underlyingValue)->getOperand(0);
890+
}
891+
assert(value->getType().isAddress());
892+
893+
UseDefChainVisitor visitor;
894+
SILValue base = visitor.visitAll(value);
895+
assert(base);
896+
if (base->getType().isObject()) {
897+
// NOTE: We purposely recurse into the cached version of our computation
898+
// rather than recurse into getUnderlyingTrackedObjectValueHelper. This is
899+
// safe since we know that value was previously an address so if our base is
900+
// an object, it cannot be the same object.
901+
return {getUnderlyingTrackedValue(base).value, visitor.actorIsolation};
902+
}
903+
904+
return {base, visitor.actorIsolation};
905+
}
906+
918907
//===----------------------------------------------------------------------===//
919908
// MARK: TrackableValue
920909
//===----------------------------------------------------------------------===//
@@ -988,6 +977,7 @@ namespace {
988977
///
989978
/// 2. Just computing reachability early is a very easy way to do this.
990979
struct PartialApplyReachabilityDataflow {
980+
RegionAnalysisValueMap &valueMap;
991981
PostOrderFunctionInfo *pofi;
992982
llvm::DenseMap<SILValue, unsigned> valueToBit;
993983
std::vector<std::pair<SILValue, SILInstruction *>> valueToGenInsts;
@@ -1002,8 +992,10 @@ struct PartialApplyReachabilityDataflow {
1002992
BasicBlockData<BlockState> blockData;
1003993
bool propagatedReachability = false;
1004994

1005-
PartialApplyReachabilityDataflow(SILFunction *fn, PostOrderFunctionInfo *pofi)
1006-
: pofi(pofi), blockData(fn) {}
995+
PartialApplyReachabilityDataflow(SILFunction *fn,
996+
RegionAnalysisValueMap &valueMap,
997+
PostOrderFunctionInfo *pofi)
998+
: valueMap(valueMap), pofi(pofi), blockData(fn) {}
1007999

10081000
/// Begin tracking an operand of a partial apply.
10091001
void add(Operand *op);
@@ -1035,7 +1027,7 @@ struct PartialApplyReachabilityDataflow {
10351027

10361028
private:
10371029
SILValue getRootValue(SILValue value) const {
1038-
return getUnderlyingTrackedValue(value).value;
1030+
return valueMap.getRepresentative(value);
10391031
}
10401032

10411033
unsigned getBitForValue(SILValue value) const {
@@ -1767,7 +1759,8 @@ class PartitionOpTranslator {
17671759
RegionAnalysisValueMap &valueMap,
17681760
IsolationHistory::Factory &historyFactory)
17691761
: function(function), functionArgPartition(), builder(),
1770-
partialApplyReachabilityDataflow(function, pofi), valueMap(valueMap) {
1762+
partialApplyReachabilityDataflow(function, valueMap, pofi),
1763+
valueMap(valueMap) {
17711764
builder.translator = this;
17721765

17731766
REGIONBASEDISOLATION_LOG(

0 commit comments

Comments
 (0)