[PATCH] D99138: [deref] Infer a few more cases of global dereferenceability in a callee

[Philip Reames via Phabricator via llvm-commits]

reames created this revision.
reames added reviewers: nlopes, jdoerfert, nlewycky, apilipenko.
Herald added subscribers: dexonsmith, dantrushin, hiraditya, mcrosier.
Herald added a reviewer: bollu.
reames requested review of this revision.
Herald added a project: LLVM.

Follow up to D99135 <https://reviews.llvm.org/D99135>.  Only real commonality between the two cases is that I noticed both of them while looking at changes in existing test cases.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D99138

Files:
  llvm/lib/IR/Value.cpp
  llvm/test/Analysis/ValueTracking/memory-dereferenceable.ll


Index: llvm/test/Analysis/ValueTracking/memory-dereferenceable.ll
===================================================================
--- llvm/test/Analysis/ValueTracking/memory-dereferenceable.ll
+++ llvm/test/Analysis/ValueTracking/memory-dereferenceable.ll
@@ -25,8 +25,7 @@
 
 ; Loads from sret arguments
 ; CHECK-LABEL: 'test_sret'
-; GLOBAL: %sret_gep{{.*}}(aligned)
-; POINT-NOT: %sret_gep{{.*}}(aligned)
+; CHECK: %sret_gep{{.*}}(aligned)
 ; CHECK-NOT: %sret_gep_outside
 define void @test_sret(%struct.A* sret(%struct.A) %result) {
   %sret_gep = getelementptr inbounds %struct.A, %struct.A* %result, i64 0, i32 1, i64 2
@@ -210,22 +209,23 @@
 
 ; Loads from byval arguments
 ; CHECK-LABEL: 'byval'
-; GLOBAL: %i8_byval{{.*}}(aligned)
-; POINT-NOT: %i8_byval{{.*}}(aligned)
-; CHECK-NOT: %byval_cast
-; GLOBAL: %byval_gep{{.*}}(aligned)
-; POINT-NOT: %byval_gep{{.*}}(aligned)
-; FIXME: Should hold in the point semantics case too
+; CHECK: %i8_byval{{.*}}(aligned)
+; CHECK-NOT: %bad_byval_cast
+; CHECK: %byval_gep{{.*}}(aligned)
+; CHECK: %good_byval_cast{{.*}}(unaligned)
 define void @byval(i8* byval(i8) %i8_byval,
-                        %struct.A* byval(%struct.A) %A_byval) {
+                   %struct.A* byval(%struct.A) %A_byval) {
   call void @mayfree()
-  %i8_byval_load = load i8, i8* %i8_byval
+  load i8, i8* %i8_byval
 
-  %byval_cast = bitcast i8* %i8_byval to i32*
-  %bad_byval_load = load i32, i32* %byval_cast
+  %bad_byval_cast = bitcast i8* %i8_byval to i32*
+  load i32, i32* %bad_byval_cast
 
   %byval_gep = getelementptr inbounds %struct.A, %struct.A* %A_byval, i64 0, i32 1, i64 2
   load i8, i8* %byval_gep
+  %good_byval_cast = bitcast %struct.A* %A_byval to i32*
+  load i32, i32* %good_byval_cast
+
   ret void
 }
 
@@ -260,9 +260,7 @@
 }
 
 ; CHECK-LABEL: 'infer_func_attrs2'
-; GLOBAL: %p
-; POINT-NOT: %p
-; FIXME: Can be inferred from attributes
+; CHECK: %p
 define void @infer_func_attrs2(i32* dereferenceable(8) %p) readonly {
   call void @mayfree()
   %v = load i32, i32* %p
Index: llvm/lib/IR/Value.cpp
===================================================================
--- llvm/lib/IR/Value.cpp
+++ llvm/lib/IR/Value.cpp
@@ -745,10 +745,25 @@
     // An argument to a function which neither frees, nor can arrange
     // for another thread to free on it's behalf, can not be freed in it's
     // defined scope.
-    // TODO: Generalize this case for noalias call results
+    // TODO: Generalize these two cases for noalias call results
     const Function *F = A->getParent();
     if (F->doesNotFreeMemory() && F->hasNoSync())
       return false;
+
+    // Free is modeled as writing to the freed memory, so a readonly implies
+    // nofree.  Additionally, coordination with another thread requires at
+    // least one write to shared memory, so readonly also implies nosync.  As
+    // such, this reduces to the case just above. (NOTE: This case is important
+    // in practice as we don't infer nosync, but do infer readonly.)
+    if (F->onlyReadsMemory())
+      return false;
+
+    // Handle byval/byref/sret/inalloca/preallocated arguments.  The storage
+    // lifetime is guaranteed to be longer than the callee's lifetime.
+    if (Type *ArgMemTy = A->getPointeeInMemoryValueType())
+      if (ArgMemTy->isSized())
+        return false;
+
     // If there's another copy of V which is freed, then the noalias fact
     // must not have held (i.e. UB).  Note that unlike the function attribute
     // equivalent, nofree on an argument implies another thread can't free it


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D99138.332507.patch
Type: text/x-patch
Size: 3550 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210323/0310dd14/attachment.bin>