Skip to content

Commit

Permalink
[CaptureTracking] Do not capture compares of same object
Browse files Browse the repository at this point in the history
Compares of the same object do not leak any bits.

This patch introduces getUnderlyingObjectLookThrough. It looks at the
output of getUnderlyingObject. If it is a PHI, it looks at all the
incoming underlying objects. If all those objects are the same, or the
original PHI, we determine that there is a new underlying object. This
is similar to getUnderlyingObjects, but provides a more efficient way to
find a single underlying object.

This is an attempt at solving huge compile time regressions in
https://reviews.llvm.org/D152082. First, we only look through a single
PHI, not nested PHIs. Second, we only use one callsite. There are likely
other callsites that could take advantage of this over the vanilla
getUnderlyingObjects. We need to be careful about compile times. Adding
this to BasicAA::aliasCheck increases compile times by 3% on local
builds.

This was inspired by the issues in
rust-lang/rust#111603. rustc used to generate
pointers comparisons that this patch can identify as non capturing. The
code emission was changed in rustc to avoid this behavior, but this
patch can help with other cases of pointer inductions.
  • Loading branch information
caojoshua committed Dec 3, 2023
1 parent 1f5b120 commit ca898a1
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 13 deletions.
7 changes: 7 additions & 0 deletions llvm/include/llvm/Analysis/ValueTracking.h
Original file line number Diff line number Diff line change
Expand Up @@ -693,6 +693,13 @@ inline Value *getArgumentAliasingToReturnedPointer(CallBase *Call,
bool isIntrinsicReturningPointerAliasingArgumentWithoutCapturing(
const CallBase *Call, bool MustPreserveNullness);

/// This method is a wrapper around getUnderlyingObject to look through PHI
/// nodes. This method will attempt to build a new underlying object based on
/// the incoming values. This method can have high compile time implications and
/// cannot be used as a direct replacement for getUnderlyingObject.
const Value *getUnderlyingObjectLookThrough(const Value *V,
unsigned MaxLookup = 6);

/// This method strips off any GEP address adjustments and pointer casts from
/// the specified value, returning the original object being addressed. Note
/// that the returned value has pointer type if the specified value does. If
Expand Down
6 changes: 5 additions & 1 deletion llvm/lib/Analysis/CaptureTracking.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,11 @@ UseCaptureKind llvm::DetermineUseCaptureKind(
if (IsDereferenceableOrNull && IsDereferenceableOrNull(O, DL))
return UseCaptureKind::NO_CAPTURE;
}
}
} else if (cast<ICmpInst>(I)->isEquality() &&
getUnderlyingObjectLookThrough(I->getOperand(Idx)) ==
getUnderlyingObjectLookThrough(I->getOperand(OtherIdx)))
// Equality comparisons against the same pointer do not capture.
return UseCaptureKind::NO_CAPTURE;

// Otherwise, be conservative. There are crazy ways to capture pointers
// using comparisons.
Expand Down
25 changes: 25 additions & 0 deletions llvm/lib/Analysis/ValueTracking.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5968,6 +5968,31 @@ static bool isSameUnderlyingObjectInLoop(const PHINode *PN,
return true;
}

const Value *llvm::getUnderlyingObjectLookThrough(const Value *V, unsigned MaxLookup) {
V = getUnderlyingObject(V, MaxLookup);

const PHINode *PN = dyn_cast<PHINode>(V);
if (!PN)
return V;

// We can look through PHIs if each underlying value has the same underlying
// object, or is the phi itself.
const Value *NewUnderlying = PN;
for (const Value *Incoming : PN->incoming_values()) {
const Value *IncomingUnderlying = getUnderlyingObject(Incoming, MaxLookup);
if (IncomingUnderlying == PN || IncomingUnderlying == NewUnderlying)
continue;
if (NewUnderlying == PN)
// Found a new possible underlying object.
NewUnderlying = IncomingUnderlying;
else
// There are >= 2 possible underlying objects. We cannot determine a new
// underlying object.
return V;
}
return NewUnderlying;
}

const Value *llvm::getUnderlyingObject(const Value *V, unsigned MaxLookup) {
if (!V->getType()->isPointerTy())
return V;
Expand Down
8 changes: 4 additions & 4 deletions llvm/test/Transforms/FunctionAttrs/nocapture.ll
Original file line number Diff line number Diff line change
Expand Up @@ -900,28 +900,28 @@ define void @readnone_indirec(ptr %f, ptr %p) {
ret void
}

; FNATTR: define i1 @identity_icmp(ptr readnone %p)
; FNATTR: define i1 @identity_icmp(ptr nocapture readnone %p)
define i1 @identity_icmp(ptr %p) {
%r = icmp eq ptr %p, %p
ret i1 %r
}

; FNATTR: define i1 @compare_against_offset(ptr readnone %p)
; FNATTR: define i1 @compare_against_offset(ptr nocapture readnone %p)
define i1 @compare_against_offset(ptr %p) {
%offset = getelementptr inbounds i32, ptr %p, i64 1
%r = icmp eq ptr %p, %offset
ret i1 %r
}

; FNATTR: define i1 @compare_offsets(ptr readnone %p)
; FNATTR: define i1 @compare_offsets(ptr nocapture readnone %p)
define i1 @compare_offsets(ptr %p) {
%offset1 = getelementptr inbounds i32, ptr %p, i64 1
%offset2 = getelementptr inbounds i32, ptr %p, i64 2
%r = icmp eq ptr %offset1, %offset2
ret i1 %r
}

; FNATTR: define void @phi_induction(ptr writeonly %p, i64 %n, i32 %x)
; FNATTR: define void @phi_induction(ptr nocapture writeonly %p, i64 %n, i32 %x)
define void @phi_induction(ptr %p, i64 %n, i32 %x) {
start:
%end = getelementptr inbounds i32, ptr %p, i64 %n
Expand Down
9 changes: 1 addition & 8 deletions llvm/unittests/Analysis/CaptureTrackingTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,10 @@ TEST(CaptureTracking, MultipleUsesInSameInstruction) {
BasicBlock *BB = &F->getEntryBlock();
Instruction *Call = &*BB->begin();
Instruction *CmpXChg = Call->getNextNode();
Instruction *ICmp = CmpXChg->getNextNode();

CollectingCaptureTracker CT;
PointerMayBeCaptured(Arg, &CT);
EXPECT_EQ(7u, CT.Captures.size());
EXPECT_EQ(5u, CT.Captures.size());
// Call arg 1
EXPECT_EQ(Call, CT.Captures[0]->getUser());
EXPECT_EQ(0u, CT.Captures[0]->getOperandNo());
Expand All @@ -125,10 +124,4 @@ TEST(CaptureTracking, MultipleUsesInSameInstruction) {
// Cmpxchg new value operand
EXPECT_EQ(CmpXChg, CT.Captures[4]->getUser());
EXPECT_EQ(2u, CT.Captures[4]->getOperandNo());
// ICmp first operand
EXPECT_EQ(ICmp, CT.Captures[5]->getUser());
EXPECT_EQ(0u, CT.Captures[5]->getOperandNo());
// ICmp second operand
EXPECT_EQ(ICmp, CT.Captures[6]->getUser());
EXPECT_EQ(1u, CT.Captures[6]->getOperandNo());
}

0 comments on commit ca898a1

Please sign in to comment.