Skip to content

Commit d69ee88

Browse files
authored
[CaptureTracking] Remove dereferenceable_or_null special case (#135613)
Remove the special case where comparing a dereferenceable_or_null pointer with null results in captures(none) instead of captures(address_is_null). This special case is not entirely correct. Let's say we have an allocated object of size 2 at address 1 and have a pointer `%p` pointing either to address 1 or 2. Then passing `gep p, -1` to a `dereferenceable_or_null(1)` function is well-defined, and allows us to distinguish between the two possible pointers, capturing information about the address. Now that we ignore address captures in alias analysis, I think we're ready to drop this special case. Additionally, if there are regressions in other places, the fact that this is inferred as address_is_null should allow us to easily address them if necessary.
1 parent 6f91bfc commit d69ee88

File tree

7 files changed

+22
-82
lines changed

7 files changed

+22
-82
lines changed

llvm/include/llvm/Analysis/CaptureTracking.h

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -168,11 +168,6 @@ namespace llvm {
168168
/// Return one of Stop, Continue or ContinueIgnoringReturn to control
169169
/// further traversal.
170170
virtual Action captured(const Use *U, UseCaptureInfo CI) = 0;
171-
172-
/// isDereferenceableOrNull - Overload to allow clients with additional
173-
/// knowledge about pointer dereferenceability to provide it and thereby
174-
/// avoid conservative responses when a pointer is compared to null.
175-
virtual bool isDereferenceableOrNull(Value *O, const DataLayout &DL);
176171
};
177172

178173
/// Determine what kind of capture behaviour \p U may exhibit.
@@ -183,12 +178,7 @@ namespace llvm {
183178
///
184179
/// \p Base is the starting value of the capture analysis, which is
185180
/// relevant for address_is_null captures.
186-
/// The \p IsDereferenceableOrNull callback is used to rule out capturing for
187-
/// certain comparisons.
188-
UseCaptureInfo
189-
DetermineUseCaptureKind(const Use &U, const Value *Base,
190-
llvm::function_ref<bool(Value *, const DataLayout &)>
191-
IsDereferenceableOrNull);
181+
UseCaptureInfo DetermineUseCaptureKind(const Use &U, const Value *Base);
192182

193183
/// PointerMayBeCaptured - Visit the value and the values derived from it and
194184
/// find values which appear to be capturing the pointer value. This feeds

llvm/lib/Analysis/CaptureTracking.cpp

Lines changed: 2 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -56,21 +56,6 @@ CaptureTracker::~CaptureTracker() = default;
5656

5757
bool CaptureTracker::shouldExplore(const Use *U) { return true; }
5858

59-
bool CaptureTracker::isDereferenceableOrNull(Value *O, const DataLayout &DL) {
60-
// We want comparisons to null pointers to not be considered capturing,
61-
// but need to guard against cases like gep(p, -ptrtoint(p2)) == null,
62-
// which are equivalent to p == p2 and would capture the pointer.
63-
//
64-
// A dereferenceable pointer is a case where this is known to be safe,
65-
// because the pointer resulting from such a construction would not be
66-
// dereferenceable.
67-
//
68-
// It is not sufficient to check for inbounds GEP here, because GEP with
69-
// zero offset is always inbounds.
70-
bool CanBeNull, CanBeFreed;
71-
return O->getPointerDereferenceableBytes(DL, CanBeNull, CanBeFreed);
72-
}
73-
7459
namespace {
7560
struct SimpleCaptureTracker : public CaptureTracker {
7661
explicit SimpleCaptureTracker(bool ReturnCaptures, CaptureComponents Mask,
@@ -281,9 +266,7 @@ Instruction *llvm::FindEarliestCapture(const Value *V, Function &F,
281266
return CB.EarliestCapture;
282267
}
283268

284-
UseCaptureInfo llvm::DetermineUseCaptureKind(
285-
const Use &U, const Value *Base,
286-
function_ref<bool(Value *, const DataLayout &)> IsDereferenceableOrNull) {
269+
UseCaptureInfo llvm::DetermineUseCaptureKind(const Use &U, const Value *Base) {
287270
Instruction *I = dyn_cast<Instruction>(U.getUser());
288271

289272
// TODO: Investigate non-instruction uses.
@@ -391,15 +374,6 @@ UseCaptureInfo llvm::DetermineUseCaptureKind(
391374
if (U->getType()->getPointerAddressSpace() == 0)
392375
if (isNoAliasCall(U.get()->stripPointerCasts()))
393376
return CaptureComponents::None;
394-
if (!I->getFunction()->nullPointerIsDefined()) {
395-
auto *O = I->getOperand(Idx)->stripPointerCastsSameRepresentation();
396-
// Comparing a dereferenceable_or_null pointer against null cannot
397-
// lead to pointer escapes, because if it is not null it must be a
398-
// valid (in-bounds) pointer.
399-
const DataLayout &DL = I->getDataLayout();
400-
if (IsDereferenceableOrNull && IsDereferenceableOrNull(O, DL))
401-
return CaptureComponents::None;
402-
}
403377

404378
// Check whether this is a comparison of the base pointer against
405379
// null.
@@ -447,12 +421,9 @@ void llvm::PointerMayBeCaptured(const Value *V, CaptureTracker *Tracker,
447421
if (!AddUses(V))
448422
return;
449423

450-
auto IsDereferenceableOrNull = [Tracker](Value *V, const DataLayout &DL) {
451-
return Tracker->isDereferenceableOrNull(V, DL);
452-
};
453424
while (!Worklist.empty()) {
454425
const Use *U = Worklist.pop_back_val();
455-
UseCaptureInfo CI = DetermineUseCaptureKind(*U, V, IsDereferenceableOrNull);
426+
UseCaptureInfo CI = DetermineUseCaptureKind(*U, V);
456427
if (capturesAnything(CI.UseCC)) {
457428
switch (Tracker->captured(U, CI)) {
458429
case CaptureTracker::Stop:

llvm/lib/Transforms/IPO/AttributorAttributes.cpp

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3928,12 +3928,6 @@ struct AANoAliasCallSiteArgument final : AANoAliasImpl {
39283928
// (iii) There is no other pointer argument which could alias with the
39293929
// value.
39303930

3931-
auto IsDereferenceableOrNull = [&](Value *O, const DataLayout &DL) {
3932-
const auto *DerefAA = A.getAAFor<AADereferenceable>(
3933-
*this, IRPosition::value(*O), DepClassTy::OPTIONAL);
3934-
return DerefAA ? DerefAA->getAssumedDereferenceableBytes() : 0;
3935-
};
3936-
39373931
const IRPosition &VIRP = IRPosition::value(getAssociatedValue());
39383932
const Function *ScopeFn = VIRP.getAnchorScope();
39393933
// Check whether the value is captured in the scope using AANoCapture.
@@ -3973,8 +3967,7 @@ struct AANoAliasCallSiteArgument final : AANoAliasImpl {
39733967
// is CGSCC runs. For those we would need to "allow" AANoCapture for
39743968
// a value in the module slice.
39753969
// TODO(captures): Make this more precise.
3976-
UseCaptureInfo CI =
3977-
DetermineUseCaptureKind(U, /*Base=*/nullptr, IsDereferenceableOrNull);
3970+
UseCaptureInfo CI = DetermineUseCaptureKind(U, /*Base=*/nullptr);
39783971
if (capturesNothing(CI))
39793972
return true;
39803973
if (CI.isPassthrough()) {
@@ -6035,16 +6028,9 @@ ChangeStatus AANoCaptureImpl::updateImpl(Attributor &A) {
60356028
}
60366029
}
60376030

6038-
auto IsDereferenceableOrNull = [&](Value *O, const DataLayout &DL) {
6039-
const auto *DerefAA = A.getAAFor<AADereferenceable>(
6040-
*this, IRPosition::value(*O), DepClassTy::OPTIONAL);
6041-
return DerefAA && DerefAA->getAssumedDereferenceableBytes();
6042-
};
6043-
60446031
auto UseCheck = [&](const Use &U, bool &Follow) -> bool {
60456032
// TODO(captures): Make this more precise.
6046-
UseCaptureInfo CI =
6047-
DetermineUseCaptureKind(U, /*Base=*/nullptr, IsDereferenceableOrNull);
6033+
UseCaptureInfo CI = DetermineUseCaptureKind(U, /*Base=*/nullptr);
60486034
if (capturesNothing(CI))
60496035
return true;
60506036
if (CI.isPassthrough()) {
@@ -12177,7 +12163,7 @@ struct AAGlobalValueInfoFloating : public AAGlobalValueInfo {
1217712163
auto UsePred = [&](const Use &U, bool &Follow) -> bool {
1217812164
Uses.insert(&U);
1217912165
// TODO(captures): Make this more precise.
12180-
UseCaptureInfo CI = DetermineUseCaptureKind(U, /*Base=*/nullptr, nullptr);
12166+
UseCaptureInfo CI = DetermineUseCaptureKind(U, /*Base=*/nullptr);
1218112167
if (CI.isPassthrough()) {
1218212168
Follow = true;
1218312169
return true;

llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1519,12 +1519,6 @@ bool MemCpyOptPass::performStackMoveOptzn(Instruction *Load, Instruction *Store,
15191519
SmallSet<Instruction *, 4> AAMetadataInstrs;
15201520
bool SrcNotDom = false;
15211521

1522-
// Recursively track the user and check whether modified alias exist.
1523-
auto IsDereferenceableOrNull = [](Value *V, const DataLayout &DL) -> bool {
1524-
bool CanBeNull, CanBeFreed;
1525-
return V->getPointerDereferenceableBytes(DL, CanBeNull, CanBeFreed);
1526-
};
1527-
15281522
auto CaptureTrackingWithModRef =
15291523
[&](Instruction *AI,
15301524
function_ref<bool(Instruction *)> ModRefCallback) -> bool {
@@ -1551,8 +1545,7 @@ bool MemCpyOptPass::performStackMoveOptzn(Instruction *Load, Instruction *Store,
15511545
}
15521546
if (!Visited.insert(&U).second)
15531547
continue;
1554-
UseCaptureInfo CI =
1555-
DetermineUseCaptureKind(U, AI, IsDereferenceableOrNull);
1548+
UseCaptureInfo CI = DetermineUseCaptureKind(U, AI);
15561549
// TODO(captures): Make this more precise.
15571550
if (capturesAnything(CI.UseCC))
15581551
return false;

llvm/test/Transforms/Attributor/nocapture-1.ll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -741,7 +741,7 @@ define i1 @nocaptureInboundsGEPICmpRev(ptr %x) {
741741
define i1 @nocaptureDereferenceableOrNullICmp(ptr dereferenceable_or_null(4) %x) {
742742
; CHECK: Function Attrs: mustprogress nofree norecurse nosync nounwind willreturn memory(none)
743743
; CHECK-LABEL: define {{[^@]+}}@nocaptureDereferenceableOrNullICmp
744-
; CHECK-SAME: (ptr nofree noundef readnone captures(none) dereferenceable_or_null(4) [[X:%.*]]) #[[ATTR0]] {
744+
; CHECK-SAME: (ptr nofree noundef readnone dereferenceable_or_null(4) [[X:%.*]]) #[[ATTR0]] {
745745
; CHECK-NEXT: [[TMP1:%.*]] = icmp eq ptr [[X]], null
746746
; CHECK-NEXT: ret i1 [[TMP1]]
747747
;

llvm/test/Transforms/Attributor/nocapture-2.ll

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -164,14 +164,14 @@ entry:
164164
define ptr @scc_A(ptr dereferenceable_or_null(4) %a) {
165165
; CHECK: Function Attrs: nofree nosync nounwind memory(none)
166166
; CHECK-LABEL: define noundef dereferenceable_or_null(4) ptr @scc_A
167-
; CHECK-SAME: (ptr nofree noundef readnone returned dereferenceable_or_null(4) "no-capture-maybe-returned" [[A:%.*]]) #[[ATTR2:[0-9]+]] {
167+
; CHECK-SAME: (ptr nofree noundef readnone returned dereferenceable_or_null(4) [[A:%.*]]) #[[ATTR2:[0-9]+]] {
168168
; CHECK-NEXT: entry:
169169
; CHECK-NEXT: [[TOBOOL:%.*]] = icmp ne ptr [[A]], null
170170
; CHECK-NEXT: br i1 [[TOBOOL]], label [[COND_TRUE:%.*]], label [[COND_FALSE:%.*]]
171171
; CHECK: cond.true:
172-
; CHECK-NEXT: [[CALL:%.*]] = call dereferenceable_or_null(4) ptr @scc_C(ptr noalias nofree noundef nonnull readnone dereferenceable(4) "no-capture-maybe-returned" [[A]]) #[[ATTR2]]
173-
; CHECK-NEXT: [[CALL1:%.*]] = call dereferenceable_or_null(8) ptr @scc_B(ptr noalias nofree noundef readnone dereferenceable_or_null(8) "no-capture-maybe-returned" [[A]]) #[[ATTR2]]
174-
; CHECK-NEXT: [[CALL2:%.*]] = call ptr @scc_A(ptr noalias nofree noundef readnone dereferenceable_or_null(8) "no-capture-maybe-returned" [[A]]) #[[ATTR2]]
172+
; CHECK-NEXT: [[CALL:%.*]] = call dereferenceable_or_null(4) ptr @scc_C(ptr noalias nofree noundef nonnull readnone dereferenceable(4) [[A]]) #[[ATTR2]]
173+
; CHECK-NEXT: [[CALL1:%.*]] = call dereferenceable_or_null(8) ptr @scc_B(ptr noalias nofree noundef readnone dereferenceable_or_null(8) [[A]]) #[[ATTR2]]
174+
; CHECK-NEXT: [[CALL2:%.*]] = call ptr @scc_A(ptr noalias nofree noundef readnone dereferenceable_or_null(8) [[A]]) #[[ATTR2]]
175175
; CHECK-NEXT: br label [[COND_END:%.*]]
176176
; CHECK: cond.false:
177177
; CHECK-NEXT: br label [[COND_END]]
@@ -201,14 +201,14 @@ cond.end: ; preds = %cond.false, %cond.t
201201
define ptr @scc_B(ptr dereferenceable_or_null(8) %a) {
202202
; CHECK: Function Attrs: nofree nosync nounwind memory(none)
203203
; CHECK-LABEL: define noundef dereferenceable_or_null(8) ptr @scc_B
204-
; CHECK-SAME: (ptr nofree noundef readnone returned dereferenceable_or_null(8) "no-capture-maybe-returned" [[A:%.*]]) #[[ATTR2]] {
204+
; CHECK-SAME: (ptr nofree noundef readnone returned dereferenceable_or_null(8) [[A:%.*]]) #[[ATTR2]] {
205205
; CHECK-NEXT: entry:
206206
; CHECK-NEXT: [[TOBOOL:%.*]] = icmp ne ptr [[A]], null
207207
; CHECK-NEXT: br i1 [[TOBOOL]], label [[COND_TRUE:%.*]], label [[COND_FALSE:%.*]]
208208
; CHECK: cond.true:
209-
; CHECK-NEXT: [[CALL:%.*]] = call dereferenceable_or_null(4) ptr @scc_A(ptr noalias nofree noundef nonnull readnone dereferenceable(8) "no-capture-maybe-returned" [[A]]) #[[ATTR2]]
210-
; CHECK-NEXT: [[CALL1:%.*]] = call dereferenceable_or_null(8) ptr @scc_B(ptr noalias nofree noundef readnone dereferenceable_or_null(8) "no-capture-maybe-returned" [[A]]) #[[ATTR2]]
211-
; CHECK-NEXT: [[CALL2:%.*]] = call ptr @scc_C(ptr noalias nofree noundef readnone dereferenceable_or_null(8) "no-capture-maybe-returned" [[A]]) #[[ATTR2]]
209+
; CHECK-NEXT: [[CALL:%.*]] = call dereferenceable_or_null(4) ptr @scc_A(ptr noalias nofree noundef nonnull readnone dereferenceable(8) [[A]]) #[[ATTR2]]
210+
; CHECK-NEXT: [[CALL1:%.*]] = call dereferenceable_or_null(8) ptr @scc_B(ptr noalias nofree noundef readnone dereferenceable_or_null(8) [[A]]) #[[ATTR2]]
211+
; CHECK-NEXT: [[CALL2:%.*]] = call ptr @scc_C(ptr noalias nofree noundef readnone dereferenceable_or_null(8) [[A]]) #[[ATTR2]]
212212
; CHECK-NEXT: br label [[COND_END:%.*]]
213213
; CHECK: cond.false:
214214
; CHECK-NEXT: br label [[COND_END]]
@@ -237,20 +237,20 @@ cond.end: ; preds = %cond.false, %cond.t
237237
define ptr @scc_C(ptr dereferenceable_or_null(2) %a) {
238238
; CHECK: Function Attrs: nofree nosync nounwind memory(none)
239239
; CHECK-LABEL: define noundef dereferenceable_or_null(4) ptr @scc_C
240-
; CHECK-SAME: (ptr nofree noundef readnone returned dereferenceable_or_null(4) "no-capture-maybe-returned" [[A:%.*]]) #[[ATTR2]] {
240+
; CHECK-SAME: (ptr nofree noundef readnone returned dereferenceable_or_null(4) [[A:%.*]]) #[[ATTR2]] {
241241
; CHECK-NEXT: entry:
242-
; CHECK-NEXT: [[CALL:%.*]] = call dereferenceable_or_null(4) ptr @scc_A(ptr noalias nofree noundef readnone dereferenceable_or_null(4) "no-capture-maybe-returned" [[A]]) #[[ATTR2]]
242+
; CHECK-NEXT: [[CALL:%.*]] = call ptr @scc_A(ptr noalias nofree noundef readnone dereferenceable_or_null(4) [[A]]) #[[ATTR2]]
243243
; CHECK-NEXT: [[TOBOOL:%.*]] = icmp ne ptr [[A]], null
244244
; CHECK-NEXT: br i1 [[TOBOOL]], label [[COND_TRUE:%.*]], label [[COND_FALSE:%.*]]
245245
; CHECK: cond.true:
246-
; CHECK-NEXT: [[CALL1:%.*]] = call ptr @scc_B(ptr noalias nofree noundef readnone dereferenceable_or_null(8) "no-capture-maybe-returned" [[A]]) #[[ATTR2]]
246+
; CHECK-NEXT: [[CALL1:%.*]] = call ptr @scc_B(ptr noalias nofree noundef readnone dereferenceable_or_null(8) [[A]]) #[[ATTR2]]
247247
; CHECK-NEXT: br label [[COND_END:%.*]]
248248
; CHECK: cond.false:
249-
; CHECK-NEXT: [[CALL2:%.*]] = call ptr @scc_C(ptr noalias nofree noundef readnone dereferenceable_or_null(4) "no-capture-maybe-returned" [[A]]) #[[ATTR2]]
249+
; CHECK-NEXT: [[CALL2:%.*]] = call ptr @scc_C(ptr noalias nofree noundef readnone dereferenceable_or_null(4) [[A]]) #[[ATTR2]]
250250
; CHECK-NEXT: br label [[COND_END]]
251251
; CHECK: cond.end:
252252
; CHECK-NEXT: [[COND:%.*]] = phi ptr [ [[A]], [[COND_TRUE]] ], [ [[A]], [[COND_FALSE]] ]
253-
; CHECK-NEXT: [[CALL3:%.*]] = call ptr @scc_A(ptr noalias nofree noundef readnone dereferenceable_or_null(4) "no-capture-maybe-returned" [[A]]) #[[ATTR2]]
253+
; CHECK-NEXT: [[CALL3:%.*]] = call ptr @scc_A(ptr noalias nofree noundef readnone dereferenceable_or_null(4) [[A]]) #[[ATTR2]]
254254
; CHECK-NEXT: ret ptr [[A]]
255255
;
256256
entry:

llvm/test/Transforms/FunctionAttrs/nocapture.ll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -960,7 +960,7 @@ define i1 @inboundsGEPICmpNullPointerDefined(ptr %x) null_pointer_is_valid {
960960
define i1 @nocaptureDereferenceableOrNullICmp(ptr dereferenceable_or_null(4) %x) {
961961
; FNATTRS: Function Attrs: mustprogress nofree norecurse nosync nounwind willreturn memory(none)
962962
; FNATTRS-LABEL: define noundef i1 @nocaptureDereferenceableOrNullICmp
963-
; FNATTRS-SAME: (ptr readnone captures(none) dereferenceable_or_null(4) [[X:%.*]]) #[[ATTR0]] {
963+
; FNATTRS-SAME: (ptr readnone captures(address_is_null) dereferenceable_or_null(4) [[X:%.*]]) #[[ATTR0]] {
964964
; FNATTRS-NEXT: [[TMP1:%.*]] = icmp eq ptr [[X]], null
965965
; FNATTRS-NEXT: ret i1 [[TMP1]]
966966
;

0 commit comments

Comments
 (0)