Skip to content

Commit 1372414

Browse files
meg-guptameg-gupta
authored
Merge pull request #33722 from meg-gupta/asochanges
Fix KnownSafety optimization bugs in ARCSequenceOpts
2 parents 73a5279 + 586de0a commit 1372414

15 files changed

+908
-146
lines changed

‎lib/SILOptimizer/ARC/ARCBBState.cpp

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,34 @@ void ARCBBState::initPredTopDown(ARCBBState &PredBBState) {
137137
PtrToTopDownState = PredBBState.PtrToTopDownState;
138138
}
139139

140+
void ARCBBState::dumpBottomUpState() {
141+
for (auto state : getBottomupStates()) {
142+
if (!state.hasValue())
143+
continue;
144+
auto elem = state.getValue();
145+
if (!elem.first)
146+
continue;
147+
llvm::dbgs() << "SILValue: ";
148+
elem.first->dump();
149+
llvm::dbgs() << "RefCountState: ";
150+
elem.second.dump();
151+
}
152+
}
153+
154+
void ARCBBState::dumpTopDownState() {
155+
for (auto state : getTopDownStates()) {
156+
if (!state.hasValue())
157+
continue;
158+
auto elem = state.getValue();
159+
if (!elem.first)
160+
continue;
161+
llvm::dbgs() << "SILValue: ";
162+
elem.first->dump();
163+
llvm::dbgs() << "RefCountState: ";
164+
elem.second.dump();
165+
}
166+
}
167+
140168
//===----------------------------------------------------------------------===//
141169
// ARCBBStateInfo
142170
//===----------------------------------------------------------------------===//

‎lib/SILOptimizer/ARC/ARCBBState.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,9 @@ class ARCSequenceDataflowEvaluator::ARCBBState {
137137
/// BB. Used to create an initial state before we merge in other
138138
/// predecessors. This is currently a stub.
139139
void initPredTopDown(ARCBBState &PredBB);
140+
141+
void dumpBottomUpState();
142+
void dumpTopDownState();
140143
};
141144

142145
class ARCSequenceDataflowEvaluator::ARCBBStateInfoHandle {

‎lib/SILOptimizer/ARC/ARCRegionState.cpp

Lines changed: 65 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -156,13 +156,21 @@ void ARCRegionState::mergePredTopDown(ARCRegionState &PredRegionState) {
156156
// Bottom Up Dataflow
157157
//
158158

159-
static bool processBlockBottomUpInsts(
160-
ARCRegionState &State, SILBasicBlock &BB,
161-
BottomUpDataflowRCStateVisitor<ARCRegionState> &DataflowVisitor,
162-
AliasAnalysis *AA, ImmutablePointerSetFactory<SILInstruction> &SetFactory) {
159+
bool ARCRegionState::processBlockBottomUp(
160+
const LoopRegion *R, AliasAnalysis *AA, RCIdentityFunctionInfo *RCIA,
161+
EpilogueARCFunctionInfo *EAFI, LoopRegionFunctionInfo *LRFI,
162+
bool FreezeOwnedArgEpilogueReleases,
163+
BlotMapVector<SILInstruction *, BottomUpRefCountState> &IncToDecStateMap,
164+
ImmutablePointerSetFactory<SILInstruction> &SetFactory) {
165+
LLVM_DEBUG(llvm::dbgs() << ">>>> Bottom Up!\n");
163166

164-
auto II = State.summarizedinterestinginsts_rbegin();
165-
auto IE = State.summarizedinterestinginsts_rend();
167+
SILBasicBlock &BB = *R->getBlock();
168+
BottomUpDataflowRCStateVisitor<ARCRegionState> DataflowVisitor(
169+
RCIA, EAFI, *this, FreezeOwnedArgEpilogueReleases, IncToDecStateMap,
170+
SetFactory);
171+
172+
auto II = summarizedinterestinginsts_rbegin();
173+
auto IE = summarizedinterestinginsts_rend();
166174

167175
// If we do not have any interesting instructions, bail and return false since
168176
// we can not have any nested instructions.
@@ -196,9 +204,15 @@ static bool processBlockBottomUpInsts(
196204
// that the instruction "visits".
197205
SILValue Op = Result.RCIdentity;
198206

207+
std::function<bool(SILInstruction *)> checkIfRefCountInstIsMatched =
208+
[&IncToDecStateMap](SILInstruction *Inst) {
209+
assert(isa<StrongRetainInst>(Inst) || isa<RetainValueInst>(Inst));
210+
return IncToDecStateMap.find(Inst) != IncToDecStateMap.end();
211+
};
212+
199213
// For all other (reference counted value, ref count state) we are
200214
// tracking...
201-
for (auto &OtherState : State.getBottomupStates()) {
215+
for (auto &OtherState : getBottomupStates()) {
202216
// If the other state's value is blotted, skip it.
203217
if (!OtherState.hasValue())
204218
continue;
@@ -208,33 +222,15 @@ static bool processBlockBottomUpInsts(
208222
if (Op && OtherState->first == Op)
209223
continue;
210224

211-
OtherState->second.updateForSameLoopInst(I, SetFactory, AA);
225+
OtherState->second.updateForSameLoopInst(I, AA);
226+
OtherState->second.checkAndResetKnownSafety(
227+
I, OtherState->first, checkIfRefCountInstIsMatched, RCIA, AA);
212228
}
213229
}
214230

215231
return NestingDetected;
216232
}
217233

218-
bool ARCRegionState::processBlockBottomUp(
219-
const LoopRegion *R, AliasAnalysis *AA, RCIdentityFunctionInfo *RCIA,
220-
EpilogueARCFunctionInfo *EAFI, LoopRegionFunctionInfo *LRFI,
221-
bool FreezeOwnedArgEpilogueReleases,
222-
BlotMapVector<SILInstruction *, BottomUpRefCountState> &IncToDecStateMap,
223-
ImmutablePointerSetFactory<SILInstruction> &SetFactory) {
224-
LLVM_DEBUG(llvm::dbgs() << ">>>> Bottom Up!\n");
225-
226-
SILBasicBlock &BB = *R->getBlock();
227-
BottomUpDataflowRCStateVisitor<ARCRegionState> DataflowVisitor(
228-
RCIA, EAFI, *this, FreezeOwnedArgEpilogueReleases, IncToDecStateMap,
229-
SetFactory);
230-
231-
// Visit each arc relevant instruction I in BB visited in reverse...
232-
bool NestingDetected =
233-
processBlockBottomUpInsts(*this, BB, DataflowVisitor, AA, SetFactory);
234-
235-
return NestingDetected;
236-
}
237-
238234
// Returns true if any of the non-local successors of the region are leaking
239235
// blocks. We currently do not handle early exits, but do handle trapping
240236
// blocks. Returns false if otherwise
@@ -257,8 +253,9 @@ static bool hasEarlyExits(
257253

258254
bool ARCRegionState::processLoopBottomUp(
259255
const LoopRegion *R, AliasAnalysis *AA, LoopRegionFunctionInfo *LRFI,
256+
RCIdentityFunctionInfo *RCIA,
260257
llvm::DenseMap<const LoopRegion *, ARCRegionState *> &RegionStateInfo,
261-
ImmutablePointerSetFactory<SILInstruction> &SetFactory) {
258+
llvm::DenseSet<SILInstruction *> &UnmatchedRefCountInsts) {
262259
ARCRegionState *State = RegionStateInfo[R];
263260

264261
// If we find that we have non-leaking early exits, clear state
@@ -268,14 +265,23 @@ bool ARCRegionState::processLoopBottomUp(
268265
return false;
269266
}
270267

268+
std::function<bool(SILInstruction *)> checkIfRefCountInstIsMatched =
269+
[&UnmatchedRefCountInsts](SILInstruction *Inst) {
270+
assert(isa<StrongRetainInst>(Inst) || isa<RetainValueInst>(Inst));
271+
return UnmatchedRefCountInsts.find(Inst) == UnmatchedRefCountInsts.end();
272+
};
273+
271274
// For each state that we are currently tracking, apply our summarized
272275
// instructions to it.
273276
for (auto &OtherState : getBottomupStates()) {
274277
if (!OtherState.hasValue())
275278
continue;
276279

277-
for (auto *I : State->getSummarizedInterestingInsts())
278-
OtherState->second.updateForDifferentLoopInst(I, SetFactory, AA);
280+
for (auto *I : State->getSummarizedInterestingInsts()) {
281+
OtherState->second.updateForDifferentLoopInst(I, AA);
282+
OtherState->second.checkAndResetKnownSafety(
283+
I, OtherState->first, checkIfRefCountInstIsMatched, RCIA, AA);
284+
}
279285
}
280286

281287
return false;
@@ -285,6 +291,7 @@ bool ARCRegionState::processBottomUp(
285291
AliasAnalysis *AA, RCIdentityFunctionInfo *RCIA,
286292
EpilogueARCFunctionInfo *EAFI, LoopRegionFunctionInfo *LRFI,
287293
bool FreezeOwnedArgEpilogueReleases,
294+
llvm::DenseSet<SILInstruction *> &UnmatchedRefCountInsts,
288295
BlotMapVector<SILInstruction *, BottomUpRefCountState> &IncToDecStateMap,
289296
llvm::DenseMap<const LoopRegion *, ARCRegionState *> &RegionStateInfo,
290297
ImmutablePointerSetFactory<SILInstruction> &SetFactory) {
@@ -293,7 +300,8 @@ bool ARCRegionState::processBottomUp(
293300
// We only process basic blocks for now. This ensures that we always propagate
294301
// the empty set from loops.
295302
if (!R->isBlock())
296-
return processLoopBottomUp(R, AA, LRFI, RegionStateInfo, SetFactory);
303+
return processLoopBottomUp(R, AA, LRFI, RCIA, RegionStateInfo,
304+
UnmatchedRefCountInsts);
297305

298306
return processBlockBottomUp(R, AA, RCIA, EAFI, LRFI, FreezeOwnedArgEpilogueReleases,
299307
IncToDecStateMap, SetFactory);
@@ -349,6 +357,12 @@ bool ARCRegionState::processBlockTopDown(
349357
// that the instruction "visits".
350358
SILValue Op = Result.RCIdentity;
351359

360+
std::function<bool(SILInstruction *)> checkIfRefCountInstIsMatched =
361+
[&DecToIncStateMap](SILInstruction *Inst) {
362+
assert(isa<StrongReleaseInst>(Inst) || isa<ReleaseValueInst>(Inst));
363+
return DecToIncStateMap.find(Inst) != DecToIncStateMap.end();
364+
};
365+
352366
// For all other [(SILValue, TopDownState)] we are tracking...
353367
for (auto &OtherState : getTopDownStates()) {
354368
// If the other state's value is blotted, skip it.
@@ -361,7 +375,9 @@ bool ARCRegionState::processBlockTopDown(
361375
if (Op && OtherState->first == Op)
362376
continue;
363377

364-
OtherState->second.updateForSameLoopInst(I, SetFactory, AA);
378+
OtherState->second.updateForSameLoopInst(I, AA);
379+
OtherState->second.checkAndResetKnownSafety(
380+
I, OtherState->first, checkIfRefCountInstIsMatched, RCIA, AA);
365381
}
366382
}
367383

@@ -370,8 +386,8 @@ bool ARCRegionState::processBlockTopDown(
370386

371387
bool ARCRegionState::processLoopTopDown(
372388
const LoopRegion *R, ARCRegionState *State, AliasAnalysis *AA,
373-
LoopRegionFunctionInfo *LRFI,
374-
ImmutablePointerSetFactory<SILInstruction> &SetFactory) {
389+
LoopRegionFunctionInfo *LRFI, RCIdentityFunctionInfo *RCIA,
390+
llvm::DenseSet<SILInstruction *> &UnmatchedRefCountInsts) {
375391

376392
assert(R->isLoop() && "We assume we are processing a loop");
377393

@@ -387,14 +403,23 @@ bool ARCRegionState::processLoopTopDown(
387403
assert(PredRegion->isBlock() && "Expected the predecessor region to be a "
388404
"block");
389405

406+
std::function<bool(SILInstruction *)> checkIfRefCountInstIsMatched =
407+
[&UnmatchedRefCountInsts](SILInstruction *Inst) {
408+
assert(isa<StrongReleaseInst>(Inst) || isa<ReleaseValueInst>(Inst));
409+
return UnmatchedRefCountInsts.find(Inst) == UnmatchedRefCountInsts.end();
410+
};
411+
390412
// For each state that we are currently tracking, apply our summarized
391413
// instructions to it.
392414
for (auto &OtherState : getTopDownStates()) {
393415
if (!OtherState.hasValue())
394416
continue;
395417

396-
for (auto *I : State->getSummarizedInterestingInsts())
397-
OtherState->second.updateForDifferentLoopInst(I, SetFactory, AA);
418+
for (auto *I : State->getSummarizedInterestingInsts()) {
419+
OtherState->second.updateForDifferentLoopInst(I, AA);
420+
OtherState->second.checkAndResetKnownSafety(
421+
I, OtherState->first, checkIfRefCountInstIsMatched, RCIA, AA);
422+
}
398423
}
399424

400425
return false;
@@ -403,6 +428,7 @@ bool ARCRegionState::processLoopTopDown(
403428
bool ARCRegionState::processTopDown(
404429
AliasAnalysis *AA, RCIdentityFunctionInfo *RCIA,
405430
LoopRegionFunctionInfo *LRFI,
431+
llvm::DenseSet<SILInstruction *> &UnmatchedRefCountInsts,
406432
BlotMapVector<SILInstruction *, TopDownRefCountState> &DecToIncStateMap,
407433
llvm::DenseMap<const LoopRegion *, ARCRegionState *> &RegionStateInfo,
408434
ImmutablePointerSetFactory<SILInstruction> &SetFactory) {
@@ -411,7 +437,8 @@ bool ARCRegionState::processTopDown(
411437
// We only process basic blocks for now. This ensures that we always propagate
412438
// the empty set from loops.
413439
if (!R->isBlock())
414-
return processLoopTopDown(R, RegionStateInfo[R], AA, LRFI, SetFactory);
440+
return processLoopTopDown(R, RegionStateInfo[R], AA, LRFI, RCIA,
441+
UnmatchedRefCountInsts);
415442

416443
return processBlockTopDown(*R->getBlock(), AA, RCIA, DecToIncStateMap,
417444
SetFactory);

‎lib/SILOptimizer/ARC/ARCRegionState.h

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,7 @@ class ARCRegionState {
188188
bool processTopDown(
189189
AliasAnalysis *AA, RCIdentityFunctionInfo *RCIA,
190190
LoopRegionFunctionInfo *LRFI,
191+
llvm::DenseSet<SILInstruction *> &UnmatchedRefCountInsts,
191192
BlotMapVector<SILInstruction *, TopDownRefCountState> &DecToIncStateMap,
192193
llvm::DenseMap<const LoopRegion *, ARCRegionState *> &LoopRegionState,
193194
ImmutablePointerSetFactory<SILInstruction> &SetFactory);
@@ -200,6 +201,7 @@ class ARCRegionState {
200201
AliasAnalysis *AA, RCIdentityFunctionInfo *RCIA,
201202
EpilogueARCFunctionInfo *EAFI, LoopRegionFunctionInfo *LRFI,
202203
bool FreezeOwnedArgEpilogueReleases,
204+
llvm::DenseSet<SILInstruction *> &UnmatchedRefCountInsts,
203205
BlotMapVector<SILInstruction *, BottomUpRefCountState> &IncToDecStateMap,
204206
llvm::DenseMap<const LoopRegion *, ARCRegionState *> &RegionStateInfo,
205207
ImmutablePointerSetFactory<SILInstruction> &SetFactory);
@@ -219,9 +221,6 @@ class ARCRegionState {
219221
void removeInterestingInst(SILInstruction *I);
220222

221223
private:
222-
void processBlockBottomUpPredTerminators(
223-
const LoopRegion *R, AliasAnalysis *AA, LoopRegionFunctionInfo *LRFI,
224-
ImmutablePointerSetFactory<SILInstruction> &SetFactory);
225224
bool processBlockBottomUp(
226225
const LoopRegion *R, AliasAnalysis *AA, RCIdentityFunctionInfo *RCIA,
227226
EpilogueARCFunctionInfo *EAFI,
@@ -230,17 +229,18 @@ class ARCRegionState {
230229
ImmutablePointerSetFactory<SILInstruction> &SetFactory);
231230
bool processLoopBottomUp(
232231
const LoopRegion *R, AliasAnalysis *AA, LoopRegionFunctionInfo *LRFI,
232+
RCIdentityFunctionInfo *RCIA,
233233
llvm::DenseMap<const LoopRegion *, ARCRegionState *> &RegionStateInfo,
234-
ImmutablePointerSetFactory<SILInstruction> &SetFactory);
234+
llvm::DenseSet<SILInstruction *> &UnmatchedRefCountInsts);
235235

236236
bool processBlockTopDown(
237237
SILBasicBlock &BB, AliasAnalysis *AA, RCIdentityFunctionInfo *RCIA,
238238
BlotMapVector<SILInstruction *, TopDownRefCountState> &DecToIncStateMap,
239239
ImmutablePointerSetFactory<SILInstruction> &SetFactory);
240-
bool
241-
processLoopTopDown(const LoopRegion *R, ARCRegionState *State,
242-
AliasAnalysis *AA, LoopRegionFunctionInfo *LRFI,
243-
ImmutablePointerSetFactory<SILInstruction> &SetFactory);
240+
bool processLoopTopDown(
241+
const LoopRegion *R, ARCRegionState *State, AliasAnalysis *AA,
242+
LoopRegionFunctionInfo *LRFI, RCIdentityFunctionInfo *RCIA,
243+
llvm::DenseSet<SILInstruction *> &UnmatchedRefCountInsts);
244244

245245
void summarizeLoop(
246246
const LoopRegion *R, LoopRegionFunctionInfo *LRFI,

‎lib/SILOptimizer/ARC/ARCSequenceOpts.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,7 @@ bool LoopARCPairingContext::processRegion(const LoopRegion *Region,
156156
}
157157

158158
MadeChange |= MatchedPair;
159+
Evaluator.saveMatchingInfo(Region);
159160
Evaluator.clearLoopState(Region);
160161
Context.DecToIncStateMap.clear();
161162
Context.IncToDecStateMap.clear();

0 commit comments

Comments
 (0)