Skip to content

Commit 9cbdcfc

Browse files
committed
[CaptureTracking] Remove StoreCaptures parameter (NFC)
The implementation doesn't use it, and is unlikely to use it in the future. The places that do set StoreCaptures=false, do so incorrectly and would be broken if the parameter actually did anything.
1 parent 61fb954 commit 9cbdcfc

13 files changed

+25
-55
lines changed

Diff for: llvm/include/llvm/Analysis/CaptureTracking.h

+5-11
Original file line numberDiff line numberDiff line change
@@ -35,31 +35,26 @@ namespace llvm {
3535
/// by the enclosing function (which is required to exist). This routine can
3636
/// be expensive, so consider caching the results. The boolean ReturnCaptures
3737
/// specifies whether returning the value (or part of it) from the function
38-
/// counts as capturing it or not. The boolean StoreCaptures specified
39-
/// whether storing the value (or part of it) into memory anywhere
40-
/// automatically counts as capturing it or not.
38+
/// counts as capturing it or not.
4139
/// MaxUsesToExplore specifies how many uses the analysis should explore for
4240
/// one value before giving up due too "too many uses". If MaxUsesToExplore
4341
/// is zero, a default value is assumed.
4442
bool PointerMayBeCaptured(const Value *V, bool ReturnCaptures,
45-
bool StoreCaptures, unsigned MaxUsesToExplore = 0);
43+
unsigned MaxUsesToExplore = 0);
4644

4745
/// PointerMayBeCapturedBefore - Return true if this pointer value may be
4846
/// captured by the enclosing function (which is required to exist). If a
4947
/// DominatorTree is provided, only captures which happen before the given
5048
/// instruction are considered. This routine can be expensive, so consider
5149
/// caching the results. The boolean ReturnCaptures specifies whether
5250
/// returning the value (or part of it) from the function counts as capturing
53-
/// it or not. The boolean StoreCaptures specified whether storing the value
54-
/// (or part of it) into memory anywhere automatically counts as capturing it
55-
/// or not. Captures by the provided instruction are considered if the
51+
/// it or not. Captures by the provided instruction are considered if the
5652
/// final parameter is true.
5753
/// MaxUsesToExplore specifies how many uses the analysis should explore for
5854
/// one value before giving up due too "too many uses". If MaxUsesToExplore
5955
/// is zero, a default value is assumed.
6056
bool PointerMayBeCapturedBefore(const Value *V, bool ReturnCaptures,
61-
bool StoreCaptures, const Instruction *I,
62-
const DominatorTree *DT,
57+
const Instruction *I, const DominatorTree *DT,
6358
bool IncludeI = false,
6459
unsigned MaxUsesToExplore = 0,
6560
const LoopInfo *LI = nullptr);
@@ -73,8 +68,7 @@ namespace llvm {
7368
// that the instruction the result value is compared against is not in a
7469
// cycle.
7570
Instruction *FindEarliestCapture(const Value *V, Function &F,
76-
bool ReturnCaptures, bool StoreCaptures,
77-
const DominatorTree &DT,
71+
bool ReturnCaptures, const DominatorTree &DT,
7872
unsigned MaxUsesToExplore = 0);
7973

8074
/// This callback is used in conjunction with PointerMayBeCaptured. In

Diff for: llvm/lib/Analysis/AliasAnalysis.cpp

+1-2
Original file line numberDiff line numberDiff line change
@@ -623,8 +623,7 @@ ModRefInfo AAResults::callCapturesBefore(const Instruction *I,
623623
if (!Call || Call == Object)
624624
return ModRefInfo::ModRef;
625625

626-
if (PointerMayBeCapturedBefore(Object, /* ReturnCaptures */ true,
627-
/* StoreCaptures */ true, I, DT,
626+
if (PointerMayBeCapturedBefore(Object, /* ReturnCaptures */ true, I, DT,
628627
/* include Object */ true))
629628
return ModRefInfo::ModRef;
630629

Diff for: llvm/lib/Analysis/BasicAliasAnalysis.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ bool EarliestEscapeAnalysis::isNotCapturedBefore(const Value *Object,
220220
if (Iter.second) {
221221
Instruction *EarliestCapture = FindEarliestCapture(
222222
Object, *const_cast<Function *>(DT.getRoot()->getParent()),
223-
/*ReturnCaptures=*/false, /*StoreCaptures=*/true, DT);
223+
/*ReturnCaptures=*/false, DT);
224224
if (EarliestCapture)
225225
Inst2Obj[EarliestCapture].push_back(Object);
226226
Iter.first->second = EarliestCapture;

Diff for: llvm/lib/Analysis/CaptureTracking.cpp

+6-25
Original file line numberDiff line numberDiff line change
@@ -198,20 +198,12 @@ struct EarliestCaptures : public CaptureTracker {
198198
/// by the enclosing function (which is required to exist). This routine can
199199
/// be expensive, so consider caching the results. The boolean ReturnCaptures
200200
/// specifies whether returning the value (or part of it) from the function
201-
/// counts as capturing it or not. The boolean StoreCaptures specified whether
202-
/// storing the value (or part of it) into memory anywhere automatically
203201
/// counts as capturing it or not.
204202
bool llvm::PointerMayBeCaptured(const Value *V, bool ReturnCaptures,
205-
bool StoreCaptures, unsigned MaxUsesToExplore) {
203+
unsigned MaxUsesToExplore) {
206204
assert(!isa<GlobalValue>(V) &&
207205
"It doesn't make sense to ask whether a global is captured.");
208206

209-
// TODO: If StoreCaptures is not true, we could do Fancy analysis
210-
// to determine whether this store is not actually an escape point.
211-
// In that case, BasicAliasAnalysis should be updated as well to
212-
// take advantage of this.
213-
(void)StoreCaptures;
214-
215207
LLVM_DEBUG(dbgs() << "Captured?: " << *V << " = ");
216208

217209
SimpleCaptureTracker SCT(ReturnCaptures);
@@ -231,23 +223,17 @@ bool llvm::PointerMayBeCaptured(const Value *V, bool ReturnCaptures,
231223
/// instruction are considered. This routine can be expensive, so consider
232224
/// caching the results. The boolean ReturnCaptures specifies whether
233225
/// returning the value (or part of it) from the function counts as capturing
234-
/// it or not. The boolean StoreCaptures specified whether storing the value
235-
/// (or part of it) into memory anywhere automatically counts as capturing it
236-
/// or not.
226+
/// it or not.
237227
bool llvm::PointerMayBeCapturedBefore(const Value *V, bool ReturnCaptures,
238-
bool StoreCaptures, const Instruction *I,
228+
const Instruction *I,
239229
const DominatorTree *DT, bool IncludeI,
240230
unsigned MaxUsesToExplore,
241231
const LoopInfo *LI) {
242232
assert(!isa<GlobalValue>(V) &&
243233
"It doesn't make sense to ask whether a global is captured.");
244234

245235
if (!DT)
246-
return PointerMayBeCaptured(V, ReturnCaptures, StoreCaptures,
247-
MaxUsesToExplore);
248-
249-
// TODO: See comment in PointerMayBeCaptured regarding what could be done
250-
// with StoreCaptures.
236+
return PointerMayBeCaptured(V, ReturnCaptures, MaxUsesToExplore);
251237

252238
CapturesBefore CB(ReturnCaptures, I, DT, IncludeI, LI);
253239
PointerMayBeCaptured(V, &CB, MaxUsesToExplore);
@@ -259,7 +245,7 @@ bool llvm::PointerMayBeCapturedBefore(const Value *V, bool ReturnCaptures,
259245
}
260246

261247
Instruction *llvm::FindEarliestCapture(const Value *V, Function &F,
262-
bool ReturnCaptures, bool StoreCaptures,
248+
bool ReturnCaptures,
263249
const DominatorTree &DT,
264250
unsigned MaxUsesToExplore) {
265251
assert(!isa<GlobalValue>(V) &&
@@ -469,12 +455,7 @@ bool llvm::isNonEscapingLocalObject(
469455

470456
// If this is an identified function-local object, check to see if it escapes.
471457
if (isIdentifiedFunctionLocal(V)) {
472-
// Set StoreCaptures to True so that we can assume in our callers that the
473-
// pointer is not the result of a load instruction. Currently
474-
// PointerMayBeCaptured doesn't have any special analysis for the
475-
// StoreCaptures=false case; if it did, our callers could be refined to be
476-
// more precise.
477-
auto Ret = !PointerMayBeCaptured(V, false, /*StoreCaptures=*/true);
458+
auto Ret = !PointerMayBeCaptured(V, /*ReturnCaptures=*/false);
478459
if (IsCapturedCache)
479460
CacheIt->second = Ret;
480461
return Ret;

Diff for: llvm/lib/Transforms/IPO/FunctionAttrs.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -1430,7 +1430,7 @@ static bool isFunctionMallocLike(Function *F, const SCCNodeSet &SCCNodes) {
14301430
return false; // Did not come from an allocation.
14311431
}
14321432

1433-
if (PointerMayBeCaptured(RetVal, false, /*StoreCaptures=*/false))
1433+
if (PointerMayBeCaptured(RetVal, /*ReturnCaptures=*/false))
14341434
return false;
14351435
}
14361436

Diff for: llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -394,7 +394,7 @@ bool maybeSharedMutable(const Value *Addr) {
394394
return true;
395395

396396
if (isa<AllocaInst>(getUnderlyingObject(Addr)) &&
397-
!PointerMayBeCaptured(Addr, true, true))
397+
!PointerMayBeCaptured(Addr, /*ReturnCaptures=*/true))
398398
return false; // Object is on stack but does not escape.
399399

400400
Addr = Addr->stripInBoundsOffsets();

Diff for: llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -449,7 +449,7 @@ void ThreadSanitizer::chooseInstructionsToInstrument(
449449
}
450450

451451
if (isa<AllocaInst>(getUnderlyingObject(Addr)) &&
452-
!PointerMayBeCaptured(Addr, true, true)) {
452+
!PointerMayBeCaptured(Addr, /*ReturnCaptures=*/true)) {
453453
// The variable is addressable but not captured, so it cannot be
454454
// referenced from a different thread and participate in a data race
455455
// (see llvm/Analysis/CaptureTracking.h for details).

Diff for: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -1200,7 +1200,7 @@ struct DSEState {
12001200
if (!isInvisibleToCallerOnUnwind(V)) {
12011201
I.first->second = false;
12021202
} else if (isNoAliasCall(V)) {
1203-
I.first->second = !PointerMayBeCaptured(V, true, false);
1203+
I.first->second = !PointerMayBeCaptured(V, /*ReturnCaptures=*/true);
12041204
}
12051205
}
12061206
return I.first->second;
@@ -1219,7 +1219,7 @@ struct DSEState {
12191219
// with the killing MemoryDef. But we refrain from doing so for now to
12201220
// limit compile-time and this does not cause any changes to the number
12211221
// of stores removed on a large test set in practice.
1222-
I.first->second = PointerMayBeCaptured(V, false, true);
1222+
I.first->second = PointerMayBeCaptured(V, /*ReturnCaptures=*/false);
12231223
return !I.first->second;
12241224
}
12251225

Diff for: llvm/lib/Transforms/Scalar/LICM.cpp

-1
Original file line numberDiff line numberDiff line change
@@ -1935,7 +1935,6 @@ bool isNotCapturedBeforeOrInLoop(const Value *V, const Loop *L,
19351935
// the loop.
19361936
// TODO: ReturnCaptures=true shouldn't be necessary here.
19371937
return !PointerMayBeCapturedBefore(V, /* ReturnCaptures */ true,
1938-
/* StoreCaptures */ true,
19391938
L->getHeader()->getTerminator(), DT);
19401939
}
19411940

Diff for: llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp

+1-2
Original file line numberDiff line numberDiff line change
@@ -994,8 +994,7 @@ bool MemCpyOptPass::performCallSlotOptzn(Instruction *cpyLoad,
994994
// or src pointer.
995995
Value *DestObj = getUnderlyingObject(cpyDest);
996996
if (!isIdentifiedFunctionLocal(DestObj) ||
997-
PointerMayBeCapturedBefore(DestObj, /* ReturnCaptures */ true,
998-
/* StoreCaptures */ true, C, DT,
997+
PointerMayBeCapturedBefore(DestObj, /* ReturnCaptures */ true, C, DT,
999998
/* IncludeI */ true))
1000999
return false;
10011000

Diff for: llvm/lib/Transforms/Utils/InlineFunction.cpp

+1-2
Original file line numberDiff line numberDiff line change
@@ -1313,8 +1313,7 @@ static void AddAliasScopeMetadata(CallBase &CB, ValueToValueMapTy &VMap,
13131313
// nocapture only guarantees that no copies outlive the function, not
13141314
// that the value cannot be locally captured.
13151315
if (!RequiresNoCaptureBefore ||
1316-
!PointerMayBeCapturedBefore(A, /* ReturnCaptures */ false,
1317-
/* StoreCaptures */ false, I, &DT))
1316+
!PointerMayBeCapturedBefore(A, /* ReturnCaptures */ false, I, &DT))
13181317
NoAliases.push_back(NewScopes[A]);
13191318
}
13201319

Diff for: llvm/lib/Transforms/Utils/SimplifyCFG.cpp

+1-2
Original file line numberDiff line numberDiff line change
@@ -3036,8 +3036,7 @@ static Value *isSafeToSpeculateStore(Instruction *I, BasicBlock *BrBB,
30363036
Value *Obj = getUnderlyingObject(StorePtr);
30373037
bool ExplicitlyDereferenceableOnly;
30383038
if (isWritableObject(Obj, ExplicitlyDereferenceableOnly) &&
3039-
!PointerMayBeCaptured(Obj, /*ReturnCaptures=*/false,
3040-
/*StoreCaptures=*/true) &&
3039+
!PointerMayBeCaptured(Obj, /*ReturnCaptures=*/false) &&
30413040
(!ExplicitlyDereferenceableOnly ||
30423041
isDereferenceablePointer(StorePtr, StoreTy,
30433042
LI->getDataLayout()))) {

Diff for: llvm/unittests/Analysis/CaptureTrackingTest.cpp

+4-4
Original file line numberDiff line numberDiff line change
@@ -56,17 +56,17 @@ TEST(CaptureTracking, MaxUsesToExplore) {
5656
ASSERT_NE(F, nullptr);
5757
Value *Arg = &*F->arg_begin();
5858
ASSERT_NE(Arg, nullptr);
59-
ASSERT_FALSE(PointerMayBeCaptured(Arg, true, true, FalseMaxUsesLimit));
60-
ASSERT_TRUE(PointerMayBeCaptured(Arg, true, true, TrueMaxUsesLimit));
59+
ASSERT_FALSE(PointerMayBeCaptured(Arg, true, FalseMaxUsesLimit));
60+
ASSERT_TRUE(PointerMayBeCaptured(Arg, true, TrueMaxUsesLimit));
6161

6262
BasicBlock *EntryBB = &F->getEntryBlock();
6363
DominatorTree DT(*F);
6464

6565
Instruction *Ret = EntryBB->getTerminator();
6666
ASSERT_TRUE(isa<ReturnInst>(Ret));
67-
ASSERT_FALSE(PointerMayBeCapturedBefore(Arg, true, true, Ret, &DT, false,
67+
ASSERT_FALSE(PointerMayBeCapturedBefore(Arg, true, Ret, &DT, false,
6868
FalseMaxUsesLimit));
69-
ASSERT_TRUE(PointerMayBeCapturedBefore(Arg, true, true, Ret, &DT, false,
69+
ASSERT_TRUE(PointerMayBeCapturedBefore(Arg, true, Ret, &DT, false,
7070
TrueMaxUsesLimit));
7171
};
7272

0 commit comments

Comments
 (0)