Skip to content

Commit 581f611

Browse files
authored
ARCSequenceOpts: Add LoopSummary verifier (#33810)
ARCSequenceOpts with loop support works on regions inside out. While processing an outer region, it uses summary from the inner loop region to compute RefCountState transitions used for CodeMotionSafe optimization. We do not compute the loop summary by iterating over it twice or iterating until a fixed point is reached. Instead loop summary is just a list of refcount effecting instructions. And BottomUpRefCountState::updateForDifferentLoopInst and TopDownRefCountState::updateForDifferentLoopInst are used to compute the RefCountState transition when processing the inner loop region. These functions are very conservative and the flow sensitive nature of the summarized instructions do no matter. This PR adds a verifying assert to confirm this.
1 parent b96eb1d commit 581f611

File tree

3 files changed

+47
-0
lines changed

3 files changed

+47
-0
lines changed

lib/SILOptimizer/ARC/ARCRegionState.cpp

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,15 @@
1818
#include "swift/SILOptimizer/Analysis/LoopRegionAnalysis.h"
1919
#include "swift/SILOptimizer/Analysis/AliasAnalysis.h"
2020
#include "swift/SILOptimizer/Analysis/RCIdentityAnalysis.h"
21+
#include "llvm/Support/CommandLine.h"
2122
#include "llvm/Support/Debug.h"
2223

2324
using namespace swift;
2425

26+
llvm::cl::opt<bool> verifyARCLoopSummary(
27+
"verify-arc-loop-summary", llvm::cl::init(false),
28+
llvm::cl::desc("Verify if loop summary is correct in ARCLoopsOpts"));
29+
2530
//===----------------------------------------------------------------------===//
2631
// ARCRegionState
2732
//===----------------------------------------------------------------------===//
@@ -282,6 +287,18 @@ bool ARCRegionState::processLoopBottomUp(
282287
OtherState->second.checkAndResetKnownSafety(
283288
I, OtherState->first, checkIfRefCountInstIsMatched, RCIA, AA);
284289
}
290+
#ifndef NDEBUG
291+
// Verify updateForDifferentLoopInst is conservative enough that the flow
292+
// sensitive native of the loop summarized instructions does not matter.
293+
if (verifyARCLoopSummary) {
294+
auto NewRefCountState = OtherState->second;
295+
for (auto *I : State->getSummarizedInterestingInsts()) {
296+
NewRefCountState.updateForDifferentLoopInst(I, AA);
297+
}
298+
assert(NewRefCountState.getLatticeState() ==
299+
OtherState->second.getLatticeState());
300+
}
301+
#endif
285302
}
286303

287304
return false;
@@ -420,6 +437,18 @@ bool ARCRegionState::processLoopTopDown(
420437
OtherState->second.checkAndResetKnownSafety(
421438
I, OtherState->first, checkIfRefCountInstIsMatched, RCIA, AA);
422439
}
440+
#ifndef NDEBUG
441+
// Verify updateForDifferentLoopInst is conservative enough that the flow
442+
// sensitive native of the loop summarized instructions does not matter.
443+
if (verifyARCLoopSummary) {
444+
auto NewRefCountState = OtherState->second;
445+
for (auto *I : State->getSummarizedInterestingInsts()) {
446+
NewRefCountState.updateForDifferentLoopInst(I, AA);
447+
}
448+
assert(NewRefCountState.getLatticeState() ==
449+
OtherState->second.getLatticeState());
450+
}
451+
#endif
423452
}
424453

425454
return false;

lib/SILOptimizer/ARC/RefCountState.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -493,13 +493,17 @@ void BottomUpRefCountState::checkAndResetKnownSafety(
493493
clearKnownSafe();
494494
}
495495

496+
// This function is conservative enough that the flow sensitive nature of
497+
// loop summarized instructions does not matter.
496498
void BottomUpRefCountState::updateForDifferentLoopInst(SILInstruction *I,
497499
AliasAnalysis *AA) {
498500
// If we are not tracking anything, bail.
499501
if (!isTrackingRefCount())
500502
return;
501503

502504
if (valueCanBeGuaranteedUsedGivenLatticeState()) {
505+
// Any instruction that may need guaranteed use or may decrement the
506+
// refcount will turn off CodeMotionSafety
503507
if (mayGuaranteedUseValue(I, getRCRoot(), AA) ||
504508
mayDecrementRefCount(I, getRCRoot(), AA)) {
505509
LLVM_DEBUG(llvm::dbgs() << " Found potential guaranteed use:\n "
@@ -917,12 +921,16 @@ void TopDownRefCountState::checkAndResetKnownSafety(
917921
clearKnownSafe();
918922
}
919923

924+
// This function is conservative enough that the flow sensitive nature of
925+
// loop summarized instructions does not matter.
920926
void TopDownRefCountState::updateForDifferentLoopInst(SILInstruction *I,
921927
AliasAnalysis *AA) {
922928
// If we are not tracking anything, bail.
923929
if (!isTrackingRefCount())
924930
return;
925931

932+
// Any instruction that may need guaranteed use or may decrement the
933+
// refcount will turn off CodeMotionSafety
926934
if (valueCanBeGuaranteedUsedGivenLatticeState()) {
927935
if (mayGuaranteedUseValue(I, getRCRoot(), AA) ||
928936
mayDecrementRefCount(I, getRCRoot(), AA)) {

lib/SILOptimizer/ARC/RefCountState.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,9 @@ class BottomUpRefCountState : public RefCountState {
178178
BottomUpRefCountState(BottomUpRefCountState &&) = default;
179179
BottomUpRefCountState &operator=(BottomUpRefCountState &&) = default;
180180

181+
/// Getter for LatticeState
182+
LatticeState getLatticeState() const {return LatState;}
183+
181184
/// Return true if the release can be moved to the retain.
182185
bool isCodeMotionSafe() const {
183186
return LatState != LatticeState::MightBeDecremented;
@@ -205,6 +208,8 @@ class BottomUpRefCountState : public RefCountState {
205208
/// The main difference in between this routine and update for same loop inst
206209
/// is that if we see any decrements on a value, we treat it as being
207210
/// guaranteed used. We treat any uses as regular uses.
211+
/// This function is conservative enough that the flow sensitive nature of
212+
/// loop summarized instructions does not matter.
208213
void updateForDifferentLoopInst(
209214
SILInstruction *I,
210215
AliasAnalysis *AA);
@@ -312,6 +317,9 @@ class TopDownRefCountState : public RefCountState {
312317
TopDownRefCountState(TopDownRefCountState &&) = default;
313318
TopDownRefCountState &operator=(TopDownRefCountState &&) = default;
314319

320+
/// Getter for LatticeState
321+
LatticeState getLatticeState() const {return LatState;}
322+
315323
/// Return true if the retain can be moved to the release.
316324
bool isCodeMotionSafe() const {
317325
return LatState != LatticeState::MightBeUsed;
@@ -350,6 +358,8 @@ class TopDownRefCountState : public RefCountState {
350358
/// The main difference in between this routine and update for same loop inst
351359
/// is that if we see any decrements on a value, we treat it as being
352360
/// guaranteed used. We treat any uses as regular uses.
361+
/// This function is conservative enough that the flow sensitive nature of
362+
/// loop summarized instructions does not matter.
353363
void updateForDifferentLoopInst(
354364
SILInstruction *I,
355365
AliasAnalysis *AA);

0 commit comments

Comments
 (0)