[CaptureTracking] Don't consider comparison of inbounds GEP with nonnull non-capturing

[nikic]

Summary

This is required to bring CaptureTracking in line with the new semantics from D154051, as gep inbounds p, 0 is now always non-poison.

There are many ways in which the inbounds special case could be preserved: If the index is known non-zero, or there is an inbounds chain down to an identified object, etc. However, I have opted to drop the special case entirely, as it appears to be low value: In cases where we can determine such things (e.g. the affected test cases) we would end up removing the compare via isGEPKnownNonNull() logic anyway.

Depends on: D154051

[efriedma]

LGTM

I'm slightly concerned there's some other impacted usage we missed, but I can't think of anything in particular.

llvm/lib/Analysis/CaptureTracking.cpp

unsigned llvm::getDefaultMaxUsesToExploreForCaptureTracking() {
   return DefaultMaxUsesToExplore;
 }
 
 CaptureTracker::~CaptureTracker() = default;
 
 bool CaptureTracker::shouldExplore(const Use *U) { return true; }
 
 bool CaptureTracker::isDereferenceableOrNull(Value *O, const DataLayout &DL) {
-  // An inbounds GEP can either be a valid pointer (pointing into
-  // or to the end of an allocation), or be null in the default
-  // address space. So for an inbounds GEP there is no way to let
-  // the pointer escape using clever GEP hacking because doing so
-  // would make the pointer point outside of the allocated object
-  // and thus make the GEP result a poison value. Similarly, other
-  // dereferenceable pointers cannot be manipulated without producing
-  // poison.
-  if (auto *GEP = dyn_cast<GetElementPtrInst>(O))
-    if (GEP->isInBounds())
-      return true;
+  // We want comparisons to null pointers to not be considered capturing,
+  // but need to guard against cases like gep(p, -ptrtoint(p2)) == null,
+  // which are equivalent to p == p2 and would capture the pointer.
+  //
+  // A dereferenceable pointer is a case where this is known to be safe,
+  // because the pointer resulting from such a construction would not be
+  // dereferenceable.
+  //
+  // It is not sufficient to check for inbounds GEP here, because GEP with
+  // zero offset is always inbounds.
   bool CanBeNull, CanBeFreed;
   return O->getPointerDereferenceableBytes(DL, CanBeNull, CanBeFreed);
 }
 
 namespace {
   struct SimpleCaptureTracker : public CaptureTracker {
     explicit SimpleCaptureTracker(
 

llvm/test/Transforms/FunctionAttrs/nocapture.ll

 ; CHECK-NEXT:    ret i1 [[TMP1]]
 ;
   %1 = icmp eq ptr null, %x
   ret i1 %1
 }
 
 define i1 @nocaptureInboundsGEPICmp(ptr %x) {
 ; CHECK-LABEL: define i1 @nocaptureInboundsGEPICmp
-; CHECK-SAME: (ptr nocapture readnone [[X:%.*]]) #[[ATTR0]] {
+; CHECK-SAME: (ptr readnone [[X:%.*]]) #[[ATTR0]] {
 ; CHECK-NEXT:    [[TMP1:%.*]] = getelementptr inbounds i32, ptr [[X]], i32 5
 ; CHECK-NEXT:    [[TMP2:%.*]] = icmp eq ptr [[TMP1]], null
 ; CHECK-NEXT:    ret i1 [[TMP2]]
 ;
   %1 = getelementptr inbounds i32, ptr %x, i32 5
   %2 = icmp eq ptr %1, null
   ret i1 %2
 }
 
 define i1 @nocaptureInboundsGEPICmpRev(ptr %x) {
 ; CHECK-LABEL: define i1 @nocaptureInboundsGEPICmpRev
-; CHECK-SAME: (ptr nocapture readnone [[X:%.*]]) #[[ATTR0]] {
+; CHECK-SAME: (ptr readnone [[X:%.*]]) #[[ATTR0]] {
 ; CHECK-NEXT:    [[TMP1:%.*]] = getelementptr inbounds i32, ptr [[X]], i32 5
 ; CHECK-NEXT:    [[TMP2:%.*]] = icmp eq ptr null, [[TMP1]]
 ; CHECK-NEXT:    ret i1 [[TMP2]]
 ;
   %1 = getelementptr inbounds i32, ptr %x, i32 5
   %2 = icmp eq ptr null, %1
   ret i1 %2
 }