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

[Nikita Popov via llvm-commits]

Author: Nikita Popov
Date: 2023-07-06T16:48:43+02:00
New Revision: f3d0613d852a90563a1e8704930a6e79368f106a

URL: https://github.com/llvm/llvm-project/commit/f3d0613d852a90563a1e8704930a6e79368f106a
DIFF: https://github.com/llvm/llvm-project/commit/f3d0613d852a90563a1e8704930a6e79368f106a.diff

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

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.

Differential Revision: https://reviews.llvm.org/D154054

Added: 
    

Modified: 
    llvm/lib/Analysis/CaptureTracking.cpp
    llvm/test/Transforms/FunctionAttrs/nocapture.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Analysis/CaptureTracking.cpp b/llvm/lib/Analysis/CaptureTracking.cpp
index e2931973112b44..00e096af3110b9 100644
--- a/llvm/lib/Analysis/CaptureTracking.cpp
+++ b/llvm/lib/Analysis/CaptureTracking.cpp
@@ -58,17 +58,16 @@ 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);
 }

diff  --git a/llvm/test/Transforms/FunctionAttrs/nocapture.ll b/llvm/test/Transforms/FunctionAttrs/nocapture.ll
index 8802bf984c0ec9..6a8372451659d5 100644
--- a/llvm/test/Transforms/FunctionAttrs/nocapture.ll
+++ b/llvm/test/Transforms/FunctionAttrs/nocapture.ll
@@ -488,7 +488,7 @@ define i1 @captureICmpRev(ptr %x) {
 
 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]]
@@ -500,7 +500,7 @@ define i1 @nocaptureInboundsGEPICmp(ptr %x) {
 
 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]]